linux-kernel - Re: Pinning down a blocked task to extract diagnostics

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200306115611.3d265296@gandalf.local.home>
Date:   Fri, 6 Mar 2020 11:56:11 -0500
From:   Steven Rostedt <rostedt@...dmis.org>
To:     "Paul E. McKenney" <paulmck@...nel.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 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.

> + */
> +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.

-- Steve

> +		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.
>   *