RE: [PATCH v6 3/4] mempool: fix cache flushing algorithm

["Morten Brørup" <mb@smartsharesystems.com>]

> From: Andrew Rybchenko [mailto:andrew.rybchenko@oktetlabs.ru]
> Sent: Sunday, 9 October 2022 16.52
> 
> On 10/9/22 17:31, Morten Brørup wrote:
> >> From: Andrew Rybchenko [mailto:andrew.rybchenko@oktetlabs.ru]
> >> Sent: Sunday, 9 October 2022 15.38
> >>
> >> From: Morten Brørup <mb@smartsharesystems.com>
> >>

[...]

> >> --- a/lib/mempool/rte_mempool.h
> >> +++ b/lib/mempool/rte_mempool.h
> >> @@ -90,7 +90,7 @@ struct rte_mempool_cache {
> >>   	 * Cache is allocated to this size to allow it to overflow in
> >> certain
> >>   	 * cases to avoid needless emptying of cache.
> >>   	 */
> >> -	void *objs[RTE_MEMPOOL_CACHE_MAX_SIZE * 3]; /**< Cache objects */
> >> +	void *objs[RTE_MEMPOOL_CACHE_MAX_SIZE * 2]; /**< Cache objects */
> >>   } __rte_cache_aligned;
> >
> > How much are we allowed to break the ABI here?
> >
> > This patch reduces the size of the structure by removing a now unused
> part at the end, which should be harmless.
> >
> > If we may also move the position of the objs array, I would add
> __rte_cache_aligned to the objs array. It makes no difference in the
> general case, but if get/put operations are always 32 objects, it will
> reduce the number of memory (or last level cache) accesses from five to
> four 64 B cache lines for every get/put operation.
> >
> > 	uint32_t len;	      /**< Current cache count */
> > -	/*
> > -	 * Cache is allocated to this size to allow it to overflow in
> certain
> > -	 * cases to avoid needless emptying of cache.
> > -	 */
> > -	void *objs[RTE_MEMPOOL_CACHE_MAX_SIZE * 3]; /**< Cache objects */
> > +	/**
> > +	 * Cache objects
> > +	 *
> > +	 * Cache is allocated to this size to allow it to overflow in
> certain
> > +	 * cases to avoid needless emptying of cache.
> > +	 */
> > +	void *objs[RTE_MEMPOOL_CACHE_MAX_SIZE * 2] __rte_cache_aligned;
> > } __rte_cache_aligned;
> 
> I think aligning objs on cacheline should be a separate patch.

Good point. I'll let you do it. :-)

PS: Thank you for following up on this patch series, Andrew!

-Morten