linux-kernel - Re: [PATCH stop-machine] Fix rcu_momentary_eqs() call in multi_cpu

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] [day] [month] [year] [list]

Message-ID: <87wmeuanti.ffs@tglx>
Date: Thu, 16 Jan 2025 14:30:17 +0100
From: Thomas Gleixner <tglx@...utronix.de>
To: paulmck@...nel.org, quic_mojha@...cinc.com, vschneid@...hat.com,
 neeraj.upadhyay@...nel.org, frederic@...nel.org
Cc: linux-kernel@...r.kernel.org, kernel-team@...a.com
Subject: Re: [PATCH stop-machine] Fix rcu_momentary_eqs() call in
 multi_cpu_stop()


$Subject: [PATCH stop-machine] Fix rcu_momentary_eqs() call in multi_cpu_stop()

[PATCH prefix] Shortlog - That's not a valid subject line.

[PATCH] prefix: Shortlog - Is what's expected, no?

On Thu, Dec 12 2024 at 11:00, Paul E. McKenney wrote:

As this patch is from Mukesh, this want's a

   From: Mukesh ....

line right here.

> The multi_cpu_stop() contains a loop that can initially be executed
> with

s/The//

> interrupts enabled (in the MULTI_STOP_NONE and MULTI_STOP_PREPARE states).

> Interrupts are guaranteed to be once the MULTI_STOP_DISABLE_IRQ state
> is reached.

That's not a parseable sentence.

> Unfortunately, the rcu_momentary_eqs() function that is currently
> invoked on each pass through this loop requires that interrupts be
> disabled.

What's unfortunate about that? It's a face rcu_momentary_eqs() requires
to be invoked with interrupts disabled.

> This commit therefore moves this call to rcu_momentary_eqs() to the body

'This commit' is equally pointless as 'This patch'.

git grep 'This patch' Documentation/process

> of the "else if (curstate > MULTI_STOP_PREPARE)" portion of the loop, thus
> guaranteeing that interrupts will be disabled on each call, as
> required.

Something like this perhaps:

  Move the invocation of rcu_momentary_eqs() into the interrupt disabled
  section of the loop.

Hmm?

> Kudos to 朱恺乾 (Kaiqian) for noting that this had not made it to mainline.
>
> [ paulmck: Update from rcu_momentary_dyntick_idle() to rcu_momentary_eqs(). ]
>
> Link: https://lore.kernel.org/all/1712649736-27058-1-git-send-email-quic_mojha@quicinc.com/

Link below the SOBs please.

> Signed-off-by: Mukesh Ojha <quic_mojha@...cinc.com>
> Signed-off-by: Paul E. McKenney <paulmck@...nel.org>
>
> diff --git a/kernel/stop_machine.c b/kernel/stop_machine.c
> index da821ce258ea7..8896d844d738f 100644
> --- a/kernel/stop_machine.c
> +++ b/kernel/stop_machine.c
> @@ -250,8 +250,8 @@ static int multi_cpu_stop(void *data)
>  			 * be detected and reported on their side.
>  			 */
>  			touch_nmi_watchdog();
> +			rcu_momentary_eqs();

Can we please have a comment why this call is actually there and what it
does, similar to the one for touch_nmi_watchdog()?

Thanks,

        tglx