[dpdk-dev] [PATCH v3 08/12] mempool: allow config override on element alignment

[Bruce Richardson]

On Tue, Jul 07, 2015 at 05:15:41PM +0800, Tony Lu wrote:
> 
> 
> >-----Original Message-----
> >From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Bruce Richardson
> >Sent: Monday, July 06, 2015 11:38 PM
> >To: Zhigang Lu
> >Cc: dev at dpdk.org
> >Subject: Re: [dpdk-dev] [PATCH v3 08/12] mempool: allow config override on
> >element alignment
> >
> >On Mon, Jul 06, 2015 at 04:51:33PM +0800, Zhigang Lu wrote:
> >> On TILE-Gx and TILE-Mx platforms, the buffers fed into the hardware
> >> buffer manager require a 128-byte alignment.  With this change, we
> >> allow configuration based override of the element alignment, and
> >> default to RTE_CACHE_LINE_SIZE if left unspecified.
> >>
> >> Change-Id: I9cd789d92b0bc9c8f44a633de59bb04d45d927a7
> >> Signed-off-by: Zhigang Lu <zlu at ezchip.com>
> >
> >This looks an OK change. However, would it be worthwhile making this a
> runtime
> >parameter rather than a compile-time one? Is it likely that we will ever
> have a
> >case where someone wants two mempools with different alignments (and
> >where using the larger of the two would be problematic)?
> 
> For now, I don't think it is very much worthwhile making this a runtime
> parameter,
> since doing so requires changing the mempool library API, and also the users
> of mempool
> do not quite care about the underlying alignments. Currently, the alignment
> for mempool
> objects is mostly a hardware requirement (currently RTE_CACHE_LINE_SIZE for
> good
> performance). And now we are defining a new RTE_MEMPOOL_ALIGN for mempool
> alignment requirement for cases where someone needs other alignments than
> RTE_CACHE_LINE_SIZE.
> 
> If someone wants two mempools with different alignments, using the larger
> one would
> not be a problem in current mempool implementation. Because even for the
> case where
> there is one alignment requirement RTE_CACHE_LINE_SIZE, we would provide
> larger one
> (2* RTE_CACHE_LINE_SIZE and larger) for allocated mempool objects, as those
> objects
> are continuous in memory. So we could not avoid larger one in current
> implementation.
> 
> Thanks again for reviewing!
> -Zhigang
> 
> >/Bruce
>
Ok, makes sense.
/Bruce