Message ID | 20170906112834.32378-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 |
On Wed, Sep 06, 2017 at 04:58:33PM +0530, Santosh Shukla wrote: > --- a/lib/librte_mempool/rte_mempool.h > +++ b/lib/librte_mempool/rte_mempool.h > @@ -271,6 +271,10 @@ struct rte_mempool { > * Note: This flag should not be passed by application. > */ > #define MEMPOOL_F_CAPA_PHYS_CONTIG 0x0040 > +/** > + * Align object start address to total elem size > + */ > +#define MEMPOOL_F_BLK_ALIGNED_OBJECTS 0x0080 Same than with the other flag: since the meaning of this flag is not obvious when we read the name, it has to be clearly described. - say that it's virtual address - say that it implies MEMPOOL_F_CAPA_PHYS_CONTIG - say that it can be advertised by a driver and the application should not pass it And, since it shall not be passed by an application, I suggest to add _CAPA too (i.e. MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS).
On Thursday 07 September 2017 01:43 PM, Olivier MATZ wrote: > On Wed, Sep 06, 2017 at 04:58:33PM +0530, Santosh Shukla wrote: >> --- a/lib/librte_mempool/rte_mempool.h >> +++ b/lib/librte_mempool/rte_mempool.h >> @@ -271,6 +271,10 @@ struct rte_mempool { >> * Note: This flag should not be passed by application. >> */ >> #define MEMPOOL_F_CAPA_PHYS_CONTIG 0x0040 >> +/** >> + * Align object start address to total elem size >> + */ >> +#define MEMPOOL_F_BLK_ALIGNED_OBJECTS 0x0080 > Same than with the other flag: since the meaning of this flag is not obvious > when we read the name, it has to be clearly described. > - say that it's virtual address > - say that it implies MEMPOOL_F_CAPA_PHYS_CONTIG > - say that it can be advertised by a driver and the application should > not pass it > > And, since it shall not be passed by an application, I suggest to add > _CAPA too (i.e. MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS). > Ok, I will elaborate on FLAG description in v6, and Rename to MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS. Can you please suggest are you ok with checking MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS | _PHYS_CONTIG in _xmem_size()/_usage(), asked in v4 [1] for same patch. [1] http://dpdk.org/dev/patchwork/patch/27600/
On Thu, Sep 07, 2017 at 01:57:57PM +0530, santosh wrote: > > On Thursday 07 September 2017 01:43 PM, Olivier MATZ wrote: > > On Wed, Sep 06, 2017 at 04:58:33PM +0530, Santosh Shukla wrote: > >> --- a/lib/librte_mempool/rte_mempool.h > >> +++ b/lib/librte_mempool/rte_mempool.h > >> @@ -271,6 +271,10 @@ struct rte_mempool { > >> * Note: This flag should not be passed by application. > >> */ > >> #define MEMPOOL_F_CAPA_PHYS_CONTIG 0x0040 > >> +/** > >> + * Align object start address to total elem size > >> + */ > >> +#define MEMPOOL_F_BLK_ALIGNED_OBJECTS 0x0080 > > Same than with the other flag: since the meaning of this flag is not obvious > > when we read the name, it has to be clearly described. > > - say that it's virtual address > > - say that it implies MEMPOOL_F_CAPA_PHYS_CONTIG > > - say that it can be advertised by a driver and the application should > > not pass it > > > > And, since it shall not be passed by an application, I suggest to add > > _CAPA too (i.e. MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS). > > > Ok, I will elaborate on FLAG description in v6, > and Rename to MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS. > > Can you please suggest are you ok with > checking MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS | _PHYS_CONTIG > in _xmem_size()/_usage(), asked in v4 [1] for same patch. > > [1] http://dpdk.org/dev/patchwork/patch/27600/ > yes, I'm ok with your proposition: - MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS and _PHYS_CONTIG are capa flags, not set by application but by the handler - the help says that _BLK_ALIGNED_OBJECTS implies _PHYS_CONTIG - test both (_BLK_ALIGNED_OBJECTS | _PHYS_CONTIG) in _xmem_size()/_usage()
diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c index 103fbd0ed..38dab1067 100644 --- a/lib/librte_mempool/rte_mempool.c +++ b/lib/librte_mempool/rte_mempool.c @@ -239,10 +239,14 @@ 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_BLK_ALIGNED_OBJECTS) + /* alignment need one additional object */ + elt_num += 1; + if (total_elt_sz == 0) return 0; @@ -265,13 +269,17 @@ 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_BLK_ALIGNED_OBJECTS) + /* alignment need one additional object */ + elt_num += 1; + /* if paddr is NULL, assume contiguous memory */ if (paddr == NULL) { start = 0; @@ -390,7 +398,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_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 63688faff..110ffb601 100644 --- a/lib/librte_mempool/rte_mempool.h +++ b/lib/librte_mempool/rte_mempool.h @@ -271,6 +271,10 @@ struct rte_mempool { * Note: This flag should not be passed by application. */ #define MEMPOOL_F_CAPA_PHYS_CONTIG 0x0040 +/** + * Align object start address to total elem size + */ +#define MEMPOOL_F_BLK_ALIGNED_OBJECTS 0x0080 /** * @internal When debug is enabled, store some statistics.