Re: [PATCH v2] mm/hugetlb: fix total hugetlbfs pages count when memory overcommit accouting

[Wanpeng Li <liwanp@linux.vnet.ibm.com>]

On Thu, Mar 14, 2013 at 12:09:27PM +0100, Michal Hocko wrote:
>On Thu 14-03-13 18:49:49, Wanpeng Li wrote:
>> Changelog:
>>  v1 -> v2:
>>   * update patch description, spotted by Michal
>> 
>> hugetlb_total_pages() does not account for all the supported hugepage
>> sizes.
>
>> This can lead to incorrect calculation of the total number of
>> page frames used by hugetlb. This patch corrects the issue.
>

Hi Michal,

>Sorry to be so picky but this doesn't tell us much. Why do we need to
>have the total number of hugetlb pages?
>
>What about the following:
>"hugetlb_total_pages is used for overcommit calculations but the
>current implementation considers only default hugetlb page size (which
>is either the first defined hugepage size or the one specified by
>default_hugepagesz kernel boot parameter).
>
>If the system is configured for more than one hugepage size (which is
>possible since a137e1cc hugetlbfs: per mount huge page sizes) then
>the overcommit estimation done by __vm_enough_memory (resp. shown by
>meminfo_proc_show) is not precise - there is an impression of more
>available/allowed memory. This can lead to an unexpected ENOMEM/EFAULT
>resp. SIGSEGV when memory is accounted."
>

Fair enough, thanks. :-)

>I think this is also worth pushing to the stable tree (it goes back to
>2.6.27)
>

Yup, I will Cc Greg in next version. 

>> Testcase:
>> boot: hugepagesz=1G hugepages=1
>> before patch:
>> egrep 'CommitLimit' /proc/meminfo
>> CommitLimit:     55434168 kB
>> after patch:
>> egrep 'CommitLimit' /proc/meminfo
>> CommitLimit:     54909880 kB
>
>This gives some more confusion to a reader because there is only
>something like 500M difference here without any explanation.
>

the default overcommit ratio is 50.

Regards,
Wanpeng Li 

>> 
>> Signed-off-by: Wanpeng Li <liwanp@linux.vnet.ibm.com>
>> ---
>>  mm/hugetlb.c | 7 +++++--
>>  1 file changed, 5 insertions(+), 2 deletions(-)
>> 
>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>> index cdb64e4..9e25040 100644
>> --- a/mm/hugetlb.c
>> +++ b/mm/hugetlb.c
>> @@ -2124,8 +2124,11 @@ int hugetlb_report_node_meminfo(int nid, char *buf)
>>  /* Return the number pages of memory we physically have, in PAGE_SIZE units. */
>>  unsigned long hugetlb_total_pages(void)
>>  {
>> -	struct hstate *h = &default_hstate;
>> -	return h->nr_huge_pages * pages_per_huge_page(h);
>> +	struct hstate *h;
>> +	unsigned long nr_total_pages = 0;
>> +	for_each_hstate(h)
>> +		nr_total_pages += h->nr_huge_pages * pages_per_huge_page(h);
>> +	return nr_total_pages;
>>  }
>>  
>>  static int hugetlb_acct_memory(struct hstate *h, long delta)
>> -- 
>> 1.7.11.7
>> 
>> --
>> To unsubscribe, send a message with 'unsubscribe linux-mm' in
>> the body to majordomo@kvack.org.  For more info on Linux MM,
>> see: http://www.linux-mm.org/ .
>> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
>
>-- 
>Michal Hocko
>SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>