Re: [dpdk-dev] [PATCH v4 2/7] mempool: add mempool arg in xmem size and usage

[Olivier MATZ <olivier.matz@6wind.com>]

On Mon, Sep 04, 2017 at 08:28:36PM +0530, santosh wrote:
> 
> On Monday 04 September 2017 08:16 PM, Olivier MATZ wrote:
> > On Mon, Sep 04, 2017 at 08:03:53PM +0530, santosh wrote:
> >>
> >> On Monday 04 September 2017 07:52 PM, Olivier MATZ wrote:
> >>> On Tue, Aug 15, 2017 at 11:37:38AM +0530, Santosh Shukla wrote:
> >>>> xmem_size and xmem_usage need to know the status of mp->flag.
> >>>> Following patch will make use of that.
> >>>>
> >>>> Signed-off-by: Santosh Shukla <santosh.shukla@caviumnetworks.com>
> >>>> ---
> >>>>  drivers/net/xenvirt/rte_mempool_gntalloc.c |  5 +++--
> >>>>  lib/librte_mempool/rte_mempool.c           | 10 ++++++----
> >>>>  lib/librte_mempool/rte_mempool.h           |  8 ++++++--
> >>>>  test/test/test_mempool.c                   |  4 ++--
> >>>>  4 files changed, 17 insertions(+), 10 deletions(-)
> >>>>
> >>>> diff --git a/drivers/net/xenvirt/rte_mempool_gntalloc.c b/drivers/net/xenvirt/rte_mempool_gntalloc.c
> >>>> index 73e82f808..ee0bda459 100644
> >>>> --- a/drivers/net/xenvirt/rte_mempool_gntalloc.c
> >>>> +++ b/drivers/net/xenvirt/rte_mempool_gntalloc.c
> >>>> @@ -114,7 +114,7 @@ _create_mempool(const char *name, unsigned elt_num, unsigned elt_size,
> >>>>  	pg_shift = rte_bsf32(pg_sz);
> >>>>  
> >>>>  	rte_mempool_calc_obj_size(elt_size, flags, &objsz);
> >>>> -	sz = rte_mempool_xmem_size(elt_num, objsz.total_size, pg_shift);
> >>>> +	sz = rte_mempool_xmem_size(elt_num, objsz.total_size, pg_shift, NULL);
> >>>>  	pg_num = sz >> pg_shift;
> >>>>  
> >>>>  	pa_arr = calloc(pg_num, sizeof(pa_arr[0]));
> >>> What is the meaning of passing NULL to rte_mempool_xmem_size()?
> >>> Does it mean that flags are ignored?
> >> Yes that mean flags are ignored.
> > But the flags change the return value of rte_mempool_xmem_size(), right?
> 
> no, It won't change.
> 
> > So, correct me if I'm wrong, but if we don't pass the proper flags, the
> > returned value won't be the one we expect.
> 
> passing flag value other than MEMPOOL_F_POOL_BLK_SZ_ALIGNED, wont impact return value.

That's the case today with your patches.

But if someone else wants to add another flag, this may change.
And you do not describe in the help that mp can be NULL, why it would
occur, and what does that mean.

> >>> Wouldn't it be better to pass the mempool flags instead of the mempool
> >>> pointer?
> >> Keeping mempool as param rather flag useful in case user want to do/refer more
> >> thing in future for xmem_size/usage() api. Otherwise he has append one more param
> >> to api and send out deprecation notice.. Btw, its const param so won;t hurt right?
> >>
> >> However if you still want to restrict param to mp->flags then pl. suggest.

Yes, it looks better to pass the flags instead of the mempool pointer.