lkml.org 
[lkml]   [2017]   [Jul]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] powerpc/mm: Implemented default_hugepagesz verification for powerpc
Em 2017-07-05 13:03, victora escreveu:
> Em 2017-07-05 01:26, Aneesh Kumar K.V escreveu:
>> On Tuesday 04 July 2017 01:35 AM, Victor Aoqui wrote:
>>> Implemented default hugepage size verification (default_hugepagesz=)
>>> in order to allow allocation of defined number of pages (hugepages=)
>>> only for supported hugepage sizes.
>>>
>>> Signed-off-by: Victor Aoqui <victora@br.ibm.com>
>>> ---
>>> arch/powerpc/mm/hugetlbpage.c | 15 +++++++++++++++
>>> 1 file changed, 15 insertions(+)
>>>
>>> diff --git a/arch/powerpc/mm/hugetlbpage.c
>>> b/arch/powerpc/mm/hugetlbpage.c
>>> index a4f33de..464e72e 100644
>>> --- a/arch/powerpc/mm/hugetlbpage.c
>>> +++ b/arch/powerpc/mm/hugetlbpage.c
>>> @@ -797,6 +797,21 @@ static int __init hugepage_setup_sz(char *str)
>>> }
>>> __setup("hugepagesz=", hugepage_setup_sz);
>>>
>>> +static int __init default_hugepage_setup_sz(char *str)
>>> +{
>>> + unsigned long long size;
>>> +
>>> + size = memparse(str, &str);
>>> +
>>> + if (add_huge_page_size(size) != 0) {
>>> + hugetlb_bad_size();
>>> + pr_err("Invalid default huge page size
>>> specified(%llu)\n", size);
>>> + }
>>> +
>>> + return 1;
>>> +}
>>> +__setup("default_hugepagesz=", default_hugepage_setup_sz);
>>
>> isn't that a behavior change in what we have now ? . Right now if size
>> specified is not supported, we fallback to HPAGE_SIZE.
>
> Yes, it is. However, is this a correct behavior? If we specify an
> unsupported value, for example default_hugepagesz=1M and
> hugepages=1000, 1M will be ignored and 1000 pages of 16M (arch
> default) will be allocated. This could lead to non-expected out of of
> memory/performance issue.
>
>>
>> mm/hugetlb.c
>>
>> if (!size_to_hstate(default_hstate_size)) {
>> default_hstate_size = HPAGE_SIZE;
>> if (!size_to_hstate(default_hstate_size))
>> hugetlb_add_hstate(HUGETLB_PAGE_ORDER);
>> }
>>
>>
>>> +
>>> struct kmem_cache *hugepte_cache;
>>> static int __init hugetlbpage_init(void)
>>> {
>>>
>>
>> Even if we want to do this, this should be done in generic code and
>> should not be powerpc specific
>>
>
> The verification of supported powerpc hugepage size (hugepagesz=) is
> being performed on add_huge_page_size(), which is currently defined in
> arch/powerpc/mm/hugetlbpage.c. I think it makes more sense to
> implement default_hugepagesz= verification on arch/powerpc, don't you
> think?
>
>> -aneesh

Hi Aneesh,

Did you have time to review the comments above?

Thanks
Victor

\
 
 \ /
  Last update: 2017-07-12 17:14    [W:0.062 / U:0.076 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site