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

[Oliver Neukum]

Am Montag, 12. Dezember 2011, 20:30:40 schrieb Greg KH:
> On Mon, Dec 12, 2011 at 11:20:16AM +0100, Oliver Neukum wrote:
> > Am Montag, 12. Dezember 2011, 10:57:31 schrieb Ming Lei:
> > > On Mon, Dec 12, 2011 at 4:54 PM, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
> > > > On Mon, 2011-12-12 at 11:48 +0800, Ming Lei wrote:
> > > >> For kref, maybe it is still multiple operations done on one cpu vs them
> > > >> being visible on another, but seems a bit implicit, see the common kref
> > > >> usage below:
> > > >>
> > > >> CPU0                            CPU1
> > > >>                                         A:kref_init(&obj->ref)
> > > >
> > > > how does CPU0 get a ref to obj?
> > > 
> > > Suppose open/close/read/.. context is run on CPU0, and driver .probe/.release
> > > context(hotplug context) is run on CPU1. There are a few examples on
> > > usb driver(eg. usb-skeleton.c, ...)
> > 
> > USB generally relies on an implied barrier just as:
> > 
> >         /* we can register the device now, as it is ready */
> >         retval = usb_register_dev(interface, &skel_class);
> > 
> > Generally reference counting cannot help you if kfree() is involved
> 
> So, Oliver, you don't have any objection to this patch removing the
> barriers in kref, right?  Originally you added them, I just wanted to
> make sure before I applied this.

I do not remember any more why I introduced this.

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.
I think I was worried about it missing.

	Regards
		Oliver
--
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/