[dpdk-dev,v2,4/4] cache/slow-path: reduce cache align requirement for 128-byte cache targets

Message ID 1449765378-29563-5-git-send-email-jerin.jacob@caviumnetworks.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Jerin Jacob Dec. 10, 2015, 4:36 p.m. UTC
  slow-path data structures need not be 128-byte cache aligned.
Reduce the alignment to 64-byte to save the memory.

No behavior change for 64-byte cache aligned systems as minimum
cache line size as 64.

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

Comments

Ananyev, Konstantin Dec. 11, 2015, 12:55 p.m. UTC | #1
Hi Jerin,

> -----Original Message-----
> From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> Sent: Thursday, December 10, 2015 4:36 PM
> To: dev@dpdk.org
> Cc: thomas.monjalon@6wind.com; Ananyev, Konstantin; viktorin@rehivetech.com; jianbo.liu@linaro.org; Jerin Jacob
> Subject: [dpdk-dev] [PATCH v2 4/4] cache/slow-path: reduce cache align requirement for 128-byte cache targets
> 
> slow-path data structures need not be 128-byte cache aligned.
> Reduce the alignment to 64-byte to save the memory.
> 
> No behavior change for 64-byte cache aligned systems as minimum
> cache line size as 64.
> 
> Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> ---
>  lib/librte_ether/rte_ethdev.h    | 4 ++--
>  lib/librte_mempool/rte_mempool.h | 2 +-
>  lib/librte_ring/rte_ring.h       | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> index bada8ad..4dbf73b 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -863,7 +863,7 @@ struct rte_eth_rxq_info {
>  	struct rte_eth_rxconf conf; /**< queue config parameters. */
>  	uint8_t scattered_rx;       /**< scattered packets RX supported. */
>  	uint16_t nb_desc;           /**< configured number of RXDs. */
> -} __rte_cache_aligned;
> +} __rte_cache_min_aligned;
> 
>  /**
>   * Ethernet device TX queue information structure.
> @@ -872,7 +872,7 @@ struct rte_eth_rxq_info {
>  struct rte_eth_txq_info {
>  	struct rte_eth_txconf conf; /**< queue config parameters. */
>  	uint16_t nb_desc;           /**< configured number of TXDs. */
> -} __rte_cache_aligned;
> +} __rte_cache_min_aligned;
> 
>  /** Maximum name length for extended statistics counters */
>  #define RTE_ETH_XSTATS_NAME_SIZE 64
> diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_mempool.h
> index 6e2390a..8e5d10c 100644
> --- a/lib/librte_mempool/rte_mempool.h
> +++ b/lib/librte_mempool/rte_mempool.h
> @@ -92,7 +92,7 @@ struct rte_mempool_debug_stats {
>  	uint64_t get_success_objs; /**< Objects successfully allocated. */
>  	uint64_t get_fail_bulk;    /**< Failed allocation number. */
>  	uint64_t get_fail_objs;    /**< Objects that failed to be allocated. */
> -} __rte_cache_aligned;
> +} __rte_cache_min_aligned;
>  #endif
> 
>  #if RTE_MEMPOOL_CACHE_MAX_SIZE > 0
> diff --git a/lib/librte_ring/rte_ring.h b/lib/librte_ring/rte_ring.h
> index de036ce..33166aa 100644
> --- a/lib/librte_ring/rte_ring.h
> +++ b/lib/librte_ring/rte_ring.h
> @@ -123,7 +123,7 @@ struct rte_ring_debug_stats {
>  	uint64_t deq_success_objs; /**< Objects successfully dequeued. */
>  	uint64_t deq_fail_bulk;    /**< Failed dequeues number. */
>  	uint64_t deq_fail_objs;    /**< Objects that failed to be dequeued. */
> -} __rte_cache_aligned;
> +} __rte_cache_min_aligned;
>  #endif

I think we better keep both struct rte_ring_debug_stats and rte_mempool_debug_stats
as __rte_cache_aligned.
Both are on a  per core basis and can be used at run-time
(when RTE_LIBRTE_RING_DEBUG/RTE_LIBRTE_MEMPOOL_DEBUG=y),
and not supposed to be shared by different cores. 
All other things in the series look good to me.
BTW, by some reason I can't find that series in the patchworks.
Konstantin


> 
>  #define RTE_RING_NAMESIZE 32 /**< The maximum length of a ring name. */
> --
> 2.1.0
  
Thomas Monjalon Dec. 11, 2015, 1:07 p.m. UTC | #2
2015-12-11 12:55, Ananyev, Konstantin:
> BTW, by some reason I can't find that series in the patchworks.

Because it is deferred:
	http://dpdk.org/dev/patchwork/project/dpdk/list/?state=10

It's too late to do this change in 2.2.
  
Jerin Jacob Dec. 11, 2015, 1:56 p.m. UTC | #3
On Fri, Dec 11, 2015 at 12:55:57PM +0000, Ananyev, Konstantin wrote:
> Hi Jerin,
>
> > -----Original Message-----
> > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> > Sent: Thursday, December 10, 2015 4:36 PM
> > To: dev@dpdk.org
> > Cc: thomas.monjalon@6wind.com; Ananyev, Konstantin; viktorin@rehivetech.com; jianbo.liu@linaro.org; Jerin Jacob
> > Subject: [dpdk-dev] [PATCH v2 4/4] cache/slow-path: reduce cache align requirement for 128-byte cache targets
> >
> > slow-path data structures need not be 128-byte cache aligned.
> > Reduce the alignment to 64-byte to save the memory.
> >
> > No behavior change for 64-byte cache aligned systems as minimum
> > cache line size as 64.
> >
> > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > ---
> >  lib/librte_ether/rte_ethdev.h    | 4 ++--
> >  lib/librte_mempool/rte_mempool.h | 2 +-
> >  lib/librte_ring/rte_ring.h       | 2 +-
> >  3 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> > index bada8ad..4dbf73b 100644
> > --- a/lib/librte_ether/rte_ethdev.h
> > +++ b/lib/librte_ether/rte_ethdev.h
> > @@ -863,7 +863,7 @@ struct rte_eth_rxq_info {
> >  	struct rte_eth_rxconf conf; /**< queue config parameters. */
> >  	uint8_t scattered_rx;       /**< scattered packets RX supported. */
> >  	uint16_t nb_desc;           /**< configured number of RXDs. */
> > -} __rte_cache_aligned;
> > +} __rte_cache_min_aligned;
> >
> >  /**
> >   * Ethernet device TX queue information structure.
> > @@ -872,7 +872,7 @@ struct rte_eth_rxq_info {
> >  struct rte_eth_txq_info {
> >  	struct rte_eth_txconf conf; /**< queue config parameters. */
> >  	uint16_t nb_desc;           /**< configured number of TXDs. */
> > -} __rte_cache_aligned;
> > +} __rte_cache_min_aligned;
> >
> >  /** Maximum name length for extended statistics counters */
> >  #define RTE_ETH_XSTATS_NAME_SIZE 64
> > diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_mempool.h
> > index 6e2390a..8e5d10c 100644
> > --- a/lib/librte_mempool/rte_mempool.h
> > +++ b/lib/librte_mempool/rte_mempool.h
> > @@ -92,7 +92,7 @@ struct rte_mempool_debug_stats {
> >  	uint64_t get_success_objs; /**< Objects successfully allocated. */
> >  	uint64_t get_fail_bulk;    /**< Failed allocation number. */
> >  	uint64_t get_fail_objs;    /**< Objects that failed to be allocated. */
> > -} __rte_cache_aligned;
> > +} __rte_cache_min_aligned;
> >  #endif
> >
> >  #if RTE_MEMPOOL_CACHE_MAX_SIZE > 0
> > diff --git a/lib/librte_ring/rte_ring.h b/lib/librte_ring/rte_ring.h
> > index de036ce..33166aa 100644
> > --- a/lib/librte_ring/rte_ring.h
> > +++ b/lib/librte_ring/rte_ring.h
> > @@ -123,7 +123,7 @@ struct rte_ring_debug_stats {
> >  	uint64_t deq_success_objs; /**< Objects successfully dequeued. */
> >  	uint64_t deq_fail_bulk;    /**< Failed dequeues number. */
> >  	uint64_t deq_fail_objs;    /**< Objects that failed to be dequeued. */
> > -} __rte_cache_aligned;
> > +} __rte_cache_min_aligned;
> >  #endif
>
> I think we better keep both struct rte_ring_debug_stats and rte_mempool_debug_stats
> as __rte_cache_aligned.
> Both are on a  per core basis and can be used at run-time
> (when RTE_LIBRTE_RING_DEBUG/RTE_LIBRTE_MEMPOOL_DEBUG=y),
> and not supposed to be shared by different cores.
> All other things in the series look good to me.

OK, I will fix the alignment of rte_ring_debug_stats and
rte_mempool_debug_stats and send the next revision with your ACK.

Jerin


> BTW, by some reason I can't find that series in the patchworks.
> Konstantin
>
>
> >
> >  #define RTE_RING_NAMESIZE 32 /**< The maximum length of a ring name. */
> > --
> > 2.1.0
>
  
Ananyev, Konstantin Dec. 11, 2015, 2:42 p.m. UTC | #4
> >
> > I think we better keep both struct rte_ring_debug_stats and rte_mempool_debug_stats
> > as __rte_cache_aligned.
> > Both are on a  per core basis and can be used at run-time
> > (when RTE_LIBRTE_RING_DEBUG/RTE_LIBRTE_MEMPOOL_DEBUG=y),
> > and not supposed to be shared by different cores.
> > All other things in the series look good to me.
> 
> OK, I will fix the alignment of rte_ring_debug_stats and
> rte_mempool_debug_stats and send the next revision with your ACK.

Works for me.
Konstantin
  

Patch

diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index bada8ad..4dbf73b 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -863,7 +863,7 @@  struct rte_eth_rxq_info {
 	struct rte_eth_rxconf conf; /**< queue config parameters. */
 	uint8_t scattered_rx;       /**< scattered packets RX supported. */
 	uint16_t nb_desc;           /**< configured number of RXDs. */
-} __rte_cache_aligned;
+} __rte_cache_min_aligned;
 
 /**
  * Ethernet device TX queue information structure.
@@ -872,7 +872,7 @@  struct rte_eth_rxq_info {
 struct rte_eth_txq_info {
 	struct rte_eth_txconf conf; /**< queue config parameters. */
 	uint16_t nb_desc;           /**< configured number of TXDs. */
-} __rte_cache_aligned;
+} __rte_cache_min_aligned;
 
 /** Maximum name length for extended statistics counters */
 #define RTE_ETH_XSTATS_NAME_SIZE 64
diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_mempool.h
index 6e2390a..8e5d10c 100644
--- a/lib/librte_mempool/rte_mempool.h
+++ b/lib/librte_mempool/rte_mempool.h
@@ -92,7 +92,7 @@  struct rte_mempool_debug_stats {
 	uint64_t get_success_objs; /**< Objects successfully allocated. */
 	uint64_t get_fail_bulk;    /**< Failed allocation number. */
 	uint64_t get_fail_objs;    /**< Objects that failed to be allocated. */
-} __rte_cache_aligned;
+} __rte_cache_min_aligned;
 #endif
 
 #if RTE_MEMPOOL_CACHE_MAX_SIZE > 0
diff --git a/lib/librte_ring/rte_ring.h b/lib/librte_ring/rte_ring.h
index de036ce..33166aa 100644
--- a/lib/librte_ring/rte_ring.h
+++ b/lib/librte_ring/rte_ring.h
@@ -123,7 +123,7 @@  struct rte_ring_debug_stats {
 	uint64_t deq_success_objs; /**< Objects successfully dequeued. */
 	uint64_t deq_fail_bulk;    /**< Failed dequeues number. */
 	uint64_t deq_fail_objs;    /**< Objects that failed to be dequeued. */
-} __rte_cache_aligned;
+} __rte_cache_min_aligned;
 #endif
 
 #define RTE_RING_NAMESIZE 32 /**< The maximum length of a ring name. */