Messages in this thread | | | Date | Mon, 30 Jan 2006 06:46:30 +0100 | From | Eric Dumazet <> | Subject | Re: [PATCH 1/5] pid: Implement task references. |
| |
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.
Maybe a change in the documentation is necessary to explain this point (release() can e called while the apparent krefcount is 1)
Or in kref_put doing this :
if (atomic_read(&kref->refcount) == 1) { atomic_set(&kref->refcount, 0); release(kref); } if (atomic_dec_and_test(&kref->refcount)) { release(kref); return 1; }
Eric
- 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/
| |