Re: [patch] cpufreq: mark cpufreq_tsc() as core_initcall_sync

["Paul E. McKenney" <paulmck@linux.vnet.ibm.com>]

On Mon, Nov 20, 2006 at 03:01:59PM -0500, Alan Stern wrote:
> On Mon, 20 Nov 2006, Oleg Nesterov wrote:
> 
> > On 11/20, Alan Stern wrote:
> > >
> > > @@ -158,6 +199,11 @@ void synchronize_srcu(struct srcu_struct
> > >
> > > [... snip ...]
> > >
> > > +#ifdef	SMP__STORE_MB_LOAD_WORKS	/* The fast path */
> > > +	if (srcu_readers_active_idx(sp, idx) == 0)
> > > +		goto done;
> > > +#endif
> >
> > I guess this is connected to another message from you,
> 
> Yes.
> 
> > > But of course it _is_ needed for the fastpath to work.  In fact, it might
> > > not be good enough, depending on the architecture.  Here's what the
> > > fastpath ends up looking like (using c[idx] is essentially the same as
> > > using hardluckref):
> > >
> > >         WRITER                          READER
> > >         ------                          ------
> > >         dataptr = &(new data)           atomic_inc(&hardluckref)
> > >         mb                              mb
> > >         while (hardluckref > 0) ;       access *dataptr
> > >
> > > Notice the pattern: Each CPU does store-mb-load.  It is known that on
> > > some architectures each CPU can end up loading the old value (the value
> > > from before the other CPU's store).  This would mean the writer would see
> > > hardluckref == 0 right away and the reader would see the old dataptr.
> >
> > So, if we have global A == B == 0,
> >
> > 	CPU_0		CPU_1
> >
> > 	A = 1;		B = 2;
> > 	mb();		mb();
> > 	b = B;		a = A;
> >
> > It could happen that a == b == 0, yes?
> 
> Exactly.
> 
> > Isn't this contradicts with definition
> > of mb?
> 
> One might think so, at first.  But if you do a careful search, you'll find
> that there _is_ no definition of mb!  People state in vague terms what
> it's supposed to do, but they are almost never specific enough to tell
> whether the example above should work.

Yep -- mb() is currently defined only for specific CPUs.  :-/

Some Linux kernel code has been written by considering each SMP-capable
CPU in turn, but that does not scale with increasing numbers of SMP-capable
CPUs.

> > By definition, when CPU_0 issues 'b = B', 'A = 1' should be visible to other
> > CPUs, yes?
> 
> No.  Memory barriers don't guarantee that any particular store will become
> visible to other CPUs at any particular time.  They guarantee only that a
> certain sequence of stores will become visible in a particular order
> (provided the other CPUs also use the correct memory barriers).
> 
> >  Now, b == 0 means that CPU_1 did not read 'a = A' yet, otherwise
> > 'B = 2' should be visible to all CPUs (by definition again).
> >
> > Could you please clarify this?
> 
> Here's an example showing how the code can fail.  (Paul can correct me if
> I get this wrong.)

Looks good to me!

							Thanx, Paul