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

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

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
>