Messages in this thread | | | Date | Tue, 15 Mar 2016 10:49:21 +0100 | From | Ingo Molnar <> | Subject | Re: [GIT PULL] NOHZ updates for v4.6 |
| |
* Peter Zijlstra <peterz@infradead.org> wrote:
> On Mon, Mar 14, 2016 at 07:44:14PM -0700, Linus Torvalds wrote: > > On Mon, Mar 14, 2016 at 5:32 AM, Ingo Molnar <mingo@kernel.org> wrote: > > > +/** > > > + * fetch_or - perform *ptr |= mask and return old value of *ptr > > > + * @ptr: pointer to value > > > + * @mask: mask to OR on the value > > > + * > > > + * cmpxchg based fetch_or, macro so it works for different integer types > > > + */ > > > +#ifndef fetch_or > > > +#define fetch_or(ptr, mask) \ > > > +({ typeof(*(ptr)) __old, __val = *(ptr); \ > > > + for (;;) { \ > > > + __old = cmpxchg((ptr), __val, __val | (mask)); \ > > > + if (__old == __val) \ > > > + break; \ > > > + __val = __old; \ > > > + } \ > > > + __old; \ > > > +}) > > > +#endif > > > > This is garbage. > > > > This macro re-uses the "mask" argument potentially many many times, so > > semantically it's very dubious. > > So the below cures that; but do we want to maybe pull this back into > sched.c and expose a version that operates on a fixed type instead?
So there are other problems with the macro - see the patch I just sent that tries to fix all of them.
> Although with xchg() and cmpxchg() we've already set a precedence for > multi-width operators.
Yes, and if we name the new API 'xchg_or()' as I did it in my patch then it all nicely fits into the existing scheme.
Thanks,
Ingo
| |