linux-kernel - Re: [PATCH 1/5] asm-generic: add smp_vcond_load

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: <1d070075c2e2fdf014d0c0ecb8a48f88bc7d229d.camel@amazon.com>
Date: Wed, 6 Nov 2024 17:18:25 +0000
From: "Okanovic, Haris" <harisokn@...zon.com>
To: "will@...nel.org" <will@...nel.org>
CC: "kvm@...r.kernel.org" <kvm@...r.kernel.org>, "rafael@...nel.org"
	<rafael@...nel.org>, "boris.ostrovsky@...cle.com"
	<boris.ostrovsky@...cle.com>, "sudeep.holla@....com" <sudeep.holla@....com>,
	"joao.m.martins@...cle.com" <joao.m.martins@...cle.com>,
	"ankur.a.arora@...cle.com" <ankur.a.arora@...cle.com>,
	"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
	"konrad.wilk@...cle.com" <konrad.wilk@...cle.com>, "wanpengli@...cent.com"
	<wanpengli@...cent.com>, "cl@...two.org" <cl@...two.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"catalin.marinas@....com" <catalin.marinas@....com>, "mingo@...hat.com"
	<mingo@...hat.com>, "pbonzini@...hat.com" <pbonzini@...hat.com>,
	"tglx@...utronix.de" <tglx@...utronix.de>, "misono.tomohiro@...itsu.com"
	<misono.tomohiro@...itsu.com>, "daniel.lezcano@...aro.org"
	<daniel.lezcano@...aro.org>, "arnd@...db.de" <arnd@...db.de>,
	"lenb@...nel.org" <lenb@...nel.org>, "mtosatti@...hat.com"
	<mtosatti@...hat.com>, "hpa@...or.com" <hpa@...or.com>,
	"peterz@...radead.org" <peterz@...radead.org>, "maobibo@...ngson.cn"
	<maobibo@...ngson.cn>, "vkuznets@...hat.com" <vkuznets@...hat.com>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "Okanovic, Haris"
	<harisokn@...zon.com>, "linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
	"bp@...en8.de" <bp@...en8.de>, "x86@...nel.org" <x86@...nel.org>,
	"mark.rutland@....com" <mark.rutland@....com>
Subject: Re: [PATCH 1/5] asm-generic: add smp_vcond_load_relaxed()

On Wed, 2024-11-06 at 11:39 +0000, Will Deacon wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
> 
> 
> 
> On Tue, Nov 05, 2024 at 12:30:37PM -0600, Haris Okanovic wrote:
> > Relaxed poll until desired mask/value is observed at the specified
> > address or timeout.
> > 
> > This macro is a specialization of the generic smp_cond_load_relaxed(),
> > which takes a simple mask/value condition (vcond) instead of an
> > arbitrary expression. It allows architectures to better specialize the
> > implementation, e.g. to enable wfe() polling of the address on arm.
> 
> This doesn't make sense to me. The existing smp_cond_load() functions
> already use wfe on arm64 and I don't see why we need a special helper
> just to do a mask.

We can't turn an arbitrary C expression into a wfe()/wfet() exit
condition, which is one of the inputs to the existing smp_cond_load().
This API is therefore more amenable to hardware acceleration.

> 
> > Signed-off-by: Haris Okanovic <harisokn@...zon.com>
> > ---
> >  include/asm-generic/barrier.h | 25 +++++++++++++++++++++++++
> >  1 file changed, 25 insertions(+)
> > 
> > diff --git a/include/asm-generic/barrier.h b/include/asm-generic/barrier.h
> > index d4f581c1e21d..112027eabbfc 100644
> > --- a/include/asm-generic/barrier.h
> > +++ b/include/asm-generic/barrier.h
> > @@ -256,6 +256,31 @@ do {                                                                     \
> >  })
> >  #endif
> > 
> > +/**
> > + * smp_vcond_load_relaxed() - (Spin) wait until an expected value at address
> > + * with no ordering guarantees. Spins until `(*addr & mask) == val` or
> > + * `nsecs` elapse, and returns the last observed `*addr` value.
> > + *
> > + * @nsecs: timeout in nanoseconds
> > + * @addr: pointer to an integer
> > + * @mask: a bit mask applied to read values
> > + * @val: Expected value with mask
> > + */
> > +#ifndef smp_vcond_load_relaxed
> 
> I know naming is hard, but "vcond" is especially terrible.
> Perhaps smp_cond_load_timeout()?

I agree, naming is hard! I was trying to differentiate it from
smp_cond_load() in some meaningful way - that one is an "expression"
condition this one is a "value" condition.

I'll think it over a bit more.

> 
> Will