From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934416AbcCOInX (ORCPT ); Tue, 15 Mar 2016 04:43:23 -0400 Received: from [198.137.202.9] ([198.137.202.9]:46441 "EHLO bombadil.infradead.org" rhost-flags-FAIL-FAIL-OK-OK) by vger.kernel.org with ESMTP id S932842AbcCOInO (ORCPT ); Tue, 15 Mar 2016 04:43:14 -0400 Date: Tue, 15 Mar 2016 09:42:20 +0100 From: Peter Zijlstra To: Linus Torvalds Cc: Ingo Molnar , Linux Kernel Mailing List , =?iso-8859-1?Q?Fr=E9d=E9ric?= Weisbecker , Thomas Gleixner , Andrew Morton Subject: Re: [GIT PULL] NOHZ updates for v4.6 Message-ID: <20160315084220.GR6344@twins.programming.kicks-ass.net> References: <20160314123200.GA15971@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2012-12-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Mar 14, 2016 at 07:44:14PM -0700, Linus Torvalds wrote: > On Mon, Mar 14, 2016 at 5:32 AM, Ingo Molnar 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? Although with xchg() and cmpxchg() we've already set a precedence for multi-width operators. The whole thread_info::flags thing is rather 'special' and I'm not sure we even want to go clean that up, I can see why 64bit arch would very much want a 64bit flags word etc. And TIF_flags are very arch specific. --- include/linux/atomic.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/linux/atomic.h b/include/linux/atomic.h index 6c502cb13c95..114caf672b11 100644 --- a/include/linux/atomic.h +++ b/include/linux/atomic.h @@ -558,8 +558,9 @@ static inline int atomic_dec_if_positive(atomic_t *v) #ifndef fetch_or #define fetch_or(ptr, mask) \ ({ typeof(*(ptr)) __old, __val = *(ptr); \ + typeof(mask) __mask = (mask); \ for (;;) { \ - __old = cmpxchg((ptr), __val, __val | (mask)); \ + __old = cmpxchg((ptr), __val, __val | __mask); \ if (__old == __val) \ break; \ __val = __old; \