[RFC,0/4] mempool: avoid objects allocations across pages

  When IOVA mode is VA, a mempool can be created with objects that
are not physically contiguous, which breaks KNI.

To solve this, this patchset changes the default behavior of mempool
populate function, to prevent objects from being located across pages.

Olivier Matz (4):
  mempool: clarify default populate function
  mempool: unalign size when calculating required mem amount
  mempool: introduce function to get mempool page size
  mempool: prevent objects from being across pages

 lib/librte_mempool/rte_mempool.c             | 106 +++++++++++----------------
 lib/librte_mempool/rte_mempool.h             |   8 +-
 lib/librte_mempool/rte_mempool_ops.c         |   4 +-
 lib/librte_mempool/rte_mempool_ops_default.c |  39 +++++++++-
 4 files changed, 90 insertions(+), 67 deletions(-)

---

Hi,

> @Olivier,
> Any suggestions..?

I took some time to go a bit deeper. I still think we can change the
default behavior to avoid objects to be located accross pages. But
it is more complex that I expected.

I made a draft patchset, that, in short:
- cleans/renames variables
- removes the optimistic full iova contiguous allocation
- changes return value of calc_mem_size to return the unaligned size,
  therefore the allocation is smaller in case of big hugepages
- changes rte_mempool_op_populate_default() to prevent allocation
  of objects accross multiple pages

Andrew, Anatoly, did I miss something?
Vamsi, can you check if it solves your issue?

Anyway, even if validate the patchset it and make it work, I'm afraid
this is not something that could go in 19.08.

The only alternative I see is a specific mempool allocation function
when used in iova=va mode + kni, as you proposed previously.

It can probably be implemented without adding a flag, starting from
rte_mempool_create(), and replacing rte_mempool_populate_default(mp) by
something else: allocate pages one by one, and call
rte_mempool_populate_iova() for each of them.

Hope it helps. Unfortunately, I may not have too much time to spend on
it in the coming days.

Regards,
Olivier
  

> -----Original Message-----
> From: Olivier Matz <olivier.matz@6wind.com>
> Sent: Friday, July 19, 2019 7:09 PM
> To: Vamsi Krishna Attunuru <vattunuru@marvell.com>; dev@dpdk.org
> Cc: Andrew Rybchenko <arybchenko@solarflare.com>; Thomas Monjalon
> <thomas@monjalon.net>; Anatoly Burakov <anatoly.burakov@intel.com>; Jerin
> Jacob Kollanukkaran <jerinj@marvell.com>; Kiran Kumar Kokkilagadda
> <kirankumark@marvell.com>; Ferruh Yigit <ferruh.yigit@intel.com>
> Subject: [RFC 0/4] mempool: avoid objects allocations across pages
> 
> When IOVA mode is VA, a mempool can be created with objects that are not
> physically contiguous, which breaks KNI.
> 
> To solve this, this patchset changes the default behavior of mempool populate
> function, to prevent objects from being located across pages.
> 
> Olivier Matz (4):
>   mempool: clarify default populate function
>   mempool: unalign size when calculating required mem amount
>   mempool: introduce function to get mempool page size
>   mempool: prevent objects from being across pages
> 
>  lib/librte_mempool/rte_mempool.c             | 106 +++++++++++----------------
>  lib/librte_mempool/rte_mempool.h             |   8 +-
>  lib/librte_mempool/rte_mempool_ops.c         |   4 +-
>  lib/librte_mempool/rte_mempool_ops_default.c |  39 +++++++++-
>  4 files changed, 90 insertions(+), 67 deletions(-)
> 
> ---
> 
> Hi,
> 
> > @Olivier,
> > Any suggestions..?
> 
> I took some time to go a bit deeper. I still think we can change the default
> behavior to avoid objects to be located accross pages. But it is more complex
> that I expected.
> 
> I made a draft patchset, that, in short:
> - cleans/renames variables
> - removes the optimistic full iova contiguous allocation
> - changes return value of calc_mem_size to return the unaligned size,
>   therefore the allocation is smaller in case of big hugepages
> - changes rte_mempool_op_populate_default() to prevent allocation
>   of objects accross multiple pages
> 
> Andrew, Anatoly, did I miss something?
> Vamsi, can you check if it solves your issue?
> 
Thanks Olivier for your time, it solves our problem with the KNI in VA mode. 
Since the change set could not go in 19.08 and as you suggested, 
implemented mempool populate without a flag and used the same for kni application. 
Sending v8 with these changes.

> Anyway, even if validate the patchset it and make it work, I'm afraid this is not
> something that could go in 19.08.
> 
> The only alternative I see is a specific mempool allocation function when used in
> iova=va mode + kni, as you proposed previously.
> 
> It can probably be implemented without adding a flag, starting from
> rte_mempool_create(), and replacing rte_mempool_populate_default(mp) by
> something else: allocate pages one by one, and call
> rte_mempool_populate_iova() for each of them.
> 
> Hope it helps. Unfortunately, I may not have too much time to spend on it in the
> coming days.
> 
> Regards,
> Olivier
  

On 7/19/19 4:38 PM, Olivier Matz wrote:
> When IOVA mode is VA, a mempool can be created with objects that
> are not physically contiguous, which breaks KNI.
>
> To solve this, this patchset changes the default behavior of mempool
> populate function, to prevent objects from being located across pages.

I'll provide top level review notes on individual patches, but what
I don't understand in general, why do we add a rule to respect
page boundaries in any case even when it is not absolutely required.
It may add holes. Can it make negative impact on performance?

I think that KNI VA-mode requirements are very specific.
It is VA-mode, but page boundaries should be respected even
if VA is contiguous.

> Olivier Matz (4):
>    mempool: clarify default populate function
>    mempool: unalign size when calculating required mem amount
>    mempool: introduce function to get mempool page size
>    mempool: prevent objects from being across pages
>
>   lib/librte_mempool/rte_mempool.c             | 106 +++++++++++----------------
>   lib/librte_mempool/rte_mempool.h             |   8 +-
>   lib/librte_mempool/rte_mempool_ops.c         |   4 +-
>   lib/librte_mempool/rte_mempool_ops_default.c |  39 +++++++++-
>   4 files changed, 90 insertions(+), 67 deletions(-)
>
> ---
>
> Hi,
>
>> @Olivier,
>> Any suggestions..?
> I took some time to go a bit deeper. I still think we can change the
> default behavior to avoid objects to be located accross pages. But
> it is more complex that I expected.
>
> I made a draft patchset, that, in short:
> - cleans/renames variables
> - removes the optimistic full iova contiguous allocation
> - changes return value of calc_mem_size to return the unaligned size,
>    therefore the allocation is smaller in case of big hugepages
> - changes rte_mempool_op_populate_default() to prevent allocation
>    of objects accross multiple pages
>
> Andrew, Anatoly, did I miss something?
> Vamsi, can you check if it solves your issue?
>
> Anyway, even if validate the patchset it and make it work, I'm afraid
> this is not something that could go in 19.08.
>
> The only alternative I see is a specific mempool allocation function
> when used in iova=va mode + kni, as you proposed previously.
>
> It can probably be implemented without adding a flag, starting from
> rte_mempool_create(), and replacing rte_mempool_populate_default(mp) by
> something else: allocate pages one by one, and call
> rte_mempool_populate_iova() for each of them.
>
> Hope it helps. Unfortunately, I may not have too much time to spend on
> it in the coming days.
>
> Regards,
> Olivier
  

Hi Andrew,

Better late than never, here are answers to your comments.
I'm sending a new version of this patchset that addresses
them.

On Wed, Aug 07, 2019 at 06:21:01PM +0300, Andrew Rybchenko wrote:
> On 7/19/19 4:38 PM, Olivier Matz wrote:
> > When IOVA mode is VA, a mempool can be created with objects that
> > are not physically contiguous, which breaks KNI.
> > 
> > To solve this, this patchset changes the default behavior of mempool
> > populate function, to prevent objects from being located across pages.
> 
> I'll provide top level review notes on individual patches, but what
> I don't understand in general, why do we add a rule to respect
> page boundaries in any case even when it is not absolutely required.
> It may add holes. Can it make negative impact on performance?

In terms of memory consumption, the amount of wasted space is not
significant as soon as hugepages are used. In terms of performance, I
don't forsee any change.

> I think that KNI VA-mode requirements are very specific.
> It is VA-mode, but page boundaries should be respected even
> if VA is contiguous.

Yes, but on the other hand, changing the behavior does not hurt the
other use-cases in my opinion. I mean, having the ability to allocate an
IOVA-contiguous area at once does not bring a big added value,
especially given that there is no guarantee that it will success, and we
can fallback to a chunk-based allocation.


Olivier


> 
> > Olivier Matz (4):
> >    mempool: clarify default populate function
> >    mempool: unalign size when calculating required mem amount
> >    mempool: introduce function to get mempool page size
> >    mempool: prevent objects from being across pages
> > 
> >   lib/librte_mempool/rte_mempool.c             | 106 +++++++++++----------------
> >   lib/librte_mempool/rte_mempool.h             |   8 +-
> >   lib/librte_mempool/rte_mempool_ops.c         |   4 +-
> >   lib/librte_mempool/rte_mempool_ops_default.c |  39 +++++++++-
> >   4 files changed, 90 insertions(+), 67 deletions(-)
> > 
> > ---
> > 
> > Hi,
> > 
> > > @Olivier,
> > > Any suggestions..?
> > I took some time to go a bit deeper. I still think we can change the
> > default behavior to avoid objects to be located accross pages. But
> > it is more complex that I expected.
> > 
> > I made a draft patchset, that, in short:
> > - cleans/renames variables
> > - removes the optimistic full iova contiguous allocation
> > - changes return value of calc_mem_size to return the unaligned size,
> >    therefore the allocation is smaller in case of big hugepages
> > - changes rte_mempool_op_populate_default() to prevent allocation
> >    of objects accross multiple pages
> > 
> > Andrew, Anatoly, did I miss something?
> > Vamsi, can you check if it solves your issue?
> > 
> > Anyway, even if validate the patchset it and make it work, I'm afraid
> > this is not something that could go in 19.08.
> > 
> > The only alternative I see is a specific mempool allocation function
> > when used in iova=va mode + kni, as you proposed previously.
> > 
> > It can probably be implemented without adding a flag, starting from
> > rte_mempool_create(), and replacing rte_mempool_populate_default(mp) by
> > something else: allocate pages one by one, and call
> > rte_mempool_populate_iova() for each of them.
> > 
> > Hope it helps. Unfortunately, I may not have too much time to spend on
> > it in the coming days.
> > 
> > Regards,
> > Olivier
>