diff mbox

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

Message ID 1462176465-21562-1-git-send-email-jerin.jacob@caviumnetworks.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers

Commit Message

Jerin Jacob May 2, 2016, 8:07 a.m. UTC 
  Elements of struct rte_eth_dev used in the fast path.
Make struct rte_eth_dev cache aligned to avoid the cases where
rte_eth_dev elements share the same cache line with other structures.

Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
---
 lib/librte_ether/rte_ethdev.c | 2 +-
 lib/librte_ether/rte_ethdev.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Bruce Richardson May 3, 2016, 9:40 a.m. UTC | #1 
On Mon, May 02, 2016 at 01:37:45PM +0530, Jerin Jacob wrote:
> Elements of struct rte_eth_dev used in the fast path.
> Make struct rte_eth_dev cache aligned to avoid the cases where
> rte_eth_dev elements share the same cache line with other structures.
> 
> Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> ---
>  lib/librte_ether/rte_ethdev.c | 2 +-
>  lib/librte_ether/rte_ethdev.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index a31018e..04f492d 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -70,7 +70,7 @@
>  #include "rte_ethdev.h"
>  
>  static const char *MZ_RTE_ETH_DEV_DATA = "rte_eth_dev_data";
> -struct rte_eth_dev rte_eth_devices[RTE_MAX_ETHPORTS];
> +struct rte_eth_dev rte_eth_devices[RTE_MAX_ETHPORTS] __rte_cache_aligned;
>  static struct rte_eth_dev_data *rte_eth_dev_data;
>  static uint8_t nb_ports;
>  
> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> index 8519ff6..e359dda 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -1630,7 +1630,7 @@ struct rte_eth_dev {
>  	struct rte_eth_rxtx_callback *pre_tx_burst_cbs[RTE_MAX_QUEUES_PER_PORT];
>  	uint8_t attached; /**< Flag indicating the port is attached */
>  	enum rte_eth_dev_type dev_type; /**< Flag indicating the device type */
> -};
> +} __rte_cache_aligned;
>  
>  struct rte_eth_dev_sriov {
>  	uint8_t active;               /**< SRIOV is active with 16, 32 or 64 pools */
> -- 
Adding cache aligned in two places is overkill, I think. If the structure is
marked as being cache aligned, there is no need to have the rte_eth_devices
marked that way too. I suggest therefore just keeping the structure alignment.

Regards,
/Bruce
Jerin Jacob May 3, 2016, 12:10 p.m. UTC | #2 
On Tue, May 03, 2016 at 10:40:53AM +0100, Bruce Richardson wrote:
> On Mon, May 02, 2016 at 01:37:45PM +0530, Jerin Jacob wrote:
> > Elements of struct rte_eth_dev used in the fast path.
> > Make struct rte_eth_dev cache aligned to avoid the cases where
> > rte_eth_dev elements share the same cache line with other structures.
> > 
> > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > ---
> >  lib/librte_ether/rte_ethdev.c | 2 +-
> >  lib/librte_ether/rte_ethdev.h | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> > index a31018e..04f492d 100644
> > --- a/lib/librte_ether/rte_ethdev.c
> > +++ b/lib/librte_ether/rte_ethdev.c
> > @@ -70,7 +70,7 @@
> >  #include "rte_ethdev.h"
> >  
> >  static const char *MZ_RTE_ETH_DEV_DATA = "rte_eth_dev_data";
> > -struct rte_eth_dev rte_eth_devices[RTE_MAX_ETHPORTS];
> > +struct rte_eth_dev rte_eth_devices[RTE_MAX_ETHPORTS] __rte_cache_aligned;
> >  static struct rte_eth_dev_data *rte_eth_dev_data;
> >  static uint8_t nb_ports;
> >  
> > diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> > index 8519ff6..e359dda 100644
> > --- a/lib/librte_ether/rte_ethdev.h
> > +++ b/lib/librte_ether/rte_ethdev.h
> > @@ -1630,7 +1630,7 @@ struct rte_eth_dev {
> >  	struct rte_eth_rxtx_callback *pre_tx_burst_cbs[RTE_MAX_QUEUES_PER_PORT];
> >  	uint8_t attached; /**< Flag indicating the port is attached */
> >  	enum rte_eth_dev_type dev_type; /**< Flag indicating the device type */
> > -};
> > +} __rte_cache_aligned;
> >  
> >  struct rte_eth_dev_sriov {
> >  	uint8_t active;               /**< SRIOV is active with 16, 32 or 64 pools */
> > -- 
> Adding cache aligned in two places is overkill, I think. If the structure is
> marked as being cache aligned, there is no need to have the rte_eth_devices
> marked that way too. I suggest therefore just keeping the structure alignment.

OK. Will fix it in v2

> 
> Regards,
> /Bruce
diff mbox

Patch

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index a31018e..04f492d 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -70,7 +70,7 @@ 
 #include "rte_ethdev.h"
 
 static const char *MZ_RTE_ETH_DEV_DATA = "rte_eth_dev_data";
-struct rte_eth_dev rte_eth_devices[RTE_MAX_ETHPORTS];
+struct rte_eth_dev rte_eth_devices[RTE_MAX_ETHPORTS] __rte_cache_aligned;
 static struct rte_eth_dev_data *rte_eth_dev_data;
 static uint8_t nb_ports;
 
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 8519ff6..e359dda 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1630,7 +1630,7 @@  struct rte_eth_dev {
 	struct rte_eth_rxtx_callback *pre_tx_burst_cbs[RTE_MAX_QUEUES_PER_PORT];
 	uint8_t attached; /**< Flag indicating the port is attached */
 	enum rte_eth_dev_type dev_type; /**< Flag indicating the device type */
-};
+} __rte_cache_aligned;
 
 struct rte_eth_dev_sriov {
 	uint8_t active;               /**< SRIOV is active with 16, 32 or 64 pools */