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

["Ananyev, Konstantin" <konstantin.ananyev@intel.com>]

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