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

[ebiederm@xmission.com (Eric W. Biederman)]

Eric Dumazet <dada1@cosmosbay.com> writes:

> 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 :)

The size all depends on your architecture.  But I agree it
is not much of a size increase in general.

> 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.

It is a valid optimization if you will normally have only one user
like a skb.  For other structures that frequently have many users I'm
not at all certain it is a useful optimization.

Eric