linux-kernel - Re: [RFC v2 7/9] x86/mm: Introduce Remote Action Request

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 for Android: free password hash cracker in your pocket

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

Message-ID: <6a3290319031cd68a383e416f53aa7549bac9407.camel@surriel.com>
Date: Tue, 20 May 2025 09:00:33 -0400
From: Rik van Riel <riel@...riel.com>
To: Nadav Amit <nadav.amit@...il.com>
Cc: Linux Kernel Mailing List <linux-kernel@...r.kernel.org>, "open
 list:MEMORY MANAGEMENT"	 <linux-mm@...ck.org>, the arch/x86 maintainers
 <x86@...nel.org>, 	kernel-team@...a.com, Dave Hansen
 <dave.hansen@...ux.intel.com>, luto@...nel.org, 	peterz@...radead.org,
 Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar	 <mingo@...hat.com>,
 Borislav Petkov <bp@...en8.de>, "H. Peter Anvin"	 <hpa@...or.com>, Yu-cheng
 Yu <yu-cheng.yu@...el.com>
Subject: Re: [RFC v2 7/9] x86/mm: Introduce Remote Action Request

On Tue, 2025-05-20 at 14:29 +0300, Nadav Amit wrote:
> Not a full review, but..
> 
> > On 20 May 2025, at 4:02, Rik van Riel <riel@...riel.com> wrote:
> > 
> > +/*
> > + * This is a modified version of smp_call_function_many() of
> > kernel/smp.c,
> 
> The updated function names is smp_call_function_many_cond() and it is
> not aligned with smp_call_rar_many. I think the new version is
> (suprisingly)
> better, so it’d be beneficial to bring smp_call_rar_many() to be like
> the
> updated one in smp.c.
> 
Agreed, it will be good to conditionally not send 
the RAR vector to some CPUs, especially ones that
are in deeper idle states.

That means structuring the code more like
smp_call_function_many_cond()

> > +	/*
> > +	 * Can deadlock when called with interrupts disabled.
> > +	 * We allow cpu's that are not yet online though, as no
> > one else can
> > +	 * send smp call function interrupt to this cpu and as
> > such deadlocks
> > +	 * can't happen.
> > +	 */
> > +	WARN_ON_ONCE(cpu_online(this_cpu) && irqs_disabled()
> > +		     && !oops_in_progress &&
> > !early_boot_irqs_disabled);
> 
> I thought you agreed to change it to make it use lockdep instead (so
> it will
> be compiled out without LOCKDEP), like done in
> smp_call_function_many_cond()
> 
I thought I had made that change in my tree.

I guess I lost it in a rebase :(

> > +
> > +	/* Try to fastpath.  So, what's a CPU they want?  Ignoring
> > this one. */
> > +	cpu = cpumask_first_and(mask, cpu_online_mask);
> > +	if (cpu == this_cpu)
> > +		cpu = cpumask_next_and(cpu, mask,
> > cpu_online_mask);
> > +
> 
> Putting aside the rest of the code, I see you don’t call
> should_flush_tlb().
> I think it is worth mentioning in commit log or comment the rationale
> behind
> it (and maybe benchmarks to justify it).
> 
> 
The long term plan here is to simply have the originating
CPU included in the cpumask, and have it send a RAR
request to itself.

That way all the CPUs can invalidate their entries in
parallel, without any extra code.

-- 
All Rights Reversed.