diff mbox

[dpdk-dev] ethdev: fix a regression due to cache alignment issue

Message ID 20180210094220.16201-1-jerin.jacob@caviumnetworks.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Jerin Jacob Feb. 10, 2018, 9:42 a.m. UTC 
  Prior to "ethdev: add port ownership" change, the fast
path structure struct rte_eth_dev->data was cache aligned
due to the fact that eth_dev_data was allocated from
rte_malloc with cache aligned attribute.
"ethdev: add port ownership" change set introduced a
rte_eth_dev_shared_data container for port ownership change,
This resulted in rte_eth_dev->data memory as cache unaligned.
Added a compiler alignment attribute to make sure
rte_eth_dev->data always cache aligned and hence compiler
can load/store the elements in struct rte_eth_dev_data
as naturally aligned.

Some platform like thunderx + l3fwd showed 1% regression in
the performance with the offending changeset.

Fixes: 5b7ba31148a8 ("ethdev: add port ownership")

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>
---
 lib/librte_ether/rte_ethdev_core.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Matan Azrad Feb. 10, 2018, 6:23 p.m. UTC | #1 
Hi Jerin

From: Jerin Jacob,Sent: Saturday, February 10, 2018 11:42 AM
> Prior to "ethdev: add port ownership" change, the fast path structure struct
> rte_eth_dev->data was cache aligned due to the fact that eth_dev_data was
> allocated from rte_malloc with cache aligned attribute.
> "ethdev: add port ownership" change set introduced a
> rte_eth_dev_shared_data container for port ownership change, This
> resulted in rte_eth_dev->data memory as cache unaligned.
> Added a compiler alignment attribute to make sure rte_eth_dev->data
> always cache aligned and hence compiler can load/store the elements in
> struct rte_eth_dev_data as naturally aligned.
> 
> Some platform like thunderx + l3fwd showed 1% regression in the
> performance with the offending changeset.
> 
> Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
> 

Are you sure the previous port data allocation(before port ownership) was cache aligned for all the ports for every system (cache line size 16\32\64\128...) or maybe it was only aligned to the first port (port_id=0)?

I think that if the answer is no, this fix fixes early patch, or just improvement. 

> 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>
> ---
>  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
Jerin Jacob Feb. 12, 2018, 5:33 a.m. UTC | #2 
-----Original Message-----
> Date: Sat, 10 Feb 2018 18:23:42 +0000
> From: Matan Azrad <matan@mellanox.com>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>, "dev@dpdk.org"
>  <dev@dpdk.org>
> CC: Thomas Monjalon <thomas@monjalon.net>, Konstantin Ananyev
>  <konstantin.ananyev@intel.com>, Pavan Nikhilesh
>  <pbhagavatula@caviumnetworks.com>
> Subject: RE: [dpdk-dev] [PATCH] ethdev: fix a regression due to cache
>  alignment issue
> 
> Hi Jerin

Hi Matan,

> 
> From: Jerin Jacob,Sent: Saturday, February 10, 2018 11:42 AM
> > Prior to "ethdev: add port ownership" change, the fast path structure struct
> > rte_eth_dev->data was cache aligned due to the fact that eth_dev_data was
> > allocated from rte_malloc with cache aligned attribute.
> > "ethdev: add port ownership" change set introduced a
> > rte_eth_dev_shared_data container for port ownership change, This
> > resulted in rte_eth_dev->data memory as cache unaligned.
> > Added a compiler alignment attribute to make sure rte_eth_dev->data
> > always cache aligned and hence compiler can load/store the elements in
> > struct rte_eth_dev_data as naturally aligned.
> > 
> > Some platform like thunderx + l3fwd showed 1% regression in the
> > performance with the offending changeset.
> > 
> > Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
> > 
> 
> Are you sure the previous port data allocation(before port ownership) was cache aligned for all the ports for every system (cache line size 16\32\64\128...) or maybe it was only aligned to the first port (port_id=0)?

Yes, It was aligned to only for the first port.

> 
> I think that if the answer is no, this fix fixes early patch, or just improvement. 

I will send v2 as fixes early patch/improvement.

> 
> > 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>
> > ---
> >  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
>
diff mbox

Patch

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