Messages in this thread | | | Date | Fri, 24 May 2019 11:53:40 +0100 | From | Mark Rutland <> | Subject | Re: [PATCH 2/2] compiler: Prevent evaluation of WRITE_ONCE() |
| |
This would be better titled as:
compiler: don't return a value from WRITE_ONCE()
... since we do want the WRITE_ONCE() itself to be evaluated.
On Fri, May 24, 2019 at 12:35:36PM +0200, Andrea Parri wrote: > Now that there's no single use of the value of WRITE_ONCE(), change > the implementation to eliminate it.
I hope that's the case, but it's possible that some macros might be relying on this, so it's probably worth waiting to see if the kbuild test robot screams.
Otherwise, I agree that WRITE_ONCE() returning a value is surprising, and unnecessary. IIRC you said that trying to suport that in other implementations was painful, so aligning on a non-returning version sounds reasonable to me.
> > Suggested-by: Mark Rutland <mark.rutland@arm.com> > Signed-off-by: Andrea Parri <andrea.parri@amarulasolutions.com> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: Jorgen Hansen <jhansen@vmware.com> > Cc: Peter Zijlstra <peterz@infradead.org> > Cc: Will Deacon <will.deacon@arm.com> > Cc: Mark Rutland <mark.rutland@arm.com> > Cc: "Paul E. McKenney" <paulmck@linux.ibm.com> > --- > include/linux/compiler.h | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/include/linux/compiler.h b/include/linux/compiler.h > index 8aaf7cd026b06..4024c809a6c63 100644 > --- a/include/linux/compiler.h > +++ b/include/linux/compiler.h > @@ -277,12 +277,11 @@ unsigned long read_word_at_a_time(const void *addr) > } > > #define WRITE_ONCE(x, val) \ > -({ \ > +do { \ > union { typeof(x) __val; char __c[1]; } __u = \ > { .__val = (__force typeof(x)) (val) }; \ > __write_once_size(&(x), __u.__c, sizeof(x)); \ > - __u.__val; \ > -}) > +} while (0)
With the title fixed, and assuming that the kbuild test robot doesn't find uses we've missed:
Acked-by: Mark Rutland <mark.rutland@arm.com>
Thanks, Mark.
> > #endif /* __KERNEL__ */ > > -- > 2.7.4 >
| |