Messages in this thread | | | From | Kyle Moffett <> | Subject | Re: [PATCH 1/5] pid: Implement task references. | Date | Mon, 30 Jan 2006 01:46:11 -0500 |
| |
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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |