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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux