Re: [PATCH 3/3] kref: Remove the memory barriers

[Oliver Neukum <oneukum@suse.de>]

Am Dienstag, 13. Dezember 2011, 00:14:19 schrieb Greg KH:
> > I guess I worried not about the increment, but the decrement.
> > Which makes me wonder what happens if you don't intend
> > to get the kref again, but need to make sure it is usually freed,
> > like:
> > 
> > CPU A                                                         CPU B
> > 
> > kref_get(p)
> > start_io(p)
> >                                                                       [interrupt from IO]
> >                                                                       kref_put(p)
> > 
> > You need an ordering primitive between start_io() and kref_get()
> > or the counter could go negative.
> 
> Really?  On an atomic variable?  I didn't think this was needed for
> atomics to ensure this type of thing couldn't happen.

If you use an atomic variable you can be sure that the result will be
mathematically correct, even if you touch the variable from many CPUs.
(with add & sub of course) That is, refering to that variable.

It does not guarantee ordering

CPU A								CPU B
atomic_set(&a, 1);
atomic_set(&b, 1);
atomic_set(&c, 1);
									while (!atomic_read(&c));
									d = atomic_read(&a) + atomic_read(&b);

is asking for trouble. You need to do:

CPU A								CPU B
atomic_set(&a, 1);
atomic_set(&b, 1);
smp_wmb();
atomic_set(&c, 1);
									while (!atomic_read(&c));
									smp_rmb();
									d = atomic_read(&a) + atomic_read(&b);

Now replace c with an interrupt and you see the problem. It definitely exists,
but my solution was quite bad. The wmb() must be in start_io() in the first example
I gave. Putting it into kref was the wrong place.

	Regards
		Oliver

PS: even in the example I first gave the result will eventually be 0.
But that is useless because the check for zero is done only in kref_put()


-- 
- - - 
SUSE LINUX Products GmbH, GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer, HRB 16746 (AG Nürnberg) 
Maxfeldstraße 5                         
90409 Nürnberg 
Germany 
- - -