Re: [PATCH 1/2] MN10300: Move asm-arm/cnt32_to_63.h to include/linux/

[Nicolas Pitre <nico@cam.org>]

On Fri, 26 Sep 2008, Peter Zijlstra wrote:

> On Fri, 2008-09-26 at 08:03 -0400, Nicolas Pitre wrote:
> > On Fri, 26 Sep 2008, Peter Zijlstra wrote:
> > 
> > > On Wed, 2008-09-24 at 17:48 +0100, David Howells wrote:
> > > > Move asm-arm/cnt32_to_63.h to include/linux/ so that MN10300 can make use of it
> > > > too.
> > > > 
> > > > Signed-off-by: David Howells <dhowells@redhat.com>
> > > > ---
> > > > 
> > > >  arch/arm/mach-pxa/time.c       |    2 +
> > > >  arch/arm/mach-sa1100/generic.c |    2 +
> > > >  arch/arm/mach-versatile/core.c |    2 +
> > > >  include/linux/cnt32_to_63.h    |   80 ++++++++++++++++++++++++++++++++++++++++
> > > >  4 files changed, 83 insertions(+), 3 deletions(-)
> > > >  create mode 100644 include/linux/cnt32_to_63.h
> > > 
> > > Didn't you forget to remove the old one?
> > > 
> > > 
> > > > +#define cnt32_to_63(cnt_lo) \
> > > > +({ \
> > > > +	static volatile u32 __m_cnt_hi; \
> > > > +	union cnt32_to_63 __x; \
> > > > +	__x.hi = __m_cnt_hi; \
> > > > +	__x.lo = (cnt_lo); \
> > > > +	if (unlikely((s32)(__x.hi ^ __x.lo) < 0)) \
> > > > +		__m_cnt_hi = __x.hi = (__x.hi ^ 0x80000000) + (__x.hi >> 31); \
> > > > +	__x.val; \
> > > > +})
> > > > +
> > > > +#endif
> > > 
> > > That code is way to smart :-)
> > > 
> > > Better make sure that non of its users are SMP capable though.
> > 
> > Why would that matter?
> 
> It has a local static variable and no synchronization.

So?  The whole point of this algorithm is to avoid all kind of locking 
and require absolutely no synchronization as it relies solely on the 
implicit synchronization from the top bit of both values.  You would get 
cache bouncing but the result will still be correct.

> Also, if the counters differ per cpu the value of __m_cnt_hi ought to 
> be different per cpu.

True.


Nicolas