Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754702AbYKJXXo (ORCPT ); Mon, 10 Nov 2008 18:23:44 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752593AbYKJXXe (ORCPT ); Mon, 10 Nov 2008 18:23:34 -0500 Received: from smtp1.linux-foundation.org ([140.211.169.13]:41716 "EHLO smtp1.linux-foundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752126AbYKJXXd (ORCPT ); Mon, 10 Nov 2008 18:23:33 -0500 Date: Mon, 10 Nov 2008 15:22:21 -0800 From: Andrew Morton To: Nicolas Pitre Cc: mathieu.desnoyers@polymtl.ca, torvalds@linux-foundation.org, rmk+lkml@arm.linux.org.uk, dhowells@redhat.com, mingo@elte.hu, a.p.zijlstra@chello.nl, linux-kernel@vger.kernel.org, ralf@linux-mips.org, benh@kernel.crashing.org, paulus@samba.org, davem@davemloft.net, mingo@redhat.com, tglx@linutronix.de, rostedt@goodmis.org, linux-arch@vger.kernel.org Subject: Re: [PATCH v2] clarify usage expectations for cnt32_to_63() Message-Id: <20081110152221.64948d23.akpm@linux-foundation.org> In-Reply-To: References: <9405.1226101315@redhat.com> <20081108001555.GE18378@flint.arm.linux.org.uk> <20081109022549.GA18508@Krystal> <20081109064855.GA23782@Krystal> <20081109162250.GB10181@Krystal> <20081109204256.89ab7925.akpm@linux-foundation.org> <20081110135850.0d620f3c.akpm@linux-foundation.org> X-Mailer: Sylpheed version 2.2.4 (GTK+ 2.8.20; i486-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2602 Lines: 68 On Mon, 10 Nov 2008 18:15:32 -0500 (EST) Nicolas Pitre wrote: > On Mon, 10 Nov 2008, Andrew Morton wrote: > > > On Mon, 10 Nov 2008 16:34:54 -0500 (EST) > > Nicolas Pitre wrote: > > > > > > It is far better to make the management of the state explicit and at > > > > the control of the caller. Get the caller to allocate the state and > > > > pass its address into this function. Simple, clear, explicit and > > > > robust. > > > > > > Sigh... What about this compromize then? > > > > > > diff --git a/include/linux/cnt32_to_63.h b/include/linux/cnt32_to_63.h > > > index 7605fdd..74ce767 100644 > > > --- a/include/linux/cnt32_to_63.h > > > +++ b/include/linux/cnt32_to_63.h > > > @@ -32,8 +32,9 @@ union cnt32_to_63 { > > > > > > > > > /** > > > - * cnt32_to_63 - Expand a 32-bit counter to a 63-bit counter > > > + * __cnt32_to_63 - Expand a 32-bit counter to a 63-bit counter > > > * @cnt_lo: The low part of the counter > > > + * @cnt_hi_p: Pointer to storage for the extended part of the counter > > > * > > > * Many hardware clock counters are only 32 bits wide and therefore have > > > * a relatively short period making wrap-arounds rather frequent. This > > > @@ -75,16 +76,31 @@ union cnt32_to_63 { > > > * clear-bit instruction. Otherwise caller must remember to clear the top > > > * bit explicitly. > > > */ > > > -#define cnt32_to_63(cnt_lo) \ > > > +#define __cnt32_to_63(cnt_lo, cnt_hi_p) \ > > > ({ \ > > > - static u32 __m_cnt_hi; \ > > > union cnt32_to_63 __x; \ > > > - __x.hi = __m_cnt_hi; \ > > > + __x.hi = *(cnt_hi_p); \ > > > smp_rmb(); \ > > > __x.lo = (cnt_lo); \ > > > if (unlikely((s32)(__x.hi ^ __x.lo) < 0)) \ > > > - __m_cnt_hi = __x.hi = (__x.hi ^ 0x80000000) + (__x.hi >> 31); \ > > > + *(cnt_hi_p) = __x.hi = (__x.hi ^ 0x80000000) + (__x.hi >> 31); \ > > > __x.val; \ > > > }) > > > > This references its second argument twice, which can cause correctness > > or efficiency problems. > > > > There is no reason that this had to be implemented in cpp. > > Implementing it in C will fix the above problem. > > No, it won't, for correctness and efficiency reasons. > > And I've explained why already. I'd be very surprised if you've really found a case where a macro is faster than an inlined function. I don't think that has happened before. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/