Re: [PATCH 3/3] kref: Remove the memory barriers
From: Oliver Neukum
Date: Tue Dec 13 2011 - 06:50:01 EST
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
- - -
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/