| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 6 Mar 2020 13:01:31 -0800
From: "Paul E. McKenney" <paulmck@...nel.org>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: Peter Zijlstra <peterz@...radead.org>, mingo@...hat.com,
juri.lelli@...hat.com, vincent.guittot@...aro.org,
dietmar.eggemann@....com, bsegall@...gle.com, mgorman@...e.de,
linux-kernel@...r.kernel.org
Subject: Re: Pinning down a blocked task to extract diagnostics
On Fri, Mar 06, 2020 at 11:56:11AM -0500, Steven Rostedt wrote:
> On Thu, 5 Mar 2020 17:40:27 -0800
> "Paul E. McKenney" <paulmck@...nel.org> wrote:
>
> > commit e2821ae6c6a6adaabc89ccd9babf4375a78e0626
> > Author: Paul E. McKenney <paulmck@...nel.org>
> > Date: Thu Mar 5 16:53:58 2020 -0800
> >
> > sched/core: Add functions to prevent sleepers from awakening
> >
> > In some cases, it is necessary to examine a consistent version of a
> > sleeping process's state, in other words, it is necessary to keep
> > that process in sleeping state. This commit therefore provides a
> > try_to_keep_sleeping() function that acquires ->pi_lock to prevent
> > wakeups from proceeding, returning true if the function is still asleep,
> > and otherwise releasing ->pi_lock and returning false.
> >
> > This commit also provides an allow_awake() function (as suggested by
> > by Steven Rostedt) that reverses the effect of a successful call to
> > try_to_keep_sleeping(), allowing the process to once again be awakened.
> >
> > Signed-off-by: Paul E. McKenney <paulmck@...nel.org>
> > [ paulmck: Apply feedback from Peter Zijlstra and Steven Rostedt. ]
> > Cc: Ingo Molnar <mingo@...hat.com>
> > Cc: Peter Zijlstra <peterz@...radead.org>
> > Cc: Juri Lelli <juri.lelli@...hat.com>
> > Cc: Vincent Guittot <vincent.guittot@...aro.org>
> > Cc: Dietmar Eggemann <dietmar.eggemann@....com>
> > Cc: Steven Rostedt <rostedt@...dmis.org>
> > Cc: Ben Segall <bsegall@...gle.com>
> > Cc: Mel Gorman <mgorman@...e.de>
> >
> > diff --git a/include/linux/wait.h b/include/linux/wait.h
> > index 3283c8d..aefea4a 100644
> > --- a/include/linux/wait.h
> > +++ b/include/linux/wait.h
> > @@ -1148,4 +1148,7 @@ int autoremove_wake_function(struct wait_queue_entry *wq_entry, unsigned mode, i
> > (wait)->flags = 0; \
> > } while (0)
> >
> > +bool try_to_keep_sleeping(struct task_struct *p);
> > +void allow_awake(struct task_struct *p);
> > +
> > #endif /* _LINUX_WAIT_H */
> > diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> > index fc1dfc0..b665ff7 100644
> > --- a/kernel/sched/core.c
> > +++ b/kernel/sched/core.c
> > @@ -2654,6 +2654,48 @@ try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags)
> > }
> >
> > /**
> > + * try_to_keep_sleeping - Attempt to force task to remain off runqueues
> > + * @p: The process to remain asleep.
> > + *
> > + * Acquires the process's ->pi_lock and checks state. If the process
> > + * is still blocked, returns @true and leave ->pi_lock held, otherwise
> > + * releases ->pi_locked and returns @false.
>
> I would add a comment here that this is paired with allow_awake(). As well
> as a "Returns" statement.
>
> * Returns:
> * false if the task is awake, then no lock is taken.
> * true if the task is sleeping, and then task's pi_lock will be held.
> * allow_awake() must be used to release the pi_lock and let
> * task @p awake again.
Good point, added.
> > + */
> > +bool try_to_keep_sleeping(struct task_struct *p)
> > +{
> > + lockdep_assert_irqs_enabled();
> > + raw_spin_lock_irq(&p->pi_lock);
> > + switch (p->state) {
> > + case TASK_RUNNING:
> > + case TASK_WAKING:
> > + raw_spin_unlock_irq(&p->pi_lock);
> > + return false;
> > +
> > + default:
> > + smp_rmb(); /* See comments in try_to_wake_up(). */
>
> I remember Peter asking to add a comment in try_to_wake_up() stating that
> this is used, so that if that code is changed, this code may also need to
> be updated.
Adding a reference to try_to_keep_sleeping() in try_to_wake_up()'s
smp_rmb() comment, as shown below?
Thanx, Paul
------------------------------------------------------------------------
commit 57008d8adaa1e93acedea23e4858b9831e0dd075
Author: Paul E. McKenney <paulmck@...nel.org>
Date: Thu Mar 5 16:53:58 2020 -0800
sched/core: Add functions to prevent sleepers from awakening
In some cases, it is necessary to examine a consistent version of a
sleeping process's state, in other words, it is necessary to keep
that process in sleeping state. This commit therefore provides a
try_to_keep_sleeping() function that acquires ->pi_lock to prevent
wakeups from proceeding, returning true if the function is still asleep,
and otherwise releasing ->pi_lock and returning false.
This commit also provides an allow_awake() function (as suggested by
by Steven Rostedt) that reverses the effect of a successful call to
try_to_keep_sleeping(), allowing the process to once again be awakened.
Signed-off-by: Paul E. McKenney <paulmck@...nel.org>
[ paulmck: Apply feedback from Peter Zijlstra and Steven Rostedt. ]
Cc: Ingo Molnar <mingo@...hat.com>
Cc: Peter Zijlstra <peterz@...radead.org>
Cc: Juri Lelli <juri.lelli@...hat.com>
Cc: Vincent Guittot <vincent.guittot@...aro.org>
Cc: Dietmar Eggemann <dietmar.eggemann@....com>
Cc: Steven Rostedt <rostedt@...dmis.org>
Cc: Ben Segall <bsegall@...gle.com>
Cc: Mel Gorman <mgorman@...e.de>
diff --git a/include/linux/wait.h b/include/linux/wait.h
index 3283c8d..aefea4a 100644
--- a/include/linux/wait.h
+++ b/include/linux/wait.h
@@ -1148,4 +1148,7 @@ int autoremove_wake_function(struct wait_queue_entry *wq_entry, unsigned mode, i
(wait)->flags = 0; \
} while (0)
+bool try_to_keep_sleeping(struct task_struct *p);
+void allow_awake(struct task_struct *p);
+
#endif /* _LINUX_WAIT_H */
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index fc1dfc0..a935a3d 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2580,6 +2580,8 @@ try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags)
*
* Pairs with the LOCK+smp_mb__after_spinlock() on rq->lock in
* __schedule(). See the comment for smp_mb__after_spinlock().
+ *
+ * A similar smp_rmb() lives in try_to_keep_sleeping().
*/
smp_rmb();
if (p->on_rq && ttwu_remote(p, wake_flags))
@@ -2654,6 +2656,54 @@ try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags)
}
/**
+ * try_to_keep_sleeping - Attempt to force task to remain off runqueues
+ * @p: The process to remain asleep.
+ *
+ * Acquires the process's ->pi_lock and checks state. If the process
+ * is still blocked, returns @true and leave ->pi_lock held, otherwise
+ * releases ->pi_lock and returns @false.
+ *
+ * Returns:
+ * @false if the task is awake, in which case no lock is held.
+ * @true if the task is sleeping, in which case the process's
+ * ->pi_lock will be held. Use allow_awake() to release
+ * this lock and thus allow process @p to awaken.
+ */
+bool try_to_keep_sleeping(struct task_struct *p)
+{
+ lockdep_assert_irqs_enabled();
+ raw_spin_lock_irq(&p->pi_lock);
+ switch (p->state) {
+ case TASK_RUNNING:
+ case TASK_WAKING:
+ raw_spin_unlock_irq(&p->pi_lock);
+ return false;
+
+ default:
+ smp_rmb(); /* See comments in try_to_wake_up(). */
+ if (p->on_rq) {
+ raw_spin_unlock_irq(&p->pi_lock);
+ return false;
+ }
+ return true; /* Process is now stuck in blocked state. */
+ }
+ /* NOTREACHED */
+}
+
+/**
+ * allow_awake - Allow a kept-sleeping process to awaken
+ * @p: Process to be allowed to awaken.
+ *
+ * Given that @p was passed to an earlier call to try_to_keep_sleeping
+ * that returned @true, hence preventing @p from waking up, allow @p
+ * to once again be awakened.
+ */
+void allow_awake(struct task_struct *p)
+{
+ raw_spin_unlock_irq(&p->pi_lock);
+}
+
+/**
* wake_up_process - Wake up a specific process
* @p: The process to be woken up.
*
Powered by blists - more mailing lists