Re: [dpdk-dev] [PATCH v2] ethdev: make struct rte_eth_dev cache aligned

[Bruce Richardson <bruce.richardson@intel.com>]

On Wed, May 04, 2016 at 08:49:38PM +0530, Jerin Jacob wrote:
> On Wed, May 04, 2016 at 01:53:39PM +0000, Richardson, Bruce wrote:
> > 
> > 
> > > -----Original Message-----
> > > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> > > Sent: Wednesday, May 4, 2016 2:43 PM
> > > To: Richardson, Bruce <bruce.richardson@intel.com>
> > > Cc: dev@dpdk.org; thomas.monjalon@6wind.com
> > > Subject: Re: [dpdk-dev] [PATCH v2] ethdev: make struct rte_eth_dev cache
> > > aligned
> > > 
> > > On Wed, May 04, 2016 at 12:09:50PM +0100, Bruce Richardson wrote:
> 
> Snip
> 
> > > >
> > > > Hi Jerin,
> > > 
> > > Hi Bruce,
> > > 
> > > >
> > > > have you seen a performance degradation due to ethdev elements sharing
> > > > a cache
> > > 
> > > No. Not because of sharing the cache line.
> > > 
> > > > line? I ask because, surprisingly for me, I actually see a performance
> > > > regression
> > > 
> > > I see performance degradation in PMD in my setup where independent changes
> > > are causing the performance issue in PMD(~<100k). That's the reason I
> > > thought making aligned cache line stuff where ever it makes sense so that
> > > independent change shouldn't impact the PMD performance and this patch was
> > > an initiative for the same.
> > > 
> > > > when I apply the above patch. It's not a big change - perf reduction
> > > > of <1% - but still noticable across multiple runs using testpmd. I'm
> > > > using two 1x40G NICs using i40e driver, and I see ~100kpps less
> > > > traffic per port after applying the patch. [CPU: Intel(R) Xeon(R) CPU
> > > > E5-2699 v3 @ 2.30GHz]
> > > 
> > > This particular patch does not have any performance degradation in my
> > > setup.
> > > CPU: ThunderX
> > 
> > Ok, so I take it that this patch is performance neutral on your setup, then?
> > If that's the case, can we hold off on merging it on the basis that it's not needed and does cause a slight regression.
> 
> OK
> 
> Can you share the base dpdk.org upstream change set where this patch
> creates the slight regression? I will test it the same on IA and
> ThunderX platform.
> 
> In my testpmd build, rte_eth_devices(0x0000000000751ef8) share the cache
> line with inactive "notify_ops" that the reason for its not creating any benefit.
> I guess the case would have been different if its active write location.
> 
>  COMMON         0x0000000000751ef0        0x8
> /home/jerin/dpdk-thunderx/build/lib/librte_vhost.a(virtio-net.o)
>                 0x0000000000751ef0                notify_ops
>  COMMON         0x0000000000751ef8    0x80900
> /home/jerin/dpdk-thunderx/build/lib/libethdev.a(rte_ethdev.o)
>                 0x0000000000751ef8                rte_eth_devices
> 
> Thanks,
> Jerin
> 
> > 
Hi Jerin,

I see no change with the latest code on dpdk.org mainline. I do see a change though,
with the latest code on the dpdk-next-net/rel_16_07 branch, which has additional
i40e driver improvements on it.
However, on re-running the tests, I realise I may have got my measurement results
backwards and that this patch may actually cause a slight increase instead. :-(

Therefore, I withdraw my current objections to this patch. But I would like to
see someone else verify what performance difference, if any, this causes on
different platforms, so your planned tests on ThunderX and IA would still be
great to see! :-)

Thanks,
/Bruce