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.
Eric W. Biederman a écrit :
> Eric Dumazet <dada1@cosmosbay.com> writes:
>
>> 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 !
>
> Largely I think you have the right of it, that the optimization is
> correct. My biggest complaint is that the common case is going to be
> several references to the data structure. Releasing the references
> will always be slow. To do the read you need to get the value into
> the cache line.
>
> So it looks to me like you are optimizing the wrong case and
> bloating the icache with unnecessary code.

This function is not inlined.

Adding a test and a branch is a matter of 7 bytes.

'Bloating the icache' is a litle bit off :)

Avoiding an atomic is important. This is already done elsewhere in the kernel,
in a inlined function with *many* call sites :

(See kfree_skb() in include/linux//skbuff.h )

/*
* If users == 1, we are the only owner and are can avoid redundant
* atomic change.
*/

/**
* kfree_skb - free an sk_buff
* @skb: buffer to free
*
* Drop a reference to the buffer and free it if the usage count has
* hit zero.
*/
static inline void kfree_skb(struct sk_buff *skb)
{
if (likely(atomic_read(&skb->users) == 1))
smp_rmb();
else if (likely(!atomic_dec_and_test(&skb->users)))
return;
__kfree_skb(skb);
}


This is a valid optimization : an atomic_dec_and_test() is very expensive.

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 22:35    [W:0.066 / U:1.896 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site