Re: [PATCH 1/5] pid: Implement task references.

[Kyle Moffett]

On Jan 30, 2006, at 00:46, Eric Dumazet wrote:
> Kyle Moffett a écrit :
> > On Jan 30, 2006, at 00:19, Eric Dumazet wrote:
> > > -    if (atomic_dec_and_test(&kref->refcount)) {
> > > +    /*
> > > +     * if current count is one, we are the last user and can  
> > > release object
> > > +     * right now, avoiding an atomic operation on 'refcount'
> > > +     */
> > > +    if ((atomic_read(&kref->refcount) == 1) ||
> > Uhh, I think you got this test reversed.  Didn't you mean != 1?   
> > Otherwise you only do the dec_and_test when the refcount is one,  
> > which means that you leak everything kref-ed.
>
> Not at all :)
>
> Your mail is just another proof why kref is a good abstraction :)
>
> If you are the last user of a kref, (refcount = 1), then
> you are sure that nobody else but you is using the object, and as  
> we are kref_put() this object, the atomic_dec_and-test *will* set  
> the count the object and you are going to release() object.
>
> The release() function is not going to look at kref_count again,  
> just free the resources and the object.

OHHH, I see where I got confused.  The indentation was bad, dunno if  
it was my end or yours, so I misread it as this:

if (atomic_read(...) == 1) {
	atomic_dec_and_test(...);
	...
}

instead of this:

if (atomic_read(...) == 1 ||
		atomic_dec_and_test(...)) {
	...
}

This should teach me not to reply this late at night.  Sorry for the  
confusion.

Cheers,
Kyle Moffett

--
They _will_ find opposing experts to say it isn't, if you push hard  
enough the wrong way.  Idiots with a PhD aren't hard to buy.
  -- Rob Landley



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