From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stephen Hemminger <stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ@public.gmane.org>
Subject: Re: [PATCH] rte_mbuf: scattered pktmbufs freeing
 optimization
Date: Thu, 26 Feb 2015 16:49:41 -0800
Message-ID: <20150226164941.68d9371a@urahara>
References: <1424992506-20484-1-git-send-email-vadim.suraev@gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Cc: dev-VfR2kkLFssw@public.gmane.org
To: "vadim.suraev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org" <vadim.suraev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Return-path: <dev-bounces-VfR2kkLFssw@public.gmane.org>
In-Reply-To: <1424992506-20484-1-git-send-email-vadim.suraev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request-VfR2kkLFssw@public.gmane.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev-VfR2kkLFssw@public.gmane.org>
List-Help: <mailto:dev-request-VfR2kkLFssw@public.gmane.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request-VfR2kkLFssw@public.gmane.org?subject=subscribe>
Errors-To: dev-bounces-VfR2kkLFssw@public.gmane.org
Sender: "dev" <dev-bounces-VfR2kkLFssw@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);
> +    }
> +}