From: Douglas Anderson <dianders@chromium.org>
To: Thomas Gleixner <tglx@linutronix.de>,
John Stultz <john.stultz@linaro.org>
Cc: Andreas Mohr <andi@lisas.de>,
briannorris@chromium.org, huangtao@rock-chips.com,
tony.xie@rock-chips.com, linux-rockchip@lists.infradead.org,
linux@roeck-us.net, heiko@sntech.de, broonie@kernel.org,
djkurtz@chromium.org, tskd08@gmail.com,
Douglas Anderson <dianders@chromium.org>,
linux-kernel@vger.kernel.org
Subject: [PATCH v3 2/2] timers: Fix documentation for schedule_timeout() and similar
Date: Thu, 20 Oct 2016 13:34:24 -0700 [thread overview]
Message-ID: <1476995664-15668-2-git-send-email-dianders@chromium.org> (raw)
In-Reply-To: <1476995664-15668-1-git-send-email-dianders@chromium.org>
The documentatoin for schedule_timeout(), schedule_hrtimeout(), and
schedule_hrtimeout_range() all claimed that the routines couldn't
possibly return early if the task state was TASK_UNINTERRUPTIBLE. This
was simply not true, since anyone calling wake_up_process() would cause
those routines to exit early.
As some evidence that the documentation was broken (not the code):
- If we changed the code to match the documentation, msleep() would be
identical to schedule_timeout_uninterruptible() and
msleep_interruptible() would be identical to
schedule_timeout_interruptible(). That doesn't seem likely to have
been the intention.
- The schedule() function sleeps until a task is woken up. Logically,
one would expect that the schedule_timeout() function would sleep
until a task is woken up or a timeout occurrs.
As part of the above observation, it can be seen that
schedule_hrtimeout() and schedule_hrtimeout_range() might return -EINTR
even if the task state was TASK_UNINTERRUPTIBLE. This isn't terrible
behavior so we'll document it and keep it as-is. After all, trying to
match schedule_timeout() and return the time left would incure a bunch
of extra calculation cost that isn't needed in all cases.
Suggested-by: Daniel Kurtz <djkurtz@chromium.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
Changes in v3:
- Documentation fix new for v3.
kernel/time/hrtimer.c | 20 ++++++++++++++------
kernel/time/timer.c | 11 +++++++----
2 files changed, 21 insertions(+), 10 deletions(-)
diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c
index bb5ec425dfe0..707ecf47a5fd 100644
--- a/kernel/time/hrtimer.c
+++ b/kernel/time/hrtimer.c
@@ -1742,15 +1742,19 @@ schedule_hrtimeout_range_clock(ktime_t *expires, u64 delta,
* You can set the task state as follows -
*
* %TASK_UNINTERRUPTIBLE - at least @timeout time is guaranteed to
- * pass before the routine returns.
+ * pass before the routine returns unless the current task is explicitly
+ * woken up, (e.g. by wake_up_process())".
*
* %TASK_INTERRUPTIBLE - the routine may return early if a signal is
- * delivered to the current task.
+ * delivered to the current task or the current task is explicitly woken
+ * up.
*
* The current task state is guaranteed to be TASK_RUNNING when this
* routine returns.
*
- * Returns 0 when the timer has expired otherwise -EINTR
+ * Returns 0 when the timer has expired otherwise -EINTR. Note that
+ * -EINTR can still be returned even if the task state is
+ * TASK_UNINTERRUPTIBLE if the current task is explicitly woken up.
*/
int __sched schedule_hrtimeout_range(ktime_t *expires, u64 delta,
const enum hrtimer_mode mode)
@@ -1772,15 +1776,19 @@ EXPORT_SYMBOL_GPL(schedule_hrtimeout_range);
* You can set the task state as follows -
*
* %TASK_UNINTERRUPTIBLE - at least @timeout time is guaranteed to
- * pass before the routine returns.
+ * pass before the routine returns unless the current task is explicitly
+ * woken up, (e.g. by wake_up_process())".
*
* %TASK_INTERRUPTIBLE - the routine may return early if a signal is
- * delivered to the current task.
+ * delivered to the current task or the current task is explicitly woken
+ * up.
*
* The current task state is guaranteed to be TASK_RUNNING when this
* routine returns.
*
- * Returns 0 when the timer has expired otherwise -EINTR
+ * Returns 0 when the timer has expired otherwise -EINTR. Note that
+ * -EINTR can still be returned even if the task state is
+ * TASK_UNINTERRUPTIBLE if the current task is explicitly woken up.
*/
int __sched schedule_hrtimeout(ktime_t *expires,
const enum hrtimer_mode mode)
diff --git a/kernel/time/timer.c b/kernel/time/timer.c
index 219439efd56a..b2ca2a6bc4d2 100644
--- a/kernel/time/timer.c
+++ b/kernel/time/timer.c
@@ -1691,11 +1691,12 @@ static void process_timeout(unsigned long __data)
* You can set the task state as follows -
*
* %TASK_UNINTERRUPTIBLE - at least @timeout jiffies are guaranteed to
- * pass before the routine returns. The routine will return 0
+ * pass before the routine returns unless the current task is explicitly
+ * woken up, (e.g. by wake_up_process())".
*
* %TASK_INTERRUPTIBLE - the routine may return early if a signal is
- * delivered to the current task. In this case the remaining time
- * in jiffies will be returned, or 0 if the timer expired in time
+ * delivered to the current task or the current task is explicitly woken
+ * up.
*
* The current task state is guaranteed to be TASK_RUNNING when this
* routine returns.
@@ -1704,7 +1705,9 @@ static void process_timeout(unsigned long __data)
* the CPU away without a bound on the timeout. In this case the return
* value will be %MAX_SCHEDULE_TIMEOUT.
*
- * In all cases the return value is guaranteed to be non-negative.
+ * Returns 0 when the timer has expired otherwise the remaining time in
+ * jiffies will be returned. In all cases the return value is guaranteed
+ * to be non-negative.
*/
signed long __sched schedule_timeout(signed long timeout)
{
--
2.8.0.rc3.226.g39d4020
next prev parent reply other threads:[~2016-10-20 20:34 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-20 20:34 [PATCH v3 1/2] timers: Fix usleep_range() in the context of wake_up_process() Douglas Anderson
2016-10-20 20:34 ` Douglas Anderson [this message]
[not found] ` <1476995664-15668-2-git-send-email-dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2016-10-20 21:06 ` [PATCH v3 2/2] timers: Fix documentation for schedule_timeout() and similar Thomas Gleixner
2016-10-20 21:25 ` [PATCH v3 1/2] timers: Fix usleep_range() in the context of wake_up_process() Thomas Gleixner
2016-10-20 23:36 ` Doug Anderson
[not found] ` <CAD=FV=U0sURwAmsiLg0q+=e8o7NfJLWEUAu4zeY1so83cDqXhA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-10-21 7:08 ` Thomas Gleixner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1476995664-15668-2-git-send-email-dianders@chromium.org \
--to=dianders@chromium.org \
--cc=andi@lisas.de \
--cc=briannorris@chromium.org \
--cc=broonie@kernel.org \
--cc=djkurtz@chromium.org \
--cc=heiko@sntech.de \
--cc=huangtao@rock-chips.com \
--cc=john.stultz@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=linux@roeck-us.net \
--cc=tglx@linutronix.de \
--cc=tony.xie@rock-chips.com \
--cc=tskd08@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox