RE: [RFC PATCH 28/56] stop_machine: Add stop_machine_nmi()

Kaplan, David
2026-01-09T22:19:21Z

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Chang S. Bae <chang.seok.bae@intel.com>
> Sent: Friday, January 9, 2026 4:16 PM
> To: Kaplan, David <David.Kaplan@amd.com>; Thomas Gleixner
> <tglx@linutronix.de>; Borislav Petkov <bp@alien8.de>; Peter Zijlstra
> <peterz@infradead.org>; Josh Poimboeuf <jpoimboe@kernel.org>; Pawan
> Gupta <pawan.kumar.gupta@linux.intel.com>; Ingo Molnar
> <mingo@redhat.com>; Dave Hansen <dave.hansen@linux.intel.com>;
> x86@kernel.org; H . Peter Anvin <hpa@zytor.com>
> Cc: Alexander Graf <graf@amazon.com>; Boris Ostrovsky
> <boris.ostrovsky@oracle.com>; linux-kernel@vger.kernel.org
> Subject: Re: [RFC PATCH 28/56] stop_machine: Add stop_machine_nmi()
>
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
>
>
> On 10/13/2025 7:34 AM, David Kaplan wrote:
> >
> > +/**
> > + * stop_machine_nmi: freeze the machine and run this function in NMI
> context
> > + * @fn: the function to run
> > + * @data: the data ptr for the @fn()
> > + * @cpus: the cpus to run the @fn() on (NULL = any online cpu)
> > + *
> > + * Like stop_machine() but runs the function in NMI context to avoid any
> risk of
> > + * interruption due to NMIs.
> > + *
> > + * Protects against CPU hotplug.
> > + */
> > +int stop_machine_nmi(cpu_stop_fn_t fn, void *data, const struct cpumask
> *cpus);
> > +
> > +/**
> > + * stop_machine_cpuslocked_nmi: freeze and run this function in NMI
> context
> > + * @fn: the function to run
> > + * @data: the data ptr for the @fn()
> > + * @cpus: the cpus to run the @fn() on (NULL = any online cpu)
> > + *
> > + * Same as above. Must be called from within a cpus_read_lock()
> protected
> > + * region. Avoids nested calls to cpus_read_lock().
> > + */
> > +int stop_machine_cpuslocked_nmi(cpu_stop_fn_t fn, void *data, const
> struct cpumask *cpus);
>
> <snip>
>
> > +int stop_machine_cpuslocked_nmi(cpu_stop_fn_t fn, void *data,
> > + const struct cpumask *cpus)
> > +{
> > + return __stop_machine_cpuslocked(fn, data, cpus, true);
> > +}
> > +
>
> It looks like this is readily missing the static key switching which is
> handled below. I think the body could be something like:
> ...
> static_branch_enable_cpuslocked(&stop_machine_nmi_handler_enable);
> ret = __stop_machine_cpuslocked(fn, data, cpus, true);
> static_branch_disable_cpuslocked(&stop_machine_nmi_handler_enable);
> ...
>
> > +int stop_machine_nmi(cpu_stop_fn_t fn, void *data, const struct cpumask
> *cpus)
> > +{
> > + int ret;
> > +
> > + cpus_read_lock();
> > +
> static_branch_enable_cpuslocked(&stop_machine_nmi_handler_enable);
> > + ret = stop_machine_cpuslocked_nmi(fn, data, cpus);
> > +
> static_branch_disable_cpuslocked(&stop_machine_nmi_handler_enable);
> > + cpus_read_unlock();
> > + return ret;
> > +}
>
> With that, here __stop_machine_cpuslocked() can be invoked instead.
>

Ah, yeah I agree that doing the static key switching is better done in stop_machine_cpuslocked_nmi instead. That's a good point.

Thanks --David Kaplan