Messages in this thread | | | Date | Mon, 30 Jan 2006 22:32:52 +0100 | From | Eric Dumazet <> | Subject | Re: [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/
| |