linux-kernel - Re: [PATCH v4 02/13] locking/ww_mutex: Remove wakeups from under mutex::wait

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: <509d5ee4-45ec-1279-97da-a308ec7f51aa@arm.com>
Date:   Fri, 23 Jun 2023 14:21:16 +0200
From:   Dietmar Eggemann <dietmar.eggemann@....com>
To:     John Stultz <jstultz@...gle.com>,
        LKML <linux-kernel@...r.kernel.org>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Joel Fernandes <joelaf@...gle.com>,
        Qais Yousef <qyousef@...gle.com>,
        Ingo Molnar <mingo@...hat.com>,
        Juri Lelli <juri.lelli@...hat.com>,
        Vincent Guittot <vincent.guittot@...aro.org>,
        Valentin Schneider <vschneid@...hat.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Ben Segall <bsegall@...gle.com>,
        Zimuzo Ezeozue <zezeozue@...gle.com>,
        Youssef Esmat <youssefesmat@...gle.com>,
        Mel Gorman <mgorman@...e.de>,
        Daniel Bristot de Oliveira <bristot@...hat.com>,
        Will Deacon <will@...nel.org>,
        Waiman Long <longman@...hat.com>,
        Boqun Feng <boqun.feng@...il.com>,
        "Paul E . McKenney" <paulmck@...nel.org>, kernel-team@...roid.com,
        Connor O'Brien <connoro@...gle.com>
Subject: Re: [PATCH v4 02/13] locking/ww_mutex: Remove wakeups from under
 mutex::wait_lock

Hi John,

On 01/06/2023 07:58, John Stultz wrote:
> From: Peter Zijlstra <peterz@...radead.org>
> 
> In preparation to nest mutex::wait_lock under rq::lock we need to remove
> wakeups from under it.

[...]

> Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
> Signed-off-by: Connor O'Brien <connoro@...gle.com>
> Signed-off-by: John Stultz <jstultz@...gle.com>
> ---
> v2:
> * Move wake_q_init() as suggested by Waiman Long
> ---
>  include/linux/ww_mutex.h  |  3 +++
>  kernel/locking/mutex.c    |  8 ++++++++
>  kernel/locking/ww_mutex.h | 10 ++++++++--
>  3 files changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/ww_mutex.h b/include/linux/ww_mutex.h
> index bb763085479a..9335b2202017 100644
> --- a/include/linux/ww_mutex.h
> +++ b/include/linux/ww_mutex.h
> @@ -19,6 +19,7 @@
>  
>  #include <linux/mutex.h>
>  #include <linux/rtmutex.h>
> +#include <linux/sched/wake_q.h>
>  
>  #if defined(CONFIG_DEBUG_MUTEXES) || \
>     (defined(CONFIG_PREEMPT_RT) && defined(CONFIG_DEBUG_RT_MUTEXES))
> @@ -58,6 +59,7 @@ struct ww_acquire_ctx {
>  	unsigned int acquired;
>  	unsigned short wounded;
>  	unsigned short is_wait_die;
> +	struct wake_q_head wake_q;

you told me that there is already an issue in this patch even w/o PE
when running `insmod /lib/modules/test-ww_mutex.ko`.

The issue is related to Connor's version (1):

https://lkml.kernel.org/r/20221003214501.2050087-2-connoro@google.com

  struct ww_acquire_ctx {

	  struct wake_q_head wake_q;


  __mutex_lock_common()

    if (ww_ctx)
        ww_ctx_wake(ww_ctx)

          wake_up_q(&ww_ctx->wake_q);
          wake_q_init(&ww_ctx->wake_q);


Juri's version (2):

https://lkml.kernel.org/r/20181009092434.26221-3-juri.lelli@redhat.com

  __mutex_lock_common()

      DEFINE_WAKE_Q(wake_q) <-- !!!

      __ww_mutex_check_waiters(..., wake_q)

        __ww_mutex_die(..., wake_q)

          wake_q_add(wake_q, waiter->task)

      wake_up_q(&wake_q)


`insmod /lib/modules/test-ww_mutex.ko` runs fine with (2) but not with
(1) (both w/o the remaining PE patches).

So to test the PE issues we talked about already which come with `[PATCH
v4 09/13] sched: Add proxy execution` and CONFIG_PROXY_EXEC=y we need to
fix (1) or go back to (2).

I haven't found any clues why (2) was changed to (1) so far.

[...]