Re: [PATCH] rte_mbuf: scattered pktmbufs freeing optimization

[Stephen Hemminger <stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ@public.gmane.org>]

On Fri, 27 Feb 2015 01:15:06 +0200
"vadim.suraev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org" <vadim.suraev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> +static inline void __attribute__((always_inline))
> +rte_pktmbuf_free_bulk(struct rte_mbuf *head)

Quit with the inlining. Inlining all the time isn't faster
it just increase the code bloat and causes i-cache misses.

> +{
> +    void *mbufs[MAX_MBUF_FREE_SIZE];
> +    unsigned mbufs_count = 0;
> +    struct rte_mbuf *next;
> +
> +    RTE_MBUF_MEMPOOL_CHECK1(head);
> +
> +    while(head) {
> +        next = head->next;
> +        head->next = NULL;
> +        if(__rte_pktmbuf_prefree_seg(head)) {

Missing space after 'if'

> +            RTE_MBUF_ASSERT(rte_mbuf_refcnt_read(head) == 0);
> +            RTE_MBUF_MEMPOOL_CHECK2(head);
> +            mbufs[mbufs_count++] = head;
> +        }
> +        head = next;
> +        if(mbufs_count == MAX_MBUF_FREE_SIZE) {
> +            rte_mempool_put_bulk(((struct rte_mbuf *)mbufs[0])->pool,mbufs,mbufs_count);
why not have mbufs[] be type struct rte_mbuf * and avoid casting.
Casting is one of the sources of bugs in C code.


> +            mbufs_count = 0;
> +        }
> +    }
> +    if(mbufs_count > 0) {
Don't need {} on one line if clause

> +        rte_mempool_put_bulk(((struct rte_mbuf *)mbufs[0])->pool,mbufs,mbufs_count);
> +    }
> +}