lkml.org 
[lkml]   [2006]   [Jan]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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/

\
 
 \ /
  Last update: 2006-01-30 06:49    [W:0.258 / U:0.616 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site