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

[Jerin Jacob <jerin.jacob@caviumnetworks.com>]

-----Original Message-----
> Date: Mon, 12 Feb 2018 09:04:07 +0000
> From: Matan Azrad <matan@mellanox.com>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>, "dev@dpdk.org"
>  <dev@dpdk.org>
> CC: "ferruh.yigit@intel.com" <ferruh.yigit@intel.com>, Thomas Monjalon
>  <thomas@monjalon.net>, Konstantin Ananyev <konstantin.ananyev@intel.com>,
>  Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> Subject: RE: [dpdk-dev] [PATCH v2] ethdev: make ethdev data cache aligned
> 
> Hi Jerin
> 
>  From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> > Since struct rte_eth_dev_data used in the fast path, making it as cache
> > aligned.
> > 
> > Fixes: af75078fece3 ("first public release")
> > Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
> 
> Looks like it is just improvement.
> No need the above "fixes" lines (also fix title is not needed as you did).

I think, It varies the way we look at it. I don't think, either way it
matters in the commit log.

See below,

> 
> I think that performance improvement results should be added to the commit log.

I added following under comment section. Do you this want to move git commit
message ? If so, I can send the v3.

 - Some platform like thunderx + l3fwd showed 1% regression in the
 performance with 5b7ba31148a8 ("ethdev: add port ownership") in one port
 setup.

> 
> Moreover, Did you investigate which fields in rte_eth_dev_data structures are important for performance and should not be in a different cache lines?

No. That can be separate patch.

> Maybe alternative order of the fields in the structure may improve the performance more... 

Maybe.

>  
> > Cc: Matan Azrad <matan@mellanox.com>
> > Cc: Thomas Monjalon <thomas@monjalon.net>
> > Cc: Konstantin Ananyev <konstantin.ananyev@intel.com>
> > 
> > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > Signed-off-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> > ---
> > v2:
> > - Change the git comments based on Matan's feedback
> > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdp
> > dk.org%2Fdev%2Fpatchwork%2Fpatch%2F35104%2F&data=02%7C01%7Cmat
> > an%40mellanox.com%7C5c2537b12e6d4e51f12a08d571dd33a2%7Ca652971c7
> > d2e4d9ba6a4d149256f461b%7C0%7C0%7C636540117238324576&sdata=8OOg
> > Zb0KzDbBce9xPVywV8ynmiKP9B%2BbYsQxgE5VlX0%3D&reserved=0
> > 
> > - Some platform like thunderx + l3fwd showed 1% regression in the
> > performance with 5b7ba31148a8 ("ethdev: add port ownership") in one port
> > setup.
> > 
> > - If there are no objection for this change then request to take it for v18.02
> > release.
> > ---
> >  lib/librte_ether/rte_ethdev_core.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/lib/librte_ether/rte_ethdev_core.h
> > b/lib/librte_ether/rte_ethdev_core.h
> > index 315b31723..e5681e466 100644
> > --- a/lib/librte_ether/rte_ethdev_core.h
> > +++ b/lib/librte_ether/rte_ethdev_core.h
> > @@ -601,7 +601,7 @@ struct rte_eth_dev_data {
> >  	struct rte_vlan_filter_conf vlan_filter_conf;
> >  	/**< VLAN filter configuration. */
> >  	struct rte_eth_dev_owner owner; /**< The port owner. */ -};
> > +} __rte_cache_aligned;
> > 
> >  /**
> >   * @internal
> > --
> > 2.16.1
>