[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <53b33d48c81b35bb3567ab19308f309b0f320e6a.camel@amazon.com>
Date: Wed, 6 Nov 2024 17:09:45 +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 2/5] arm64: add __READ_ONCE_EX()
On Wed, 2024-11-06 at 11:43 +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:38PM -0600, Haris Okanovic wrote:
> > Perform an exclusive load, which atomically loads a word and arms the
> > exclusive monitor to enable wfet()/wfe() accelerated polling.
> >
> > https://developer.arm.com/documentation/dht0008/a/arm-synchronization-primitives/exclusive-accesses/exclusive-monitors
> >
> > Signed-off-by: Haris Okanovic <harisokn@...zon.com>
> > ---
> > arch/arm64/include/asm/readex.h | 46 +++++++++++++++++++++++++++++++++
> > 1 file changed, 46 insertions(+)
> > create mode 100644 arch/arm64/include/asm/readex.h
> >
> > diff --git a/arch/arm64/include/asm/readex.h b/arch/arm64/include/asm/readex.h
> > new file mode 100644
> > index 000000000000..51963c3107e1
> > --- /dev/null
> > +++ b/arch/arm64/include/asm/readex.h
> > @@ -0,0 +1,46 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Based on arch/arm64/include/asm/rwonce.h
> > + *
> > + * Copyright (C) 2020 Google LLC.
> > + * Copyright (C) 2024 Amazon.com, Inc. or its affiliates.
> > + */
> > +
> > +#ifndef __ASM_READEX_H
> > +#define __ASM_READEX_H
> > +
> > +#define __LOAD_EX(sfx, regs...) "ldaxr" #sfx "\t" #regs
> > +
> > +#define __READ_ONCE_EX(x) \
> > +({ \
> > + typeof(&(x)) __x = &(x); \
> > + int atomic = 1; \
> > + union { __unqual_scalar_typeof(*__x) __val; char __c[1]; } __u; \
> > + switch (sizeof(x)) { \
> > + case 1: \
> > + asm volatile(__LOAD_EX(b, %w0, %1) \
> > + : "=r" (*(__u8 *)__u.__c) \
> > + : "Q" (*__x) : "memory"); \
> > + break; \
> > + case 2: \
> > + asm volatile(__LOAD_EX(h, %w0, %1) \
> > + : "=r" (*(__u16 *)__u.__c) \
> > + : "Q" (*__x) : "memory"); \
> > + break; \
> > + case 4: \
> > + asm volatile(__LOAD_EX(, %w0, %1) \
> > + : "=r" (*(__u32 *)__u.__c) \
> > + : "Q" (*__x) : "memory"); \
> > + break; \
> > + case 8: \
> > + asm volatile(__LOAD_EX(, %0, %1) \
> > + : "=r" (*(__u64 *)__u.__c) \
> > + : "Q" (*__x) : "memory"); \
> > + break; \
> > + default: \
> > + atomic = 0; \
> > + } \
> > + atomic ? (typeof(*__x))__u.__val : (*(volatile typeof(__x))__x);\
> > +})
>
> I think this is a bad idea. Load-exclusive needs to be used very carefully,
> preferably when you're able to see exactly what instructions it's
> interacting with. By making this into a macro, we're at the mercy of the
> compiler and we give the wrong impression that you could e.g. build atomic
> critical sections out of this macro.
>
> So I'm fairly strongly against this interface.
Fair point. I'll post an alternate delay() implementation in asm. It's
a simple routine.
>
> Will
Powered by blists - more mailing lists