[dpdk-dev,v2] mempool: avoid memory waste with large pagesize

  If page size is large (like 64K on ARM) and object size is small
then don't waste lots of memory by rounding up to page size.
Instead, round up so that 1 or more objects all fit in a page.

This preserves the requirement that an object must not a page
or virt2phys would break, and makes sure 62K is not wasted per mbuf.

Also, fix invalid use of printf (versus log) for error reporting.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
v2 - fix trailer_size calculation, and replace printf with log

 lib/librte_mempool/rte_mempool.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)
  

Hello,

On 03/09/2016 10:12 PM, Stephen Hemminger wrote:
> If page size is large (like 64K on ARM) and object size is small
> then don't waste lots of memory by rounding up to page size.
> Instead, round up so that 1 or more objects all fit in a page.
> 
> This preserves the requirement that an object must not a page
> or virt2phys would break, and makes sure 62K is not wasted per mbuf.
> 
> Also, fix invalid use of printf (versus log) for error reporting.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> v2 - fix trailer_size calculation, and replace printf with log
> 
>  lib/librte_mempool/rte_mempool.c | 23 ++++++++++++++---------
>  1 file changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c
> index f8781e1..ff08a1a 100644
> --- a/lib/librte_mempool/rte_mempool.c
> +++ b/lib/librte_mempool/rte_mempool.c
> @@ -300,18 +300,23 @@ rte_mempool_calc_obj_size(uint32_t elt_size, uint32_t flags,
>  	if (! rte_eal_has_hugepages()) {
>  		/*
>  		 * compute trailer size so that pool elements fit exactly in
> -		 * a standard page
> +		 * a standard page. If elements are smaller than a page
> +		 * then allow multiple elements per page
>  		 */
> -		int page_size = getpagesize();
> -		int new_size = page_size - sz->header_size - sz->elt_size;
> -		if (new_size < 0 || (unsigned int)new_size < sz->trailer_size) {
> -			printf("When hugepages are disabled, pool objects "
> -			       "can't exceed PAGE_SIZE: %d + %d + %d > %d\n",
> -			       sz->header_size, sz->elt_size, sz->trailer_size,
> -			       page_size);
> +		unsigned new_size, orig_size, page_size;
> +
> +		page_size = getpagesize();
> +		orig_size = sz->header_size + sz->elt_size;
> +		new_size = rte_align32pow2(orig_size);
> +		if (new_size > page_size) {
> +			RTE_LOG(ERR, MEMPOOL,
> +				"When hugepages are disabled, pool objects "
> +				"can't exceed PAGE_SIZE: %u + %u + %u > %u\n",
> +				sz->header_size, sz->elt_size, sz->trailer_size,
> +				page_size);
>  			return 0;
>  		}
> -		sz->trailer_size = new_size;
> +		sz->trailer_size = new_size - orig_size;
>  	}
>  
>  	/* this is the size of an object, including header and trailer */
> 

It still does not work. When CONFIG_RTE_LIBRTE_MEMPOOL_DEBUG=y:

mp = rte_mempool_create("test", 128,
	64, 0, 0, NULL, NULL, NULL, NULL, SOCKET_ID_ANY, 0);
rte_mempool_dump(stdout, mp);

  populated_size=128
  header_size=64
  elt_size=64
  trailer_size=64
  total_obj_size=192


Regards,
Olivier
  

On 3/10/2016 8:37 AM, Olivier MATZ wrote:
> Hello,
> 
> On 03/09/2016 10:12 PM, Stephen Hemminger wrote:
>> If page size is large (like 64K on ARM) and object size is small
>> then don't waste lots of memory by rounding up to page size.
>> Instead, round up so that 1 or more objects all fit in a page.
>>
>> This preserves the requirement that an object must not a page
>> or virt2phys would break, and makes sure 62K is not wasted per mbuf.
>>
>> Also, fix invalid use of printf (versus log) for error reporting.
>>
>> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>> ---
>> v2 - fix trailer_size calculation, and replace printf with log
>>
>>  lib/librte_mempool/rte_mempool.c | 23 ++++++++++++++---------
>>  1 file changed, 14 insertions(+), 9 deletions(-)
>>
>> diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c
>> index f8781e1..ff08a1a 100644
>> --- a/lib/librte_mempool/rte_mempool.c
>> +++ b/lib/librte_mempool/rte_mempool.c
>> @@ -300,18 +300,23 @@ rte_mempool_calc_obj_size(uint32_t elt_size, uint32_t flags,
>>  	if (! rte_eal_has_hugepages()) {
>>  		/*
>>  		 * compute trailer size so that pool elements fit exactly in
>> -		 * a standard page
>> +		 * a standard page. If elements are smaller than a page
>> +		 * then allow multiple elements per page
>>  		 */
>> -		int page_size = getpagesize();
>> -		int new_size = page_size - sz->header_size - sz->elt_size;
>> -		if (new_size < 0 || (unsigned int)new_size < sz->trailer_size) {
>> -			printf("When hugepages are disabled, pool objects "
>> -			       "can't exceed PAGE_SIZE: %d + %d + %d > %d\n",
>> -			       sz->header_size, sz->elt_size, sz->trailer_size,
>> -			       page_size);
>> +		unsigned new_size, orig_size, page_size;
>> +
>> +		page_size = getpagesize();
>> +		orig_size = sz->header_size + sz->elt_size;
>> +		new_size = rte_align32pow2(orig_size);
>> +		if (new_size > page_size) {
>> +			RTE_LOG(ERR, MEMPOOL,
>> +				"When hugepages are disabled, pool objects "
>> +				"can't exceed PAGE_SIZE: %u + %u + %u > %u\n",
>> +				sz->header_size, sz->elt_size, sz->trailer_size,
>> +				page_size);
>>  			return 0;
>>  		}
>> -		sz->trailer_size = new_size;
>> +		sz->trailer_size = new_size - orig_size;
>>  	}
>>  
>>  	/* this is the size of an object, including header and trailer */
>>
> 
> It still does not work. When CONFIG_RTE_LIBRTE_MEMPOOL_DEBUG=y:
> 
> mp = rte_mempool_create("test", 128,
> 	64, 0, 0, NULL, NULL, NULL, NULL, SOCKET_ID_ANY, 0);
> rte_mempool_dump(stdout, mp);
> 
>   populated_size=128
>   header_size=64
>   elt_size=64
>   trailer_size=64
>   total_obj_size=192
> 
With --no-huge (since patch updates that part), it gives:
  header_size=64
  elt_size=64
  trailer_size=0
  total_obj_size=128
  private_data_size=3904

1- private_data still rounds to page size, not sure if this is a problem.
2- with MEMPOOL_DEBUG=y, trailer_size is 0, comment in code says it
should keep cookie, not sure if this is a problem.

Regards,
ferruh
  

>> It still does not work. When CONFIG_RTE_LIBRTE_MEMPOOL_DEBUG=y:
>>
>> mp = rte_mempool_create("test", 128,
>> 	64, 0, 0, NULL, NULL, NULL, NULL, SOCKET_ID_ANY, 0);
>> rte_mempool_dump(stdout, mp);
>>
>>   populated_size=128
>>   header_size=64
>>   elt_size=64
>>   trailer_size=64
>>   total_obj_size=192
>>
> With --no-huge (since patch updates that part), it gives:
>   header_size=64
>   elt_size=64
>   trailer_size=0
>   total_obj_size=128
>   private_data_size=3904
> 
> 1- private_data still rounds to page size, not sure if this is a problem.
> 2- with MEMPOOL_DEBUG=y, trailer_size is 0, comment in code says it
> should keep cookie, not sure if this is a problem.

Yes the trailer should at least be 8 bytes to store the cookie.
  

On Thu, 10 Mar 2016 12:12:12 +0100
Olivier MATZ <olivier.matz@6wind.com> wrote:

> >> It still does not work. When CONFIG_RTE_LIBRTE_MEMPOOL_DEBUG=y:
> >>
> >> mp = rte_mempool_create("test", 128,
> >> 	64, 0, 0, NULL, NULL, NULL, NULL, SOCKET_ID_ANY, 0);
> >> rte_mempool_dump(stdout, mp);
> >>
> >>   populated_size=128
> >>   header_size=64
> >>   elt_size=64
> >>   trailer_size=64
> >>   total_obj_size=192
> >>
> > With --no-huge (since patch updates that part), it gives:
> >   header_size=64
> >   elt_size=64
> >   trailer_size=0
> >   total_obj_size=128
> >   private_data_size=3904
> > 
> > 1- private_data still rounds to page size, not sure if this is a problem.
> > 2- with MEMPOOL_DEBUG=y, trailer_size is 0, comment in code says it
> > should keep cookie, not sure if this is a problem.
> 
> Yes the trailer should at least be 8 bytes to store the cookie.
> 

Ok, will fix. that.