| Message ID | 20170907153042.30890-8-santosh.shukla@caviumnetworks.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Context | Check | Description |
|---|---|---|
| ci/Intel-compilation | success | Compilation OK |
| ci/checkpatch | success | coding style OK |
Tested-by: Hemant Agrawal <hemant.agrawal@nxp.com> On 9/7/2017 9:00 PM, Santosh Shukla wrote: > Some mempool hw like octeontx/fpa block, demands block size > (/total_elem_sz) aligned object start address. > > Introducing an MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS flag. > If this flag is set: > - Align object start address(vaddr) to a multiple of total_elt_sz. > - Allocate one additional object. Additional object is needed to make > sure that requested 'n' object gets correctly populated. > > Example: > - Let's say that we get 'x' size of memory chunk from memzone. > - And application has requested 'n' object from mempool. > - Ideally, we start using objects at start address 0 to...(x-block_sz) > for n obj. > - Not necessarily first object address i.e. 0 is aligned to block_sz. > - So we derive 'offset' value for block_sz alignment purpose i.e..'off'. > - That 'off' makes sure that start address of object is blk_sz aligned. > - Calculating 'off' may end up sacrificing first block_sz area of > memzone area x. So total number of the object which can fit in the > pool area is n-1, Which is incorrect behavior. > > Therefore we request one additional object (/block_sz area) from memzone > when MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS flag is set. > > Signed-off-by: Santosh Shukla <santosh.shukla@caviumnetworks.com> > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com> > --- > v5 --> v6: > - Renamed from MEMPOOL_F_BLK_ALIGNED_OBJECTS to > MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS. (Suggested by Olivier) > - Updated Capability flag descrioption (Suggested by Olivier) > > History refer [1] > [1] http://dpdk.org/dev/patchwork/patch/28418/ > > v4 --> v5: > - Added vaddr in git description of patch (suggested by Olivier) > - Renamed to aligned flag to MEMPOOL_F_BLK_ALIGNED_OBJECTS (suggested by > Olivier) > Refer [2]. > [2] http://dpdk.org/dev/patchwork/patch/27600/ > > lib/librte_mempool/rte_mempool.c | 19 ++++++++++++++++--- > lib/librte_mempool/rte_mempool.h | 12 ++++++++++++ > 2 files changed, 28 insertions(+), 3 deletions(-) > > diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c > index 146e38675..decdda3a6 100644 > --- a/lib/librte_mempool/rte_mempool.c > +++ b/lib/librte_mempool/rte_mempool.c > @@ -239,10 +239,15 @@ rte_mempool_calc_obj_size(uint32_t elt_size, uint32_t flags, > */ > size_t > rte_mempool_xmem_size(uint32_t elt_num, size_t total_elt_sz, uint32_t pg_shift, > - __rte_unused unsigned int flags) > + unsigned int flags) > { > size_t obj_per_page, pg_num, pg_sz; > > + if (flags & (MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS | > + MEMPOOL_F_CAPA_PHYS_CONTIG)) > + /* alignment need one additional object */ > + elt_num += 1; > + > if (total_elt_sz == 0) > return 0; > > @@ -265,13 +270,18 @@ rte_mempool_xmem_size(uint32_t elt_num, size_t total_elt_sz, uint32_t pg_shift, > ssize_t > rte_mempool_xmem_usage(__rte_unused void *vaddr, uint32_t elt_num, > size_t total_elt_sz, const phys_addr_t paddr[], uint32_t pg_num, > - uint32_t pg_shift, __rte_unused unsigned int flags) > + uint32_t pg_shift, unsigned int flags) > { > uint32_t elt_cnt = 0; > phys_addr_t start, end; > uint32_t paddr_idx; > size_t pg_sz = (size_t)1 << pg_shift; > > + if (flags & (MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS | > + MEMPOOL_F_CAPA_PHYS_CONTIG)) > + /* alignment need one additional object */ > + elt_num += 1; > + > /* if paddr is NULL, assume contiguous memory */ > if (paddr == NULL) { > start = 0; > @@ -390,7 +400,10 @@ rte_mempool_populate_phys(struct rte_mempool *mp, char *vaddr, > memhdr->free_cb = free_cb; > memhdr->opaque = opaque; > > - if (mp->flags & MEMPOOL_F_NO_CACHE_ALIGN) > + if (mp->flags & MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS) > + /* align object start address to a multiple of total_elt_sz */ > + off = total_elt_sz - ((uintptr_t)vaddr % total_elt_sz); > + else if (mp->flags & MEMPOOL_F_NO_CACHE_ALIGN) > off = RTE_PTR_ALIGN_CEIL(vaddr, 8) - vaddr; > else > off = RTE_PTR_ALIGN_CEIL(vaddr, RTE_CACHE_LINE_SIZE) - vaddr; > diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_mempool.h > index 734392556..24195dda0 100644 > --- a/lib/librte_mempool/rte_mempool.h > +++ b/lib/librte_mempool/rte_mempool.h > @@ -271,6 +271,18 @@ struct rte_mempool { > * Note: This flag should not be passed by application. > */ > #define MEMPOOL_F_CAPA_PHYS_CONTIG 0x0040 > +/** > + * This capability flag is advertised by a mempool handler. Used for a case > + * where mempool driver wants object start address(vaddr) aligned to block > + * size(/ total element size). > + * > + * Note: > + * - This flag should not be passed by application. > + * Flag used for mempool driver only. > + * - Mempool driver must also set MEMPOOL_F_CAPA_PHYS_CONTIG flag along with > + * MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS. > + */ > +#define MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS 0x0080 > > /** > * @internal When debug is enabled, store some statistics. >
On Thu, Sep 07, 2017 at 09:00:41PM +0530, Santosh Shukla wrote: > Some mempool hw like octeontx/fpa block, demands block size > (/total_elem_sz) aligned object start address. > > Introducing an MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS flag. > If this flag is set: > - Align object start address(vaddr) to a multiple of total_elt_sz. > - Allocate one additional object. Additional object is needed to make > sure that requested 'n' object gets correctly populated. > > Example: > - Let's say that we get 'x' size of memory chunk from memzone. > - And application has requested 'n' object from mempool. > - Ideally, we start using objects at start address 0 to...(x-block_sz) > for n obj. > - Not necessarily first object address i.e. 0 is aligned to block_sz. > - So we derive 'offset' value for block_sz alignment purpose i.e..'off'. > - That 'off' makes sure that start address of object is blk_sz aligned. > - Calculating 'off' may end up sacrificing first block_sz area of > memzone area x. So total number of the object which can fit in the > pool area is n-1, Which is incorrect behavior. > > Therefore we request one additional object (/block_sz area) from memzone > when MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS flag is set. > > Signed-off-by: Santosh Shukla <santosh.shukla@caviumnetworks.com> > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com> > > [...] > > --- a/lib/librte_mempool/rte_mempool.c > +++ b/lib/librte_mempool/rte_mempool.c > @@ -239,10 +239,15 @@ rte_mempool_calc_obj_size(uint32_t elt_size, uint32_t flags, > */ > size_t > rte_mempool_xmem_size(uint32_t elt_num, size_t total_elt_sz, uint32_t pg_shift, > - __rte_unused unsigned int flags) > + unsigned int flags) > { > size_t obj_per_page, pg_num, pg_sz; > > + if (flags & (MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS | > + MEMPOOL_F_CAPA_PHYS_CONTIG)) > + /* alignment need one additional object */ > + elt_num += 1; > + In previous version, we agreed to test both _BLK_ALIGNED_OBJECTS and _PHYS_CONTIG in _xmem_size()/_usage(). Here, the test will also be true if only MEMPOOL_F_CAPA_PHYS_CONTIG is set. If we want to test both, the test should be: mask = MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS | MEMPOOL_F_CAPA_PHYS_CONTIG; if ((flags & mask) == mask) > @@ -265,13 +270,18 @@ rte_mempool_xmem_size(uint32_t elt_num, size_t total_elt_sz, uint32_t pg_shift, > ssize_t > rte_mempool_xmem_usage(__rte_unused void *vaddr, uint32_t elt_num, > size_t total_elt_sz, const phys_addr_t paddr[], uint32_t pg_num, > - uint32_t pg_shift, __rte_unused unsigned int flags) > + uint32_t pg_shift, unsigned int flags) > { > uint32_t elt_cnt = 0; > phys_addr_t start, end; > uint32_t paddr_idx; > size_t pg_sz = (size_t)1 << pg_shift; > > + if (flags & (MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS | > + MEMPOOL_F_CAPA_PHYS_CONTIG)) > + /* alignment need one additional object */ > + elt_num += 1; > + Same here
On Monday 25 September 2017 12:32 PM, Olivier MATZ wrote: > On Thu, Sep 07, 2017 at 09:00:41PM +0530, Santosh Shukla wrote: >> Some mempool hw like octeontx/fpa block, demands block size >> (/total_elem_sz) aligned object start address. >> >> Introducing an MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS flag. >> If this flag is set: >> - Align object start address(vaddr) to a multiple of total_elt_sz. >> - Allocate one additional object. Additional object is needed to make >> sure that requested 'n' object gets correctly populated. >> >> Example: >> - Let's say that we get 'x' size of memory chunk from memzone. >> - And application has requested 'n' object from mempool. >> - Ideally, we start using objects at start address 0 to...(x-block_sz) >> for n obj. >> - Not necessarily first object address i.e. 0 is aligned to block_sz. >> - So we derive 'offset' value for block_sz alignment purpose i.e..'off'. >> - That 'off' makes sure that start address of object is blk_sz aligned. >> - Calculating 'off' may end up sacrificing first block_sz area of >> memzone area x. So total number of the object which can fit in the >> pool area is n-1, Which is incorrect behavior. >> >> Therefore we request one additional object (/block_sz area) from memzone >> when MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS flag is set. >> >> Signed-off-by: Santosh Shukla <santosh.shukla@caviumnetworks.com> >> Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com> >> >> [...] >> >> --- a/lib/librte_mempool/rte_mempool.c >> +++ b/lib/librte_mempool/rte_mempool.c >> @@ -239,10 +239,15 @@ rte_mempool_calc_obj_size(uint32_t elt_size, uint32_t flags, >> */ >> size_t >> rte_mempool_xmem_size(uint32_t elt_num, size_t total_elt_sz, uint32_t pg_shift, >> - __rte_unused unsigned int flags) >> + unsigned int flags) >> { >> size_t obj_per_page, pg_num, pg_sz; >> >> + if (flags & (MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS | >> + MEMPOOL_F_CAPA_PHYS_CONTIG)) >> + /* alignment need one additional object */ >> + elt_num += 1; >> + > In previous version, we agreed to test both _BLK_ALIGNED_OBJECTS > and _PHYS_CONTIG in _xmem_size()/_usage(). Here, the test will > also be true if only MEMPOOL_F_CAPA_PHYS_CONTIG is set. > > If we want to test both, the test should be: > > mask = MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS | MEMPOOL_F_CAPA_PHYS_CONTIG; > if ((flags & mask) == mask) queued for v7. agree strict check. Thanks. >> @@ -265,13 +270,18 @@ rte_mempool_xmem_size(uint32_t elt_num, size_t total_elt_sz, uint32_t pg_shift, >> ssize_t >> rte_mempool_xmem_usage(__rte_unused void *vaddr, uint32_t elt_num, >> size_t total_elt_sz, const phys_addr_t paddr[], uint32_t pg_num, >> - uint32_t pg_shift, __rte_unused unsigned int flags) >> + uint32_t pg_shift, unsigned int flags) >> { >> uint32_t elt_cnt = 0; >> phys_addr_t start, end; >> uint32_t paddr_idx; >> size_t pg_sz = (size_t)1 << pg_shift; >> >> + if (flags & (MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS | >> + MEMPOOL_F_CAPA_PHYS_CONTIG)) >> + /* alignment need one additional object */ >> + elt_num += 1; >> + > Same here >
diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c index 146e38675..decdda3a6 100644 --- a/lib/librte_mempool/rte_mempool.c +++ b/lib/librte_mempool/rte_mempool.c @@ -239,10 +239,15 @@ rte_mempool_calc_obj_size(uint32_t elt_size, uint32_t flags, */ size_t rte_mempool_xmem_size(uint32_t elt_num, size_t total_elt_sz, uint32_t pg_shift, - __rte_unused unsigned int flags) + unsigned int flags) { size_t obj_per_page, pg_num, pg_sz; + if (flags & (MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS | + MEMPOOL_F_CAPA_PHYS_CONTIG)) + /* alignment need one additional object */ + elt_num += 1; + if (total_elt_sz == 0) return 0; @@ -265,13 +270,18 @@ rte_mempool_xmem_size(uint32_t elt_num, size_t total_elt_sz, uint32_t pg_shift, ssize_t rte_mempool_xmem_usage(__rte_unused void *vaddr, uint32_t elt_num, size_t total_elt_sz, const phys_addr_t paddr[], uint32_t pg_num, - uint32_t pg_shift, __rte_unused unsigned int flags) + uint32_t pg_shift, unsigned int flags) { uint32_t elt_cnt = 0; phys_addr_t start, end; uint32_t paddr_idx; size_t pg_sz = (size_t)1 << pg_shift; + if (flags & (MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS | + MEMPOOL_F_CAPA_PHYS_CONTIG)) + /* alignment need one additional object */ + elt_num += 1; + /* if paddr is NULL, assume contiguous memory */ if (paddr == NULL) { start = 0; @@ -390,7 +400,10 @@ rte_mempool_populate_phys(struct rte_mempool *mp, char *vaddr, memhdr->free_cb = free_cb; memhdr->opaque = opaque; - if (mp->flags & MEMPOOL_F_NO_CACHE_ALIGN) + if (mp->flags & MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS) + /* align object start address to a multiple of total_elt_sz */ + off = total_elt_sz - ((uintptr_t)vaddr % total_elt_sz); + else if (mp->flags & MEMPOOL_F_NO_CACHE_ALIGN) off = RTE_PTR_ALIGN_CEIL(vaddr, 8) - vaddr; else off = RTE_PTR_ALIGN_CEIL(vaddr, RTE_CACHE_LINE_SIZE) - vaddr; diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_mempool.h index 734392556..24195dda0 100644 --- a/lib/librte_mempool/rte_mempool.h +++ b/lib/librte_mempool/rte_mempool.h @@ -271,6 +271,18 @@ struct rte_mempool { * Note: This flag should not be passed by application. */ #define MEMPOOL_F_CAPA_PHYS_CONTIG 0x0040 +/** + * This capability flag is advertised by a mempool handler. Used for a case + * where mempool driver wants object start address(vaddr) aligned to block + * size(/ total element size). + * + * Note: + * - This flag should not be passed by application. + * Flag used for mempool driver only. + * - Mempool driver must also set MEMPOOL_F_CAPA_PHYS_CONTIG flag along with + * MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS. + */ +#define MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS 0x0080 /** * @internal When debug is enabled, store some statistics.