New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 658801 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: Dec 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

usleep_range patch might end up busy waiting; pick newer version upstream

Project Member Reported by diand...@chromium.org, Oct 24 2016

Issue description

As discovered in upstream review, the patch that landed in  bug #655255  could possibly end up busy waiting for the end of the delay.  That's not awful but not ideal.

Once the newer version actually lands upstream we should pick fixup patches.
 
Cc: diand...@chromium.org
Owner: dtor@chromium.org
dtor@ has already started this, though he marked with the old bug (chrome-os-partner:58431)
Dmitry: thanks for starting this!  ...are you planning to pick to other kernel trees, also?  I think we landed this across the board.
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 14 2016

Labels: merge-merged-chromeos-3.18
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/b6bb4eafce260860d9667c55107af3c51868141d

commit b6bb4eafce260860d9667c55107af3c51868141d
Author: Dmitry Torokhov <dtor@chromium.org>
Date: Sat Dec 10 00:46:53 2016

Revert "FROMLIST: timers: Fix usleep_range() in the context of wake_up_process()"

This reverts commit f5f520d79ec610cc9c7faf8d6aa73cb5ecbac792 as it is
broken (does bosy-wait in case task is woken up early) and we'll pick up
fixed version from upstream.

BUG= chromium:658801 
TEST=See CL:Ib2eaa31e272c15a9a7a17796a4b22fe954f332bf

Change-Id: Id10b3d6ac0767385a2e5534c459f54ba6bf2216c
Reviewed-on: https://chromium-review.googlesource.com/418877
Commit-Ready: Dmitry Torokhov <dtor@chromium.org>
Tested-by: Dmitry Torokhov <dtor@chromium.org>
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>
(cherry picked from commit 7c9ac25fa67c209666536392ca873cce4ae658e8)
Reviewed-on: https://chromium-review.googlesource.com/419358
Reviewed-by: Douglas Anderson <dianders@chromium.org>

[modify] https://crrev.com/b6bb4eafce260860d9667c55107af3c51868141d/kernel/time/timer.c

Project Member

Comment 4 by bugdroid1@chromium.org, Dec 14 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/d425b6ad9194c09bc6821f8ebd448a9f12af7435

commit d425b6ad9194c09bc6821f8ebd448a9f12af7435
Author: Douglas Anderson <dianders@chromium.org>
Date: Fri Oct 21 15:58:50 2016

UPSTREAM: timers: Fix usleep_range() in the context of wake_up_process()

Users of usleep_range() expect that it will _never_ return in less time
than the minimum passed parameter. However, nothing in the code ensures
this, when the sleeping task is woken by wake_up_process() or any other
mechanism which can wake a task from uninterruptible state.

Neither usleep_range() nor schedule_hrtimeout_range*() have any protection
against wakeups. schedule_hrtimeout_range*() is designed this way despite
the fact that the API documentation does not mention it.

msleep() already has code to handle this case since it will loop as long
as there was still time left.  usleep_range() has no such loop, add it.

Presumably this problem was not detected before because usleep_range() is
only used in a few places and the function is mostly used in contexts which
are not exposed to wakeups of any form.

An effort was made to look for users relying on the old behavior by
looking for usleep_range() in the same file as wake_up_process().
No problems were found by this search, though it is conceivable that
someone could have put the sleep and wakeup in two different files.

An effort was made to ask several upstream maintainers if they were aware
of people relying on wake_up_process() to wake up usleep_range(). No
maintainers were aware of that but they were aware of many people relying
on usleep_range() never returning before the minimum.

Reported-by: Tao Huang <huangtao@rock-chips.com>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Cc: heiko@sntech.de
Cc: broonie@kernel.org
Cc: briannorris@chromium.org
Cc: Andreas Mohr <andi@lisas.de>
Cc: linux-rockchip@lists.infradead.org
Cc: tony.xie@rock-chips.com
Cc: John Stultz <john.stultz@linaro.org>
Cc: djkurtz@chromium.org
Cc: linux@roeck-us.net
Cc: tskd08@gmail.com
Link: http://lkml.kernel.org/r/1477065531-30342-1-git-send-email-dianders@chromium.org
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

BUG= chromium:658801 
TEST=See CL:Ib2eaa31e272c15a9a7a17796a4b22fe954f332bf

(cherry picked from
 git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git
 commit 6c5e9059692567740a4ee51530dffe51a4b9584d)
Signed-off-by: Dmitry Torokhov <dtor@chromium.org>
Change-Id: I69566b3de45c83fbbe52d2b23f2942cb0231f55d
Reviewed-on: https://chromium-review.googlesource.com/418878
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>
(cherry picked from commit 8c5a40400217ed0d1546ccc644d7f78fb6396613)
Reviewed-on: https://chromium-review.googlesource.com/419359
Reviewed-by: Douglas Anderson <dianders@chromium.org>

[modify] https://crrev.com/d425b6ad9194c09bc6821f8ebd448a9f12af7435/kernel/time/timer.c

Project Member

Comment 5 by bugdroid1@chromium.org, Dec 14 2016

Labels: merge-merged-chromeos-3.14
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/19bcccd53e01f6e201aad10221b278479e984fd9

commit 19bcccd53e01f6e201aad10221b278479e984fd9
Author: Dmitry Torokhov <dtor@chromium.org>
Date: Sat Dec 10 00:46:53 2016

Revert "FROMLIST: timers: Fix usleep_range() in the context of wake_up_process()"

This reverts commit f5f520d79ec610cc9c7faf8d6aa73cb5ecbac792 as it is
broken (does busy-wait in case task is woken up early) and we'll pick up
fixed version from upstream.

BUG= chromium:658801 
TEST=See CL:Ib2eaa31e272c15a9a7a17796a4b22fe954f332bf

Change-Id: Id10b3d6ac0767385a2e5534c459f54ba6bf2216c
Reviewed-on: https://chromium-review.googlesource.com/418877
Commit-Ready: Dmitry Torokhov <dtor@chromium.org>
Tested-by: Dmitry Torokhov <dtor@chromium.org>
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>
(cherry picked from commit 7c9ac25fa67c209666536392ca873cce4ae658e8)
 Conflicts:
	kernel/time/timer.c
	... because it is in kernel/timer.c in older kernels.
Reviewed-on: https://chromium-review.googlesource.com/419130
Reviewed-by: Douglas Anderson <dianders@chromium.org>

[modify] https://crrev.com/19bcccd53e01f6e201aad10221b278479e984fd9/kernel/timer.c

Project Member

Comment 6 by bugdroid1@chromium.org, Dec 14 2016

Labels: merge-merged-chromeos-3.14
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/c16e4614294011301767fca32111a1285a06dba5

commit c16e4614294011301767fca32111a1285a06dba5
Author: Douglas Anderson <dianders@chromium.org>
Date: Fri Oct 21 15:58:50 2016

UPSTREAM: timers: Fix usleep_range() in the context of wake_up_process()

Users of usleep_range() expect that it will _never_ return in less time
than the minimum passed parameter. However, nothing in the code ensures
this, when the sleeping task is woken by wake_up_process() or any other
mechanism which can wake a task from uninterruptible state.

Neither usleep_range() nor schedule_hrtimeout_range*() have any protection
against wakeups. schedule_hrtimeout_range*() is designed this way despite
the fact that the API documentation does not mention it.

msleep() already has code to handle this case since it will loop as long
as there was still time left.  usleep_range() has no such loop, add it.

Presumably this problem was not detected before because usleep_range() is
only used in a few places and the function is mostly used in contexts which
are not exposed to wakeups of any form.

An effort was made to look for users relying on the old behavior by
looking for usleep_range() in the same file as wake_up_process().
No problems were found by this search, though it is conceivable that
someone could have put the sleep and wakeup in two different files.

An effort was made to ask several upstream maintainers if they were aware
of people relying on wake_up_process() to wake up usleep_range(). No
maintainers were aware of that but they were aware of many people relying
on usleep_range() never returning before the minimum.

Reported-by: Tao Huang <huangtao@rock-chips.com>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Cc: heiko@sntech.de
Cc: broonie@kernel.org
Cc: briannorris@chromium.org
Cc: Andreas Mohr <andi@lisas.de>
Cc: linux-rockchip@lists.infradead.org
Cc: tony.xie@rock-chips.com
Cc: John Stultz <john.stultz@linaro.org>
Cc: djkurtz@chromium.org
Cc: linux@roeck-us.net
Cc: tskd08@gmail.com
Link: http://lkml.kernel.org/r/1477065531-30342-1-git-send-email-dianders@chromium.org
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

BUG= chromium:658801 
TEST=See CL:Ib2eaa31e272c15a9a7a17796a4b22fe954f332bf

(cherry picked from
 git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git
 commit 6c5e9059692567740a4ee51530dffe51a4b9584d)
Signed-off-by: Dmitry Torokhov <dtor@chromium.org>
Change-Id: I69566b3de45c83fbbe52d2b23f2942cb0231f55d
Reviewed-on: https://chromium-review.googlesource.com/418878
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>
(cherry picked from commit 8c5a40400217ed0d1546ccc644d7f78fb6396613)
 Conflicts:
	kernel/time/timer.c
	... because it is in kernel/timer.c in older kernels.
	... also keeping delta as unsigned long.
Reviewed-on: https://chromium-review.googlesource.com/419131
Reviewed-by: Douglas Anderson <dianders@chromium.org>

[modify] https://crrev.com/c16e4614294011301767fca32111a1285a06dba5/kernel/timer.c

Project Member

Comment 7 by bugdroid1@chromium.org, Dec 16 2016

Labels: merge-merged-chromeos-3.10
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/924ad1dcb3681fef44b76088620ae0ab9193d0fa

commit 924ad1dcb3681fef44b76088620ae0ab9193d0fa
Author: Dmitry Torokhov <dtor@chromium.org>
Date: Sat Dec 10 00:46:53 2016

Revert "FROMLIST: timers: Fix usleep_range() in the context of wake_up_process()"

This reverts commit f5f520d79ec610cc9c7faf8d6aa73cb5ecbac792 as it is
broken (does busy-wait in case task is woken up early) and we'll pick up
fixed version from upstream.

BUG= chromium:658801 
TEST=See CL:Ib2eaa31e272c15a9a7a17796a4b22fe954f332bf

Change-Id: Id10b3d6ac0767385a2e5534c459f54ba6bf2216c
Reviewed-on: https://chromium-review.googlesource.com/418877
Commit-Ready: Dmitry Torokhov <dtor@chromium.org>
Tested-by: Dmitry Torokhov <dtor@chromium.org>
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>
(cherry picked from commit 7c9ac25fa67c209666536392ca873cce4ae658e8)
 Conflicts:
	kernel/time/timer.c
	... because it is in kernel/timer.c in older kernels.
Reviewed-on: https://chromium-review.googlesource.com/419639
Reviewed-by: Douglas Anderson <dianders@chromium.org>

[modify] https://crrev.com/924ad1dcb3681fef44b76088620ae0ab9193d0fa/kernel/timer.c

Project Member

Comment 8 by bugdroid1@chromium.org, Dec 16 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/c3d787f39dd7b9e8d0135bd34f5bf75405b4ccb4

commit c3d787f39dd7b9e8d0135bd34f5bf75405b4ccb4
Author: Douglas Anderson <dianders@chromium.org>
Date: Fri Oct 21 15:58:50 2016

UPSTREAM: timers: Fix usleep_range() in the context of wake_up_process()

Users of usleep_range() expect that it will _never_ return in less time
than the minimum passed parameter. However, nothing in the code ensures
this, when the sleeping task is woken by wake_up_process() or any other
mechanism which can wake a task from uninterruptible state.

Neither usleep_range() nor schedule_hrtimeout_range*() have any protection
against wakeups. schedule_hrtimeout_range*() is designed this way despite
the fact that the API documentation does not mention it.

msleep() already has code to handle this case since it will loop as long
as there was still time left.  usleep_range() has no such loop, add it.

Presumably this problem was not detected before because usleep_range() is
only used in a few places and the function is mostly used in contexts which
are not exposed to wakeups of any form.

An effort was made to look for users relying on the old behavior by
looking for usleep_range() in the same file as wake_up_process().
No problems were found by this search, though it is conceivable that
someone could have put the sleep and wakeup in two different files.

An effort was made to ask several upstream maintainers if they were aware
of people relying on wake_up_process() to wake up usleep_range(). No
maintainers were aware of that but they were aware of many people relying
on usleep_range() never returning before the minimum.

Reported-by: Tao Huang <huangtao@rock-chips.com>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Cc: heiko@sntech.de
Cc: broonie@kernel.org
Cc: briannorris@chromium.org
Cc: Andreas Mohr <andi@lisas.de>
Cc: linux-rockchip@lists.infradead.org
Cc: tony.xie@rock-chips.com
Cc: John Stultz <john.stultz@linaro.org>
Cc: djkurtz@chromium.org
Cc: linux@roeck-us.net
Cc: tskd08@gmail.com
Link: http://lkml.kernel.org/r/1477065531-30342-1-git-send-email-dianders@chromium.org
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

BUG= chromium:658801 
TEST=See CL:Ib2eaa31e272c15a9a7a17796a4b22fe954f332bf

(cherry picked from
 git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git
 commit 6c5e9059692567740a4ee51530dffe51a4b9584d)
Signed-off-by: Dmitry Torokhov <dtor@chromium.org>
Change-Id: I69566b3de45c83fbbe52d2b23f2942cb0231f55d
Reviewed-on: https://chromium-review.googlesource.com/418878
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>
(cherry picked from commit 8c5a40400217ed0d1546ccc644d7f78fb6396613)
 Conflicts:
	kernel/time/timer.c
	... because it is in kernel/timer.c in older kernels.
	... also keeping delta as unsigned long.
Reviewed-on: https://chromium-review.googlesource.com/419640
Reviewed-by: Douglas Anderson <dianders@chromium.org>

[modify] https://crrev.com/c3d787f39dd7b9e8d0135bd34f5bf75405b4ccb4/kernel/timer.c

Project Member

Comment 9 by bugdroid1@chromium.org, Dec 17 2016

Labels: merge-merged-chromeos-3.8
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/61059d65a368e851e542ca5ee250cd93cb54647d

commit 61059d65a368e851e542ca5ee250cd93cb54647d
Author: Dmitry Torokhov <dtor@chromium.org>
Date: Sat Dec 10 00:46:53 2016

Revert "FROMLIST: timers: Fix usleep_range() in the context of wake_up_process()"

This reverts commit f5f520d79ec610cc9c7faf8d6aa73cb5ecbac792 as it is
broken (does busy-wait in case task is woken up early) and we'll pick up
fixed version from upstream.

BUG= chromium:658801 
TEST=See CL:Ib2eaa31e272c15a9a7a17796a4b22fe954f332bf

Change-Id: Id10b3d6ac0767385a2e5534c459f54ba6bf2216c
Reviewed-on: https://chromium-review.googlesource.com/418877
Commit-Ready: Dmitry Torokhov <dtor@chromium.org>
Tested-by: Dmitry Torokhov <dtor@chromium.org>
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>
(cherry picked from commit 7c9ac25fa67c209666536392ca873cce4ae658e8)
 Conflicts:
	kernel/time/timer.c
	... because it is in kernel/timer.c in older kernels.
Reviewed-on: https://chromium-review.googlesource.com/419598
Commit-Ready: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>

[modify] https://crrev.com/61059d65a368e851e542ca5ee250cd93cb54647d/kernel/timer.c

Project Member

Comment 10 by bugdroid1@chromium.org, Dec 17 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/96b25af11162057916c6b58f575928047b8c6134

commit 96b25af11162057916c6b58f575928047b8c6134
Author: Douglas Anderson <dianders@chromium.org>
Date: Fri Oct 21 15:58:50 2016

UPSTREAM: timers: Fix usleep_range() in the context of wake_up_process()

Users of usleep_range() expect that it will _never_ return in less time
than the minimum passed parameter. However, nothing in the code ensures
this, when the sleeping task is woken by wake_up_process() or any other
mechanism which can wake a task from uninterruptible state.

Neither usleep_range() nor schedule_hrtimeout_range*() have any protection
against wakeups. schedule_hrtimeout_range*() is designed this way despite
the fact that the API documentation does not mention it.

msleep() already has code to handle this case since it will loop as long
as there was still time left.  usleep_range() has no such loop, add it.

Presumably this problem was not detected before because usleep_range() is
only used in a few places and the function is mostly used in contexts which
are not exposed to wakeups of any form.

An effort was made to look for users relying on the old behavior by
looking for usleep_range() in the same file as wake_up_process().
No problems were found by this search, though it is conceivable that
someone could have put the sleep and wakeup in two different files.

An effort was made to ask several upstream maintainers if they were aware
of people relying on wake_up_process() to wake up usleep_range(). No
maintainers were aware of that but they were aware of many people relying
on usleep_range() never returning before the minimum.

Reported-by: Tao Huang <huangtao@rock-chips.com>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Cc: heiko@sntech.de
Cc: broonie@kernel.org
Cc: briannorris@chromium.org
Cc: Andreas Mohr <andi@lisas.de>
Cc: linux-rockchip@lists.infradead.org
Cc: tony.xie@rock-chips.com
Cc: John Stultz <john.stultz@linaro.org>
Cc: djkurtz@chromium.org
Cc: linux@roeck-us.net
Cc: tskd08@gmail.com
Link: http://lkml.kernel.org/r/1477065531-30342-1-git-send-email-dianders@chromium.org
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

BUG= chromium:658801 
TEST=See CL:Ib2eaa31e272c15a9a7a17796a4b22fe954f332bf

(cherry picked from
 git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git
 commit 6c5e9059692567740a4ee51530dffe51a4b9584d)
Signed-off-by: Dmitry Torokhov <dtor@chromium.org>
Change-Id: I69566b3de45c83fbbe52d2b23f2942cb0231f55d
Reviewed-on: https://chromium-review.googlesource.com/418878
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>
(cherry picked from commit 8c5a40400217ed0d1546ccc644d7f78fb6396613)
 Conflicts:
	kernel/time/timer.c
	... because it is in kernel/timer.c in older kernels.
	... also keeping delta as unsigned long.
Reviewed-on: https://chromium-review.googlesource.com/419599
Commit-Ready: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>

[modify] https://crrev.com/96b25af11162057916c6b58f575928047b8c6134/kernel/timer.c

I think this is fixed everywhere in M-57 and on kernel-4.4 on M-56.  That's probably fine unless we try to pick the sched tune stuff to 3.18.

Comment 12 by dtor@chromium.org, Dec 20 2016

Status: Fixed (was: Untriaged)
Even if we try picking up EAS/cpufreq_sched into 3.18 I do not think it is going to happen on M-56, so marking as fixed.

Comment 13 by ka...@chromium.org, Jan 26 2017

Status: Verified (was: Fixed)
Verified

Sign in to add a comment