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

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

 



Greg KH a écrit :
On Mon, Jan 30, 2006 at 06:19:35AM +0100, Eric Dumazet wrote:
Example of improvement in kref_put() :

[PATCH] kref : Avoid an atomic operation in kref_put() when the last reference is dropped. On most platforms, atomic_read() is a plan read of the counter and involves no atomic at all.

No, we wat to decrement and test at the same time, to protect against
any race where someone is incrementing right when we are dropping the
last reference.

Sorry ? Me confused !

What I am saying is :

If a CPUA is doing a kref_put() and refcount == 1, then another CPU *cannot* change the refcount, because only one CPU has a valid reference on the object. (CPUA )

Proof :

If CPUB cannot kref_put() as well because only CPUA owns a refcount. (If two CPUS could kref_put, then counter would become -1 !)

If CPUB is going to do a kref_get() : Not allowed by kref specs  (Rule 3)
(It would mean that both CPUA/B have a reference)

'The kref_get() does not require a lock,
since we already have a valid pointer that we own a refcount for.'

So CPUA and CPUB cannot both own a refcount on a object having refcount=1

If you allow this, then current implementation is buggy as well.

kref->refcount == 1

CPUA :

if (atomic_dec_and_test(&kref->refcount)) {
   returns TRUE condition
[refcount is now 0]

CPUB :
   atomic_inc(&kref->refcount)
[refcount is now 1]

CPUA :
	release(kref); (object freed)

CPUB :
    doing some work on object freed by CPUA. *kaboom*

CPUB : kref_put()
    refcount back to 0 -> object freed twice *kaboom*


So, thanks, but I'm not going to apply this.

greg k-h


Me confused.

Yes, kref abstraction is good :)
-
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