|
Message-ID: <20170206081237.GG6515@twins.programming.kicks-ass.net> Date: Mon, 6 Feb 2017 09:12:37 +0100 From: Peter Zijlstra <peterz@...radead.org> To: Boqun Feng <boqun.feng@...il.com> Cc: elena.reshetova@...el.com, gregkh@...uxfoundation.org, keescook@...omium.org, arnd@...db.de, tglx@...utronix.de, mingo@...nel.org, h.peter.anvin@...el.com, will.deacon@....com, dwindsor@...il.com, dhowells@...hat.com, linux-kernel@...r.kernel.org, kernel-hardening@...ts.openwall.com Subject: Re: [PATCH 4/5] atomic: Introduce atomic_try_cmpxchg() On Mon, Feb 06, 2017 at 12:24:28PM +0800, Boqun Feng wrote: > On Fri, Feb 03, 2017 at 02:26:02PM +0100, Peter Zijlstra wrote: > > > > for (;;) { > > new = val $op $imm; > > if (try_cmpxchg(ptr, &val, new)) > > break; > > } > > > > while also generating better code (GCC6 and onwards). > > > > But switching to try_cmpxchg() will make @val a memory location, which > could not be put in a register. And this will generate unnecessary > memory accesses on archs having enough registers(PPC, e.g.). GCC was perfectly capable of making @val a register in the code I was looking at. > > +#ifndef atomic_try_cmpxchg > > + > > +#define __atomic_try_cmpxchg(type, _p, _po, _n) \ > > +({ \ > > + typeof(_po) __po = (_po); \ > > + typeof(*(_po)) __o = *__po; \ > > + bool success = (atomic_cmpxchg##type((_p), __o, (_n)) == __o); \ > > + *__po = __o; \ > > Besides, is this part correct? atomic_cmpxchg_*() wouldn't change the > value of __o, so *__po wouldn't be changed.. IOW, in case of failure, > *ptr wouldn't be updated to a new value. > > Maybe this should be: > > bool success; > *__po = atomic_cmpxchg##type((_p), __o, (_n)); > sucess = (*__po == _o); > > , right? Yes, botched that. Don't think I even compiled it to be honest :/
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.