[dpdk-dev] [memnic PATCH v2 6/7] pmd: add branch hint in recv/xmit
> Subject: Re: [dpdk-dev] [memnic PATCH v2 6/7] pmd: add branch hint in > recv/xmit > > On Wed, Oct 01, 2014 at 11:33:23PM +, Hiroshi Shimamoto wrote: > > > Subject: Re: [dpdk-dev] [memnic PATCH v2 6/7] pmd: add branch hint in > > > recv/xmit > > > > > > On Wed, Oct 01, 2014 at 09:12:44AM +, Hiroshi Shimamoto wrote: > > > > > Subject: Re: [dpdk-dev] [memnic PATCH v2 6/7] pmd: add branch hint in > > > > > recv/xmit > > > > > > > > > > On Tue, Sep 30, 2014 at 11:52:00PM +0000, Hiroshi Shimamoto wrote: > > > > > > Hi, > > > > > > > > > > > > > Subject: Re: [dpdk-dev] [memnic PATCH v2 6/7] pmd: add branch > > > > > > > hint in recv/xmit > > > > > > > > > > > > > > On Tue, Sep 30, 2014 at 11:14:40AM +, Hiroshi Shimamoto wrote: > > > > > > > > From: Hiroshi Shimamoto > > > > > > > > > > > > > > > > To reduce instruction cache miss, add branch condition hints > > > > > > > > into > > > > > > > > recv/xmit functions. This improves a bit performance. > > > > > > > > > > > > > > > > We can see performance improvements with memnic-tester. > > > > > > > > Using Xeon E5-2697 v2 @ 2.70GHz, 4 vCPU. > > > > > > > > size | before | after > > > > > > > >64 | 5.54Mpps | 5.55Mpps > > > > > > > > 128 | 5.46Mpps | 5.44Mpps > > > > > > > > 256 | 5.21Mpps | 5.22Mpps > > > > > > > > 512 | 4.50Mpps | 4.52Mpps > > > > > > > > 1024 | 3.71Mpps | 3.73Mpps > > > > > > > > 1280 | 3.21Mpps | 3.22Mpps > > > > > > > > 1518 | 2.92Mpps | 2.93Mpps > > > > > > > > > > > > > > > > Signed-off-by: Hiroshi Shimamoto > > > > > > > > Reviewed-by: Hayato Momma > > > > > > > > --- > > > > > > > > pmd/pmd_memnic.c | 18 +- > > > > > > > > 1 file changed, 9 insertions(+), 9 deletions(-) > > > > > > > > > > > > > > > > diff --git a/pmd/pmd_memnic.c b/pmd/pmd_memnic.c > > > > > > > > index 7fc3093..875d3ea 100644 > > > > > > > > --- a/pmd/pmd_memnic.c > > > > > > > > +++ b/pmd/pmd_memnic.c > > > > > > > > @@ -289,26 +289,26 @@ static uint16_t memnic_recv_pkts(void > > > > > > > > *rx_queue, > > > > > > > > int idx, next; > > > > > > > > struct rte_eth_stats *st = > > > > > > > > &adapter->stats[rte_lcore_id()]; > > > > > > > > > > > > > > > > - if (!adapter->nic->hdr.valid) > > > > > > > > + if (unlikely(!adapter->nic->hdr.valid)) > > > > > > > > return 0; > > > > > > > > > > > > > > > > pkts = bytes = errs = 0; > > > > > > > > idx = adapter->up_idx; > > > > > > > > for (nr = 0; nr < nb_pkts; nr++) { > > > > > > > > p = &data->packets[idx]; > > > > > > > > - if (p->status != MEMNIC_PKT_ST_FILLED) > > > > > > > > + if (unlikely(p->status != MEMNIC_PKT_ST_FILLED)) > > > > > > > > break; > > > > > > > > /* prefetch the next area */ > > > > > > > > next = idx; > > > > > > > > - if (++next >= MEMNIC_NR_PACKET) > > > > > > > > + if (unlikely(++next >= MEMNIC_NR_PACKET)) > > > > > > > > next = 0; > > > > > > > > rte_prefetch0(&data->packets[next]); > > > > > > > > - if (p->len > framesz) { > > > > > > > > + if (unlikely(p->len > framesz)) { > > > > > > > > errs++; > > > > > > > > goto drop; > > > > > > > >
[dpdk-dev] [memnic PATCH v2 6/7] pmd: add branch hint in recv/xmit
> Subject: Re: [dpdk-dev] [memnic PATCH v2 6/7] pmd: add branch hint in > recv/xmit > > On Wed, Oct 01, 2014 at 09:12:44AM +, Hiroshi Shimamoto wrote: > > > Subject: Re: [dpdk-dev] [memnic PATCH v2 6/7] pmd: add branch hint in > > > recv/xmit > > > > > > On Tue, Sep 30, 2014 at 11:52:00PM +, Hiroshi Shimamoto wrote: > > > > Hi, > > > > > > > > > Subject: Re: [dpdk-dev] [memnic PATCH v2 6/7] pmd: add branch hint in > > > > > recv/xmit > > > > > > > > > > On Tue, Sep 30, 2014 at 11:14:40AM +, Hiroshi Shimamoto wrote: > > > > > > From: Hiroshi Shimamoto > > > > > > > > > > > > To reduce instruction cache miss, add branch condition hints into > > > > > > recv/xmit functions. This improves a bit performance. > > > > > > > > > > > > We can see performance improvements with memnic-tester. > > > > > > Using Xeon E5-2697 v2 @ 2.70GHz, 4 vCPU. > > > > > > size | before | after > > > > > >64 | 5.54Mpps | 5.55Mpps > > > > > > 128 | 5.46Mpps | 5.44Mpps > > > > > > 256 | 5.21Mpps | 5.22Mpps > > > > > > 512 | 4.50Mpps | 4.52Mpps > > > > > > 1024 | 3.71Mpps | 3.73Mpps > > > > > > 1280 | 3.21Mpps | 3.22Mpps > > > > > > 1518 | 2.92Mpps | 2.93Mpps > > > > > > > > > > > > Signed-off-by: Hiroshi Shimamoto > > > > > > Reviewed-by: Hayato Momma > > > > > > --- > > > > > > pmd/pmd_memnic.c | 18 +- > > > > > > 1 file changed, 9 insertions(+), 9 deletions(-) > > > > > > > > > > > > diff --git a/pmd/pmd_memnic.c b/pmd/pmd_memnic.c > > > > > > index 7fc3093..875d3ea 100644 > > > > > > --- a/pmd/pmd_memnic.c > > > > > > +++ b/pmd/pmd_memnic.c > > > > > > @@ -289,26 +289,26 @@ static uint16_t memnic_recv_pkts(void > > > > > > *rx_queue, > > > > > > int idx, next; > > > > > > struct rte_eth_stats *st = &adapter->stats[rte_lcore_id()]; > > > > > > > > > > > > - if (!adapter->nic->hdr.valid) > > > > > > + if (unlikely(!adapter->nic->hdr.valid)) > > > > > > return 0; > > > > > > > > > > > > pkts = bytes = errs = 0; > > > > > > idx = adapter->up_idx; > > > > > > for (nr = 0; nr < nb_pkts; nr++) { > > > > > > p = &data->packets[idx]; > > > > > > - if (p->status != MEMNIC_PKT_ST_FILLED) > > > > > > + if (unlikely(p->status != MEMNIC_PKT_ST_FILLED)) > > > > > > break; > > > > > > /* prefetch the next area */ > > > > > > next = idx; > > > > > > - if (++next >= MEMNIC_NR_PACKET) > > > > > > + if (unlikely(++next >= MEMNIC_NR_PACKET)) > > > > > > next = 0; > > > > > > rte_prefetch0(&data->packets[next]); > > > > > > - if (p->len > framesz) { > > > > > > + if (unlikely(p->len > framesz)) { > > > > > > errs++; > > > > > > goto drop; > > > > > > } > > > > > > mb = rte_pktmbuf_alloc(adapter->mp); > > > > > > - if (!mb) > > > > > > + if (unlikely(!mb)) > > > > > > break; > > > > > > > > > > > > rte_memcpy(rte_pktmbuf_mtod(mb, void *), p->data, > > > > > > p->len); > > > > > > @@ -350,7 +350,7 @@ static uint16_t memnic_xmit_pkts(void *tx_queue, > > > > > > uint64_t pkts, bytes, errs; > > > > > > uint32_t framesz = adapter->framesz; > > > > > > > > > > > > - if (!adapter->nic->hdr.valid) > > > > > > + if (unlikely(!adapter->nic->hdr.valid)) > > > > > > return 0; > > > > > > > > > > > > pkts = bytes = errs = 0; > > > > > > @
[dpdk-dev] [memnic PATCH v2 6/7] pmd: add branch hint in recv/xmit
On Thu, Oct 02, 2014 at 02:07:09AM +, Hiroshi Shimamoto wrote: > > Subject: Re: [dpdk-dev] [memnic PATCH v2 6/7] pmd: add branch hint in > > recv/xmit > > > > On Wed, Oct 01, 2014 at 11:33:23PM +, Hiroshi Shimamoto wrote: > > > > Subject: Re: [dpdk-dev] [memnic PATCH v2 6/7] pmd: add branch hint in > > > > recv/xmit > > > > > > > > On Wed, Oct 01, 2014 at 09:12:44AM +0000, Hiroshi Shimamoto wrote: > > > > > > Subject: Re: [dpdk-dev] [memnic PATCH v2 6/7] pmd: add branch hint > > > > > > in recv/xmit > > > > > > > > > > > > On Tue, Sep 30, 2014 at 11:52:00PM +, Hiroshi Shimamoto wrote: > > > > > > > Hi, > > > > > > > > > > > > > > > Subject: Re: [dpdk-dev] [memnic PATCH v2 6/7] pmd: add branch > > > > > > > > hint in recv/xmit > > > > > > > > > > > > > > > > On Tue, Sep 30, 2014 at 11:14:40AM +, Hiroshi Shimamoto > > > > > > > > wrote: > > > > > > > > > From: Hiroshi Shimamoto > > > > > > > > > > > > > > > > > > To reduce instruction cache miss, add branch condition hints > > > > > > > > > into > > > > > > > > > recv/xmit functions. This improves a bit performance. > > > > > > > > > > > > > > > > > > We can see performance improvements with memnic-tester. > > > > > > > > > Using Xeon E5-2697 v2 @ 2.70GHz, 4 vCPU. > > > > > > > > > size | before | after > > > > > > > > >64 | 5.54Mpps | 5.55Mpps > > > > > > > > > 128 | 5.46Mpps | 5.44Mpps > > > > > > > > > 256 | 5.21Mpps | 5.22Mpps > > > > > > > > > 512 | 4.50Mpps | 4.52Mpps > > > > > > > > > 1024 | 3.71Mpps | 3.73Mpps > > > > > > > > > 1280 | 3.21Mpps | 3.22Mpps > > > > > > > > > 1518 | 2.92Mpps | 2.93Mpps > > > > > > > > > > > > > > > > > > Signed-off-by: Hiroshi Shimamoto > > > > > > > > ct.jp.nec.com> > > > > > > > > > Reviewed-by: Hayato Momma > > > > > > > > > --- > > > > > > > > > pmd/pmd_memnic.c | 18 +- > > > > > > > > > 1 file changed, 9 insertions(+), 9 deletions(-) > > > > > > > > > > > > > > > > > > diff --git a/pmd/pmd_memnic.c b/pmd/pmd_memnic.c > > > > > > > > > index 7fc3093..875d3ea 100644 > > > > > > > > > --- a/pmd/pmd_memnic.c > > > > > > > > > +++ b/pmd/pmd_memnic.c > > > > > > > > > @@ -289,26 +289,26 @@ static uint16_t memnic_recv_pkts(void > > > > > > > > > *rx_queue, > > > > > > > > > int idx, next; > > > > > > > > > struct rte_eth_stats *st = > > > > > > > > > &adapter->stats[rte_lcore_id()]; > > > > > > > > > > > > > > > > > > - if (!adapter->nic->hdr.valid) > > > > > > > > > + if (unlikely(!adapter->nic->hdr.valid)) > > > > > > > > > return 0; > > > > > > > > > > > > > > > > > > pkts = bytes = errs = 0; > > > > > > > > > idx = adapter->up_idx; > > > > > > > > > for (nr = 0; nr < nb_pkts; nr++) { > > > > > > > > > p = &data->packets[idx]; > > > > > > > > > - if (p->status != MEMNIC_PKT_ST_FILLED) > > > > > > > > > + if (unlikely(p->status != MEMNIC_PKT_ST_FILLED)) > > > > > > > > > break; > > > > > > > > > /* prefetch the next area */ > > > > > > > > > next = idx; > > > > > > > > > - if (++next >= MEMNIC_NR_PACKET) > > > > > > > > > + if (unlikely(++next >= MEMNIC_NR_PACKET)) > > > > > > > > > ne
[dpdk-dev] [memnic PATCH v2 6/7] pmd: add branch hint in recv/xmit
On Wed, Oct 01, 2014 at 11:33:23PM +, Hiroshi Shimamoto wrote: > > Subject: Re: [dpdk-dev] [memnic PATCH v2 6/7] pmd: add branch hint in > > recv/xmit > > > > On Wed, Oct 01, 2014 at 09:12:44AM +, Hiroshi Shimamoto wrote: > > > > Subject: Re: [dpdk-dev] [memnic PATCH v2 6/7] pmd: add branch hint in > > > > recv/xmit > > > > > > > > On Tue, Sep 30, 2014 at 11:52:00PM +, Hiroshi Shimamoto wrote: > > > > > Hi, > > > > > > > > > > > Subject: Re: [dpdk-dev] [memnic PATCH v2 6/7] pmd: add branch hint > > > > > > in recv/xmit > > > > > > > > > > > > On Tue, Sep 30, 2014 at 11:14:40AM +, Hiroshi Shimamoto wrote: > > > > > > > From: Hiroshi Shimamoto > > > > > > > > > > > > > > To reduce instruction cache miss, add branch condition hints into > > > > > > > recv/xmit functions. This improves a bit performance. > > > > > > > > > > > > > > We can see performance improvements with memnic-tester. > > > > > > > Using Xeon E5-2697 v2 @ 2.70GHz, 4 vCPU. > > > > > > > size | before | after > > > > > > >64 | 5.54Mpps | 5.55Mpps > > > > > > > 128 | 5.46Mpps | 5.44Mpps > > > > > > > 256 | 5.21Mpps | 5.22Mpps > > > > > > > 512 | 4.50Mpps | 4.52Mpps > > > > > > > 1024 | 3.71Mpps | 3.73Mpps > > > > > > > 1280 | 3.21Mpps | 3.22Mpps > > > > > > > 1518 | 2.92Mpps | 2.93Mpps > > > > > > > > > > > > > > Signed-off-by: Hiroshi Shimamoto > > > > > > > Reviewed-by: Hayato Momma > > > > > > > --- > > > > > > > pmd/pmd_memnic.c | 18 +- > > > > > > > 1 file changed, 9 insertions(+), 9 deletions(-) > > > > > > > > > > > > > > diff --git a/pmd/pmd_memnic.c b/pmd/pmd_memnic.c > > > > > > > index 7fc3093..875d3ea 100644 > > > > > > > --- a/pmd/pmd_memnic.c > > > > > > > +++ b/pmd/pmd_memnic.c > > > > > > > @@ -289,26 +289,26 @@ static uint16_t memnic_recv_pkts(void > > > > > > > *rx_queue, > > > > > > > int idx, next; > > > > > > > struct rte_eth_stats *st = &adapter->stats[rte_lcore_id()]; > > > > > > > > > > > > > > - if (!adapter->nic->hdr.valid) > > > > > > > + if (unlikely(!adapter->nic->hdr.valid)) > > > > > > > return 0; > > > > > > > > > > > > > > pkts = bytes = errs = 0; > > > > > > > idx = adapter->up_idx; > > > > > > > for (nr = 0; nr < nb_pkts; nr++) { > > > > > > > p = &data->packets[idx]; > > > > > > > - if (p->status != MEMNIC_PKT_ST_FILLED) > > > > > > > + if (unlikely(p->status != MEMNIC_PKT_ST_FILLED)) > > > > > > > break; > > > > > > > /* prefetch the next area */ > > > > > > > next = idx; > > > > > > > - if (++next >= MEMNIC_NR_PACKET) > > > > > > > + if (unlikely(++next >= MEMNIC_NR_PACKET)) > > > > > > > next = 0; > > > > > > > rte_prefetch0(&data->packets[next]); > > > > > > > - if (p->len > framesz) { > > > > > > > + if (unlikely(p->len > framesz)) { > > > > > > > errs++; > > > > > > > goto drop; > > > > > > > } > > > > > > > mb = rte_pktmbuf_alloc(adapter->mp); > > > > > > > - if (!mb) > > > > > > > + if (unlikely(!mb)) > > > > > > > break; > > > > > > > > > > > > > > rte_memcpy(rte_pktmbuf_mtod(mb, void *), p->data, > > > > > > > p->len); > > > > > > > @@ -350,7 +350,7 @@ static uint16_t memnic_xmit_pkts(void > > > > > > > *tx_queue, > > > > > > > uint64_t pkts, bytes, errs
[dpdk-dev] [memnic PATCH v2 6/7] pmd: add branch hint in recv/xmit
On Tue, Sep 30, 2014 at 11:52:00PM +, Hiroshi Shimamoto wrote: > Hi, > > > Subject: Re: [dpdk-dev] [memnic PATCH v2 6/7] pmd: add branch hint in > > recv/xmit > > > > On Tue, Sep 30, 2014 at 11:14:40AM +, Hiroshi Shimamoto wrote: > > > From: Hiroshi Shimamoto > > > > > > To reduce instruction cache miss, add branch condition hints into > > > recv/xmit functions. This improves a bit performance. > > > > > > We can see performance improvements with memnic-tester. > > > Using Xeon E5-2697 v2 @ 2.70GHz, 4 vCPU. > > > size | before | after > > >64 | 5.54Mpps | 5.55Mpps > > > 128 | 5.46Mpps | 5.44Mpps > > > 256 | 5.21Mpps | 5.22Mpps > > > 512 | 4.50Mpps | 4.52Mpps > > > 1024 | 3.71Mpps | 3.73Mpps > > > 1280 | 3.21Mpps | 3.22Mpps > > > 1518 | 2.92Mpps | 2.93Mpps > > > > > > Signed-off-by: Hiroshi Shimamoto > > > Reviewed-by: Hayato Momma > > > --- > > > pmd/pmd_memnic.c | 18 +- > > > 1 file changed, 9 insertions(+), 9 deletions(-) > > > > > > diff --git a/pmd/pmd_memnic.c b/pmd/pmd_memnic.c > > > index 7fc3093..875d3ea 100644 > > > --- a/pmd/pmd_memnic.c > > > +++ b/pmd/pmd_memnic.c > > > @@ -289,26 +289,26 @@ static uint16_t memnic_recv_pkts(void *rx_queue, > > > int idx, next; > > > struct rte_eth_stats *st = &adapter->stats[rte_lcore_id()]; > > > > > > - if (!adapter->nic->hdr.valid) > > > + if (unlikely(!adapter->nic->hdr.valid)) > > > return 0; > > > > > > pkts = bytes = errs = 0; > > > idx = adapter->up_idx; > > > for (nr = 0; nr < nb_pkts; nr++) { > > > p = &data->packets[idx]; > > > - if (p->status != MEMNIC_PKT_ST_FILLED) > > > + if (unlikely(p->status != MEMNIC_PKT_ST_FILLED)) > > > break; > > > /* prefetch the next area */ > > > next = idx; > > > - if (++next >= MEMNIC_NR_PACKET) > > > + if (unlikely(++next >= MEMNIC_NR_PACKET)) > > > next = 0; > > > rte_prefetch0(&data->packets[next]); > > > - if (p->len > framesz) { > > > + if (unlikely(p->len > framesz)) { > > > errs++; > > > goto drop; > > > } > > > mb = rte_pktmbuf_alloc(adapter->mp); > > > - if (!mb) > > > + if (unlikely(!mb)) > > > break; > > > > > > rte_memcpy(rte_pktmbuf_mtod(mb, void *), p->data, p->len); > > > @@ -350,7 +350,7 @@ static uint16_t memnic_xmit_pkts(void *tx_queue, > > > uint64_t pkts, bytes, errs; > > > uint32_t framesz = adapter->framesz; > > > > > > - if (!adapter->nic->hdr.valid) > > > + if (unlikely(!adapter->nic->hdr.valid)) > > > return 0; > > > > > > pkts = bytes = errs = 0; > > > @@ -360,7 +360,7 @@ static uint16_t memnic_xmit_pkts(void *tx_queue, > > > struct rte_mbuf *sg; > > > void *ptr; > > > > > > - if (pkt_len > framesz) { > > > + if (unlikely(pkt_len > framesz)) { > > > errs++; > > > break; > > > } > > > @@ -379,7 +379,7 @@ retry: > > > goto retry; > > > } > > > > > > - if (idx != ACCESS_ONCE(adapter->down_idx)) { > > > + if (unlikely(idx != ACCESS_ONCE(adapter->down_idx))) { > > Why are you using ACCESS_ONCE here? Or for that matter, anywhere else in > > this > > PMD? The whole idea of the ACCESS_ONCE macro is to assign a value to a > > variable > > once and prevent it from getting reloaded from memory at a later time, this > > is > > exactly contrary to that, both in the sense that you're explicitly > > reloading the > > same variable multiple times, and that you're using it as part of a > > comparison > > operation, rather than an asignment operation > > ACCESS_ONCE prevents compiler optimization and ensures load from memory. > There could be multiple threads which read/write that index. > We should compare the value previous and the current value in memory. > In that reason, I use ACCESS_ONCE macro to get value in the memory. Should you not just make the variable volatile? That's the normal way to guarantee reads from memory and prevent the compiler caching things in registers. /Bruce > > thanks, > Hiroshi > > > > > Neil >
[dpdk-dev] [memnic PATCH v2 6/7] pmd: add branch hint in recv/xmit
> Subject: Re: [dpdk-dev] [memnic PATCH v2 6/7] pmd: add branch hint in > recv/xmit > > On Tue, Sep 30, 2014 at 11:52:00PM +, Hiroshi Shimamoto wrote: > > Hi, > > > > > Subject: Re: [dpdk-dev] [memnic PATCH v2 6/7] pmd: add branch hint in > > > recv/xmit > > > > > > On Tue, Sep 30, 2014 at 11:14:40AM +, Hiroshi Shimamoto wrote: > > > > From: Hiroshi Shimamoto > > > > > > > > To reduce instruction cache miss, add branch condition hints into > > > > recv/xmit functions. This improves a bit performance. > > > > > > > > We can see performance improvements with memnic-tester. > > > > Using Xeon E5-2697 v2 @ 2.70GHz, 4 vCPU. > > > > size | before | after > > > >64 | 5.54Mpps | 5.55Mpps > > > > 128 | 5.46Mpps | 5.44Mpps > > > > 256 | 5.21Mpps | 5.22Mpps > > > > 512 | 4.50Mpps | 4.52Mpps > > > > 1024 | 3.71Mpps | 3.73Mpps > > > > 1280 | 3.21Mpps | 3.22Mpps > > > > 1518 | 2.92Mpps | 2.93Mpps > > > > > > > > Signed-off-by: Hiroshi Shimamoto > > > > Reviewed-by: Hayato Momma > > > > --- > > > > pmd/pmd_memnic.c | 18 +- > > > > 1 file changed, 9 insertions(+), 9 deletions(-) > > > > > > > > diff --git a/pmd/pmd_memnic.c b/pmd/pmd_memnic.c > > > > index 7fc3093..875d3ea 100644 > > > > --- a/pmd/pmd_memnic.c > > > > +++ b/pmd/pmd_memnic.c > > > > @@ -289,26 +289,26 @@ static uint16_t memnic_recv_pkts(void *rx_queue, > > > > int idx, next; > > > > struct rte_eth_stats *st = &adapter->stats[rte_lcore_id()]; > > > > > > > > - if (!adapter->nic->hdr.valid) > > > > + if (unlikely(!adapter->nic->hdr.valid)) > > > > return 0; > > > > > > > > pkts = bytes = errs = 0; > > > > idx = adapter->up_idx; > > > > for (nr = 0; nr < nb_pkts; nr++) { > > > > p = &data->packets[idx]; > > > > - if (p->status != MEMNIC_PKT_ST_FILLED) > > > > + if (unlikely(p->status != MEMNIC_PKT_ST_FILLED)) > > > > break; > > > > /* prefetch the next area */ > > > > next = idx; > > > > - if (++next >= MEMNIC_NR_PACKET) > > > > + if (unlikely(++next >= MEMNIC_NR_PACKET)) > > > > next = 0; > > > > rte_prefetch0(&data->packets[next]); > > > > - if (p->len > framesz) { > > > > + if (unlikely(p->len > framesz)) { > > > > errs++; > > > > goto drop; > > > > } > > > > mb = rte_pktmbuf_alloc(adapter->mp); > > > > - if (!mb) > > > > + if (unlikely(!mb)) > > > > break; > > > > > > > > rte_memcpy(rte_pktmbuf_mtod(mb, void *), p->data, > > > > p->len); > > > > @@ -350,7 +350,7 @@ static uint16_t memnic_xmit_pkts(void *tx_queue, > > > > uint64_t pkts, bytes, errs; > > > > uint32_t framesz = adapter->framesz; > > > > > > > > - if (!adapter->nic->hdr.valid) > > > > + if (unlikely(!adapter->nic->hdr.valid)) > > > > return 0; > > > > > > > > pkts = bytes = errs = 0; > > > > @@ -360,7 +360,7 @@ static uint16_t memnic_xmit_pkts(void *tx_queue, > > > > struct rte_mbuf *sg; > > > > void *ptr; > > > > > > > > - if (pkt_len > framesz) { > > > > + if (unlikely(pkt_len > framesz)) { > > > > errs++; > > > > break; > > > > } > > > > @@ -379,7 +379,7 @@ retry: > > > > goto retry; > > > > } > > > > > > > > - if (idx != ACCESS_ONCE(adapter->down_idx)) { > > > > + if (unlikely(idx != ACCESS_ONCE(adapter->down_idx))) { > > > Why are you using AC
[dpdk-dev] [memnic PATCH v2 6/7] pmd: add branch hint in recv/xmit
2014-10-01 00:01, Hiroshi Shimamoto: > > The patch is ok. For the commit message, is it better > > "to reduce branch mispredication"? > > yes, that seems more suitable to explain the situation. > > Thomas, what do you think? Can you replace the message when you apply > this patch? Yes, I did it ;) -- Thomas
[dpdk-dev] [memnic PATCH v2 6/7] pmd: add branch hint in recv/xmit
On Wed, Oct 01, 2014 at 09:12:44AM +, Hiroshi Shimamoto wrote: > > Subject: Re: [dpdk-dev] [memnic PATCH v2 6/7] pmd: add branch hint in > > recv/xmit > > > > On Tue, Sep 30, 2014 at 11:52:00PM +, Hiroshi Shimamoto wrote: > > > Hi, > > > > > > > Subject: Re: [dpdk-dev] [memnic PATCH v2 6/7] pmd: add branch hint in > > > > recv/xmit > > > > > > > > On Tue, Sep 30, 2014 at 11:14:40AM +, Hiroshi Shimamoto wrote: > > > > > From: Hiroshi Shimamoto > > > > > > > > > > To reduce instruction cache miss, add branch condition hints into > > > > > recv/xmit functions. This improves a bit performance. > > > > > > > > > > We can see performance improvements with memnic-tester. > > > > > Using Xeon E5-2697 v2 @ 2.70GHz, 4 vCPU. > > > > > size | before | after > > > > >64 | 5.54Mpps | 5.55Mpps > > > > > 128 | 5.46Mpps | 5.44Mpps > > > > > 256 | 5.21Mpps | 5.22Mpps > > > > > 512 | 4.50Mpps | 4.52Mpps > > > > > 1024 | 3.71Mpps | 3.73Mpps > > > > > 1280 | 3.21Mpps | 3.22Mpps > > > > > 1518 | 2.92Mpps | 2.93Mpps > > > > > > > > > > Signed-off-by: Hiroshi Shimamoto > > > > > Reviewed-by: Hayato Momma > > > > > --- > > > > > pmd/pmd_memnic.c | 18 +- > > > > > 1 file changed, 9 insertions(+), 9 deletions(-) > > > > > > > > > > diff --git a/pmd/pmd_memnic.c b/pmd/pmd_memnic.c > > > > > index 7fc3093..875d3ea 100644 > > > > > --- a/pmd/pmd_memnic.c > > > > > +++ b/pmd/pmd_memnic.c > > > > > @@ -289,26 +289,26 @@ static uint16_t memnic_recv_pkts(void *rx_queue, > > > > > int idx, next; > > > > > struct rte_eth_stats *st = &adapter->stats[rte_lcore_id()]; > > > > > > > > > > - if (!adapter->nic->hdr.valid) > > > > > + if (unlikely(!adapter->nic->hdr.valid)) > > > > > return 0; > > > > > > > > > > pkts = bytes = errs = 0; > > > > > idx = adapter->up_idx; > > > > > for (nr = 0; nr < nb_pkts; nr++) { > > > > > p = &data->packets[idx]; > > > > > - if (p->status != MEMNIC_PKT_ST_FILLED) > > > > > + if (unlikely(p->status != MEMNIC_PKT_ST_FILLED)) > > > > > break; > > > > > /* prefetch the next area */ > > > > > next = idx; > > > > > - if (++next >= MEMNIC_NR_PACKET) > > > > > + if (unlikely(++next >= MEMNIC_NR_PACKET)) > > > > > next = 0; > > > > > rte_prefetch0(&data->packets[next]); > > > > > - if (p->len > framesz) { > > > > > + if (unlikely(p->len > framesz)) { > > > > > errs++; > > > > > goto drop; > > > > > } > > > > > mb = rte_pktmbuf_alloc(adapter->mp); > > > > > - if (!mb) > > > > > + if (unlikely(!mb)) > > > > > break; > > > > > > > > > > rte_memcpy(rte_pktmbuf_mtod(mb, void *), p->data, > > > > > p->len); > > > > > @@ -350,7 +350,7 @@ static uint16_t memnic_xmit_pkts(void *tx_queue, > > > > > uint64_t pkts, bytes, errs; > > > > > uint32_t framesz = adapter->framesz; > > > > > > > > > > - if (!adapter->nic->hdr.valid) > > > > > + if (unlikely(!adapter->nic->hdr.valid)) > > > > > return 0; > > > > > > > > > > pkts = bytes = errs = 0; > > > > > @@ -360,7 +360,7 @@ static uint16_t memnic_xmit_pkts(void *tx_queue, > > > > > struct rte_mbuf *sg; > > > > > void *ptr; > > > > > > > > > > - if (pkt_len > framesz) { > > > > > + if (unlikely(pkt_len > framesz)) { > > > > > errs++; > > > > > break; >
[dpdk-dev] [memnic PATCH v2 6/7] pmd: add branch hint in recv/xmit
On Wed, Oct 01, 2014 at 09:44:45AM +0100, Bruce Richardson wrote: > On Tue, Sep 30, 2014 at 11:52:00PM +, Hiroshi Shimamoto wrote: > > Hi, > > > > > Subject: Re: [dpdk-dev] [memnic PATCH v2 6/7] pmd: add branch hint in > > > recv/xmit > > > > > > On Tue, Sep 30, 2014 at 11:14:40AM +, Hiroshi Shimamoto wrote: > > > > From: Hiroshi Shimamoto > > > > > > > > To reduce instruction cache miss, add branch condition hints into > > > > recv/xmit functions. This improves a bit performance. > > > > > > > > We can see performance improvements with memnic-tester. > > > > Using Xeon E5-2697 v2 @ 2.70GHz, 4 vCPU. > > > > size | before | after > > > >64 | 5.54Mpps | 5.55Mpps > > > > 128 | 5.46Mpps | 5.44Mpps > > > > 256 | 5.21Mpps | 5.22Mpps > > > > 512 | 4.50Mpps | 4.52Mpps > > > > 1024 | 3.71Mpps | 3.73Mpps > > > > 1280 | 3.21Mpps | 3.22Mpps > > > > 1518 | 2.92Mpps | 2.93Mpps > > > > > > > > Signed-off-by: Hiroshi Shimamoto > > > > Reviewed-by: Hayato Momma > > > > --- > > > > pmd/pmd_memnic.c | 18 +- > > > > 1 file changed, 9 insertions(+), 9 deletions(-) > > > > > > > > diff --git a/pmd/pmd_memnic.c b/pmd/pmd_memnic.c > > > > index 7fc3093..875d3ea 100644 > > > > --- a/pmd/pmd_memnic.c > > > > +++ b/pmd/pmd_memnic.c > > > > @@ -289,26 +289,26 @@ static uint16_t memnic_recv_pkts(void *rx_queue, > > > > int idx, next; > > > > struct rte_eth_stats *st = &adapter->stats[rte_lcore_id()]; > > > > > > > > - if (!adapter->nic->hdr.valid) > > > > + if (unlikely(!adapter->nic->hdr.valid)) > > > > return 0; > > > > > > > > pkts = bytes = errs = 0; > > > > idx = adapter->up_idx; > > > > for (nr = 0; nr < nb_pkts; nr++) { > > > > p = &data->packets[idx]; > > > > - if (p->status != MEMNIC_PKT_ST_FILLED) > > > > + if (unlikely(p->status != MEMNIC_PKT_ST_FILLED)) > > > > break; > > > > /* prefetch the next area */ > > > > next = idx; > > > > - if (++next >= MEMNIC_NR_PACKET) > > > > + if (unlikely(++next >= MEMNIC_NR_PACKET)) > > > > next = 0; > > > > rte_prefetch0(&data->packets[next]); > > > > - if (p->len > framesz) { > > > > + if (unlikely(p->len > framesz)) { > > > > errs++; > > > > goto drop; > > > > } > > > > mb = rte_pktmbuf_alloc(adapter->mp); > > > > - if (!mb) > > > > + if (unlikely(!mb)) > > > > break; > > > > > > > > rte_memcpy(rte_pktmbuf_mtod(mb, void *), p->data, > > > > p->len); > > > > @@ -350,7 +350,7 @@ static uint16_t memnic_xmit_pkts(void *tx_queue, > > > > uint64_t pkts, bytes, errs; > > > > uint32_t framesz = adapter->framesz; > > > > > > > > - if (!adapter->nic->hdr.valid) > > > > + if (unlikely(!adapter->nic->hdr.valid)) > > > > return 0; > > > > > > > > pkts = bytes = errs = 0; > > > > @@ -360,7 +360,7 @@ static uint16_t memnic_xmit_pkts(void *tx_queue, > > > > struct rte_mbuf *sg; > > > > void *ptr; > > > > > > > > - if (pkt_len > framesz) { > > > > + if (unlikely(pkt_len > framesz)) { > > > > errs++; > > > > break; > > > > } > > > > @@ -379,7 +379,7 @@ retry: > > > > goto retry; > > > > } > > > > > > > > - if (idx != ACCESS_ONCE(adapter->down_idx)) { > > > > + if (unlikely(idx != ACCESS_ONCE(adapter->down_idx))) { > > > Why are you using ACCESS_ONCE here? Or for that matter,
[dpdk-dev] [memnic PATCH v2 6/7] pmd: add branch hint in recv/xmit
Hi, > Subject: RE: [memnic PATCH v2 6/7] pmd: add branch hint in recv/xmit > > The patch is ok. For the commit message, is it better > "to reduce branch mispredication"? yes, that seems more suitable to explain the situation. Thomas, what do you think? Can you replace the message when you apply this patch? thanks, Hiroshi > > > -Original Message- > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Hiroshi Shimamoto > > Sent: Tuesday, September 30, 2014 7:15 PM > > To: dev at dpdk.org > > Cc: Hayato Momma > > Subject: [dpdk-dev] [memnic PATCH v2 6/7] pmd: add branch hint in recv/xmit > > > > From: Hiroshi Shimamoto > > > > To reduce instruction cache miss, add branch condition hints into > > recv/xmit functions. This improves a bit performance. > > > > We can see performance improvements with memnic-tester. > > Using Xeon E5-2697 v2 @ 2.70GHz, 4 vCPU. > > size | before | after > >64 | 5.54Mpps | 5.55Mpps > > 128 | 5.46Mpps | 5.44Mpps > > 256 | 5.21Mpps | 5.22Mpps > > 512 | 4.50Mpps | 4.52Mpps > > 1024 | 3.71Mpps | 3.73Mpps > > 1280 | 3.21Mpps | 3.22Mpps > > 1518 | 2.92Mpps | 2.93Mpps > > > > Signed-off-by: Hiroshi Shimamoto > > Reviewed-by: Hayato Momma > > --- > > pmd/pmd_memnic.c | 18 +- > > 1 file changed, 9 insertions(+), 9 deletions(-) > > > > diff --git a/pmd/pmd_memnic.c b/pmd/pmd_memnic.c > > index 7fc3093..875d3ea 100644 > > --- a/pmd/pmd_memnic.c > > +++ b/pmd/pmd_memnic.c > > @@ -289,26 +289,26 @@ static uint16_t memnic_recv_pkts(void *rx_queue, > > int idx, next; > > struct rte_eth_stats *st = &adapter->stats[rte_lcore_id()]; > > > > - if (!adapter->nic->hdr.valid) > > + if (unlikely(!adapter->nic->hdr.valid)) > > return 0; > > > > pkts = bytes = errs = 0; > > idx = adapter->up_idx; > > for (nr = 0; nr < nb_pkts; nr++) { > > p = &data->packets[idx]; > > - if (p->status != MEMNIC_PKT_ST_FILLED) > > + if (unlikely(p->status != MEMNIC_PKT_ST_FILLED)) > > break; > > /* prefetch the next area */ > > next = idx; > > - if (++next >= MEMNIC_NR_PACKET) > > + if (unlikely(++next >= MEMNIC_NR_PACKET)) > > next = 0; > > rte_prefetch0(&data->packets[next]); > > - if (p->len > framesz) { > > + if (unlikely(p->len > framesz)) { > > errs++; > > goto drop; > > } > > mb = rte_pktmbuf_alloc(adapter->mp); > > - if (!mb) > > + if (unlikely(!mb)) > > break; > > > > rte_memcpy(rte_pktmbuf_mtod(mb, void *), p->data, p->len); > > @@ -350,7 +350,7 @@ static uint16_t memnic_xmit_pkts(void *tx_queue, > > uint64_t pkts, bytes, errs; > > uint32_t framesz = adapter->framesz; > > > > - if (!adapter->nic->hdr.valid) > > + if (unlikely(!adapter->nic->hdr.valid)) > > return 0; > > > > pkts = bytes = errs = 0; > > @@ -360,7 +360,7 @@ static uint16_t memnic_xmit_pkts(void *tx_queue, > > struct rte_mbuf *sg; > > void *ptr; > > > > - if (pkt_len > framesz) { > > + if (unlikely(pkt_len > framesz)) { > > errs++; > > break; > > } > > @@ -379,7 +379,7 @@ retry: > > goto retry; > > } > > > > - if (idx != ACCESS_ONCE(adapter->down_idx)) { > > + if (unlikely(idx != ACCESS_ONCE(adapter->down_idx))) { > > /* > > * host freed this and got false positive, > > * need to recover the status and retry. > > @@ -388,7 +388,7 @@ retry: > > goto retry; > > } > > > > - if (++idx >= MEMNIC_NR_PACKET) > > + if (unlikely(++idx >= MEMNIC_NR_PACKET)) > > idx = 0; > > adapter->down_idx = idx; > > > > -- > > 1.8.3.1
[dpdk-dev] [memnic PATCH v2 6/7] pmd: add branch hint in recv/xmit
Hi, > Subject: Re: [dpdk-dev] [memnic PATCH v2 6/7] pmd: add branch hint in > recv/xmit > > On Tue, Sep 30, 2014 at 11:14:40AM +, Hiroshi Shimamoto wrote: > > From: Hiroshi Shimamoto > > > > To reduce instruction cache miss, add branch condition hints into > > recv/xmit functions. This improves a bit performance. > > > > We can see performance improvements with memnic-tester. > > Using Xeon E5-2697 v2 @ 2.70GHz, 4 vCPU. > > size | before | after > >64 | 5.54Mpps | 5.55Mpps > > 128 | 5.46Mpps | 5.44Mpps > > 256 | 5.21Mpps | 5.22Mpps > > 512 | 4.50Mpps | 4.52Mpps > > 1024 | 3.71Mpps | 3.73Mpps > > 1280 | 3.21Mpps | 3.22Mpps > > 1518 | 2.92Mpps | 2.93Mpps > > > > Signed-off-by: Hiroshi Shimamoto > > Reviewed-by: Hayato Momma > > --- > > pmd/pmd_memnic.c | 18 +- > > 1 file changed, 9 insertions(+), 9 deletions(-) > > > > diff --git a/pmd/pmd_memnic.c b/pmd/pmd_memnic.c > > index 7fc3093..875d3ea 100644 > > --- a/pmd/pmd_memnic.c > > +++ b/pmd/pmd_memnic.c > > @@ -289,26 +289,26 @@ static uint16_t memnic_recv_pkts(void *rx_queue, > > int idx, next; > > struct rte_eth_stats *st = &adapter->stats[rte_lcore_id()]; > > > > - if (!adapter->nic->hdr.valid) > > + if (unlikely(!adapter->nic->hdr.valid)) > > return 0; > > > > pkts = bytes = errs = 0; > > idx = adapter->up_idx; > > for (nr = 0; nr < nb_pkts; nr++) { > > p = &data->packets[idx]; > > - if (p->status != MEMNIC_PKT_ST_FILLED) > > + if (unlikely(p->status != MEMNIC_PKT_ST_FILLED)) > > break; > > /* prefetch the next area */ > > next = idx; > > - if (++next >= MEMNIC_NR_PACKET) > > + if (unlikely(++next >= MEMNIC_NR_PACKET)) > > next = 0; > > rte_prefetch0(&data->packets[next]); > > - if (p->len > framesz) { > > + if (unlikely(p->len > framesz)) { > > errs++; > > goto drop; > > } > > mb = rte_pktmbuf_alloc(adapter->mp); > > - if (!mb) > > + if (unlikely(!mb)) > > break; > > > > rte_memcpy(rte_pktmbuf_mtod(mb, void *), p->data, p->len); > > @@ -350,7 +350,7 @@ static uint16_t memnic_xmit_pkts(void *tx_queue, > > uint64_t pkts, bytes, errs; > > uint32_t framesz = adapter->framesz; > > > > - if (!adapter->nic->hdr.valid) > > + if (unlikely(!adapter->nic->hdr.valid)) > > return 0; > > > > pkts = bytes = errs = 0; > > @@ -360,7 +360,7 @@ static uint16_t memnic_xmit_pkts(void *tx_queue, > > struct rte_mbuf *sg; > > void *ptr; > > > > - if (pkt_len > framesz) { > > + if (unlikely(pkt_len > framesz)) { > > errs++; > > break; > > } > > @@ -379,7 +379,7 @@ retry: > > goto retry; > > } > > > > - if (idx != ACCESS_ONCE(adapter->down_idx)) { > > + if (unlikely(idx != ACCESS_ONCE(adapter->down_idx))) { > Why are you using ACCESS_ONCE here? Or for that matter, anywhere else in this > PMD? The whole idea of the ACCESS_ONCE macro is to assign a value to a > variable > once and prevent it from getting reloaded from memory at a later time, this is > exactly contrary to that, both in the sense that you're explicitly reloading > the > same variable multiple times, and that you're using it as part of a comparison > operation, rather than an asignment operation ACCESS_ONCE prevents compiler optimization and ensures load from memory. There could be multiple threads which read/write that index. We should compare the value previous and the current value in memory. In that reason, I use ACCESS_ONCE macro to get value in the memory. thanks, Hiroshi > > Neil
[dpdk-dev] [memnic PATCH v2 6/7] pmd: add branch hint in recv/xmit
2014-09-30 14:38, Xie, Huawei: > > - if (++next >= MEMNIC_NR_PACKET) > > + if (unlikely(++next >= MEMNIC_NR_PACKET)) > > On IA, compiler can use add, cmp and cmov to avoid branch. > But If MEMNIC_NR_PACKET is always power of 2, > it is better just next = (next + 1) & (MEMNIC_NR_PACKET - 1) Power of 2 is not enforced for MEMNIC_NR_PACKET. -- Thomas
[dpdk-dev] [memnic PATCH v2 6/7] pmd: add branch hint in recv/xmit
> -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Hiroshi Shimamoto > Sent: Tuesday, September 30, 2014 7:15 PM > To: dev at dpdk.org > Cc: Hayato Momma > Subject: [dpdk-dev] [memnic PATCH v2 6/7] pmd: add branch hint in recv/xmit > > From: Hiroshi Shimamoto > > To reduce instruction cache miss, add branch condition hints into > recv/xmit functions. This improves a bit performance. > > We can see performance improvements with memnic-tester. > Using Xeon E5-2697 v2 @ 2.70GHz, 4 vCPU. > size | before | after >64 | 5.54Mpps | 5.55Mpps > 128 | 5.46Mpps | 5.44Mpps > 256 | 5.21Mpps | 5.22Mpps > 512 | 4.50Mpps | 4.52Mpps > 1024 | 3.71Mpps | 3.73Mpps > 1280 | 3.21Mpps | 3.22Mpps > 1518 | 2.92Mpps | 2.93Mpps > > Signed-off-by: Hiroshi Shimamoto > Reviewed-by: Hayato Momma > --- > pmd/pmd_memnic.c | 18 +- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/pmd/pmd_memnic.c b/pmd/pmd_memnic.c > index 7fc3093..875d3ea 100644 > --- a/pmd/pmd_memnic.c > +++ b/pmd/pmd_memnic.c > @@ -289,26 +289,26 @@ static uint16_t memnic_recv_pkts(void *rx_queue, > int idx, next; > struct rte_eth_stats *st = &adapter->stats[rte_lcore_id()]; > > - if (!adapter->nic->hdr.valid) > + if (unlikely(!adapter->nic->hdr.valid)) > return 0; > > pkts = bytes = errs = 0; > idx = adapter->up_idx; > for (nr = 0; nr < nb_pkts; nr++) { > p = &data->packets[idx]; > - if (p->status != MEMNIC_PKT_ST_FILLED) > + if (unlikely(p->status != MEMNIC_PKT_ST_FILLED)) > break; > /* prefetch the next area */ > next = idx; > - if (++next >= MEMNIC_NR_PACKET) > + if (unlikely(++next >= MEMNIC_NR_PACKET)) On IA, compiler can use add, cmp and cmov to avoid branch. But If MEMNIC_NR_PACKET is always power of 2, it is better just next = (next + 1) & (MEMNIC_NR_PACKET - 1) > next = 0; > rte_prefetch0(&data->packets[next]); > - if (p->len > framesz) { > + if (unlikely(p->len > framesz)) { > errs++; > goto drop; > } > mb = rte_pktmbuf_alloc(adapter->mp); > - if (!mb) > + if (unlikely(!mb)) > break; > > rte_memcpy(rte_pktmbuf_mtod(mb, void *), p->data, p->len); > @@ -350,7 +350,7 @@ static uint16_t memnic_xmit_pkts(void *tx_queue, > uint64_t pkts, bytes, errs; > uint32_t framesz = adapter->framesz; > > - if (!adapter->nic->hdr.valid) > + if (unlikely(!adapter->nic->hdr.valid)) > return 0; > > pkts = bytes = errs = 0; > @@ -360,7 +360,7 @@ static uint16_t memnic_xmit_pkts(void *tx_queue, > struct rte_mbuf *sg; > void *ptr; > > - if (pkt_len > framesz) { > + if (unlikely(pkt_len > framesz)) { > errs++; > break; > } > @@ -379,7 +379,7 @@ retry: > goto retry; > } > > - if (idx != ACCESS_ONCE(adapter->down_idx)) { > + if (unlikely(idx != ACCESS_ONCE(adapter->down_idx))) { > /* >* host freed this and got false positive, >* need to recover the status and retry. > @@ -388,7 +388,7 @@ retry: > goto retry; > } > > - if (++idx >= MEMNIC_NR_PACKET) > + if (unlikely(++idx >= MEMNIC_NR_PACKET)) > idx = 0; > adapter->down_idx = idx; > > -- > 1.8.3.1
[dpdk-dev] [memnic PATCH v2 6/7] pmd: add branch hint in recv/xmit
The patch is ok. For the commit message, is it better "to reduce branch mispredication"? > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Hiroshi Shimamoto > Sent: Tuesday, September 30, 2014 7:15 PM > To: dev at dpdk.org > Cc: Hayato Momma > Subject: [dpdk-dev] [memnic PATCH v2 6/7] pmd: add branch hint in recv/xmit > > From: Hiroshi Shimamoto > > To reduce instruction cache miss, add branch condition hints into > recv/xmit functions. This improves a bit performance. > > We can see performance improvements with memnic-tester. > Using Xeon E5-2697 v2 @ 2.70GHz, 4 vCPU. > size | before | after >64 | 5.54Mpps | 5.55Mpps > 128 | 5.46Mpps | 5.44Mpps > 256 | 5.21Mpps | 5.22Mpps > 512 | 4.50Mpps | 4.52Mpps > 1024 | 3.71Mpps | 3.73Mpps > 1280 | 3.21Mpps | 3.22Mpps > 1518 | 2.92Mpps | 2.93Mpps > > Signed-off-by: Hiroshi Shimamoto > Reviewed-by: Hayato Momma > --- > pmd/pmd_memnic.c | 18 +- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/pmd/pmd_memnic.c b/pmd/pmd_memnic.c > index 7fc3093..875d3ea 100644 > --- a/pmd/pmd_memnic.c > +++ b/pmd/pmd_memnic.c > @@ -289,26 +289,26 @@ static uint16_t memnic_recv_pkts(void *rx_queue, > int idx, next; > struct rte_eth_stats *st = &adapter->stats[rte_lcore_id()]; > > - if (!adapter->nic->hdr.valid) > + if (unlikely(!adapter->nic->hdr.valid)) > return 0; > > pkts = bytes = errs = 0; > idx = adapter->up_idx; > for (nr = 0; nr < nb_pkts; nr++) { > p = &data->packets[idx]; > - if (p->status != MEMNIC_PKT_ST_FILLED) > + if (unlikely(p->status != MEMNIC_PKT_ST_FILLED)) > break; > /* prefetch the next area */ > next = idx; > - if (++next >= MEMNIC_NR_PACKET) > + if (unlikely(++next >= MEMNIC_NR_PACKET)) > next = 0; > rte_prefetch0(&data->packets[next]); > - if (p->len > framesz) { > + if (unlikely(p->len > framesz)) { > errs++; > goto drop; > } > mb = rte_pktmbuf_alloc(adapter->mp); > - if (!mb) > + if (unlikely(!mb)) > break; > > rte_memcpy(rte_pktmbuf_mtod(mb, void *), p->data, p->len); > @@ -350,7 +350,7 @@ static uint16_t memnic_xmit_pkts(void *tx_queue, > uint64_t pkts, bytes, errs; > uint32_t framesz = adapter->framesz; > > - if (!adapter->nic->hdr.valid) > + if (unlikely(!adapter->nic->hdr.valid)) > return 0; > > pkts = bytes = errs = 0; > @@ -360,7 +360,7 @@ static uint16_t memnic_xmit_pkts(void *tx_queue, > struct rte_mbuf *sg; > void *ptr; > > - if (pkt_len > framesz) { > + if (unlikely(pkt_len > framesz)) { > errs++; > break; > } > @@ -379,7 +379,7 @@ retry: > goto retry; > } > > - if (idx != ACCESS_ONCE(adapter->down_idx)) { > + if (unlikely(idx != ACCESS_ONCE(adapter->down_idx))) { > /* >* host freed this and got false positive, >* need to recover the status and retry. > @@ -388,7 +388,7 @@ retry: > goto retry; > } > > - if (++idx >= MEMNIC_NR_PACKET) > + if (unlikely(++idx >= MEMNIC_NR_PACKET)) > idx = 0; > adapter->down_idx = idx; > > -- > 1.8.3.1
[dpdk-dev] [memnic PATCH v2 6/7] pmd: add branch hint in recv/xmit
From: Hiroshi Shimamoto To reduce instruction cache miss, add branch condition hints into recv/xmit functions. This improves a bit performance. We can see performance improvements with memnic-tester. Using Xeon E5-2697 v2 @ 2.70GHz, 4 vCPU. size | before | after 64 | 5.54Mpps | 5.55Mpps 128 | 5.46Mpps | 5.44Mpps 256 | 5.21Mpps | 5.22Mpps 512 | 4.50Mpps | 4.52Mpps 1024 | 3.71Mpps | 3.73Mpps 1280 | 3.21Mpps | 3.22Mpps 1518 | 2.92Mpps | 2.93Mpps Signed-off-by: Hiroshi Shimamoto Reviewed-by: Hayato Momma --- pmd/pmd_memnic.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/pmd/pmd_memnic.c b/pmd/pmd_memnic.c index 7fc3093..875d3ea 100644 --- a/pmd/pmd_memnic.c +++ b/pmd/pmd_memnic.c @@ -289,26 +289,26 @@ static uint16_t memnic_recv_pkts(void *rx_queue, int idx, next; struct rte_eth_stats *st = &adapter->stats[rte_lcore_id()]; - if (!adapter->nic->hdr.valid) + if (unlikely(!adapter->nic->hdr.valid)) return 0; pkts = bytes = errs = 0; idx = adapter->up_idx; for (nr = 0; nr < nb_pkts; nr++) { p = &data->packets[idx]; - if (p->status != MEMNIC_PKT_ST_FILLED) + if (unlikely(p->status != MEMNIC_PKT_ST_FILLED)) break; /* prefetch the next area */ next = idx; - if (++next >= MEMNIC_NR_PACKET) + if (unlikely(++next >= MEMNIC_NR_PACKET)) next = 0; rte_prefetch0(&data->packets[next]); - if (p->len > framesz) { + if (unlikely(p->len > framesz)) { errs++; goto drop; } mb = rte_pktmbuf_alloc(adapter->mp); - if (!mb) + if (unlikely(!mb)) break; rte_memcpy(rte_pktmbuf_mtod(mb, void *), p->data, p->len); @@ -350,7 +350,7 @@ static uint16_t memnic_xmit_pkts(void *tx_queue, uint64_t pkts, bytes, errs; uint32_t framesz = adapter->framesz; - if (!adapter->nic->hdr.valid) + if (unlikely(!adapter->nic->hdr.valid)) return 0; pkts = bytes = errs = 0; @@ -360,7 +360,7 @@ static uint16_t memnic_xmit_pkts(void *tx_queue, struct rte_mbuf *sg; void *ptr; - if (pkt_len > framesz) { + if (unlikely(pkt_len > framesz)) { errs++; break; } @@ -379,7 +379,7 @@ retry: goto retry; } - if (idx != ACCESS_ONCE(adapter->down_idx)) { + if (unlikely(idx != ACCESS_ONCE(adapter->down_idx))) { /* * host freed this and got false positive, * need to recover the status and retry. @@ -388,7 +388,7 @@ retry: goto retry; } - if (++idx >= MEMNIC_NR_PACKET) + if (unlikely(++idx >= MEMNIC_NR_PACKET)) idx = 0; adapter->down_idx = idx; -- 1.8.3.1
[dpdk-dev] [memnic PATCH v2 6/7] pmd: add branch hint in recv/xmit
On Tue, Sep 30, 2014 at 11:14:40AM +, Hiroshi Shimamoto wrote: > From: Hiroshi Shimamoto > > To reduce instruction cache miss, add branch condition hints into > recv/xmit functions. This improves a bit performance. > > We can see performance improvements with memnic-tester. > Using Xeon E5-2697 v2 @ 2.70GHz, 4 vCPU. > size | before | after >64 | 5.54Mpps | 5.55Mpps > 128 | 5.46Mpps | 5.44Mpps > 256 | 5.21Mpps | 5.22Mpps > 512 | 4.50Mpps | 4.52Mpps > 1024 | 3.71Mpps | 3.73Mpps > 1280 | 3.21Mpps | 3.22Mpps > 1518 | 2.92Mpps | 2.93Mpps > > Signed-off-by: Hiroshi Shimamoto > Reviewed-by: Hayato Momma > --- > pmd/pmd_memnic.c | 18 +- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/pmd/pmd_memnic.c b/pmd/pmd_memnic.c > index 7fc3093..875d3ea 100644 > --- a/pmd/pmd_memnic.c > +++ b/pmd/pmd_memnic.c > @@ -289,26 +289,26 @@ static uint16_t memnic_recv_pkts(void *rx_queue, > int idx, next; > struct rte_eth_stats *st = &adapter->stats[rte_lcore_id()]; > > - if (!adapter->nic->hdr.valid) > + if (unlikely(!adapter->nic->hdr.valid)) > return 0; > > pkts = bytes = errs = 0; > idx = adapter->up_idx; > for (nr = 0; nr < nb_pkts; nr++) { > p = &data->packets[idx]; > - if (p->status != MEMNIC_PKT_ST_FILLED) > + if (unlikely(p->status != MEMNIC_PKT_ST_FILLED)) > break; > /* prefetch the next area */ > next = idx; > - if (++next >= MEMNIC_NR_PACKET) > + if (unlikely(++next >= MEMNIC_NR_PACKET)) > next = 0; > rte_prefetch0(&data->packets[next]); > - if (p->len > framesz) { > + if (unlikely(p->len > framesz)) { > errs++; > goto drop; > } > mb = rte_pktmbuf_alloc(adapter->mp); > - if (!mb) > + if (unlikely(!mb)) > break; > > rte_memcpy(rte_pktmbuf_mtod(mb, void *), p->data, p->len); > @@ -350,7 +350,7 @@ static uint16_t memnic_xmit_pkts(void *tx_queue, > uint64_t pkts, bytes, errs; > uint32_t framesz = adapter->framesz; > > - if (!adapter->nic->hdr.valid) > + if (unlikely(!adapter->nic->hdr.valid)) > return 0; > > pkts = bytes = errs = 0; > @@ -360,7 +360,7 @@ static uint16_t memnic_xmit_pkts(void *tx_queue, > struct rte_mbuf *sg; > void *ptr; > > - if (pkt_len > framesz) { > + if (unlikely(pkt_len > framesz)) { > errs++; > break; > } > @@ -379,7 +379,7 @@ retry: > goto retry; > } > > - if (idx != ACCESS_ONCE(adapter->down_idx)) { > + if (unlikely(idx != ACCESS_ONCE(adapter->down_idx))) { Why are you using ACCESS_ONCE here? Or for that matter, anywhere else in this PMD? The whole idea of the ACCESS_ONCE macro is to assign a value to a variable once and prevent it from getting reloaded from memory at a later time, this is exactly contrary to that, both in the sense that you're explicitly reloading the same variable multiple times, and that you're using it as part of a comparison operation, rather than an asignment operation Neil