linux-kernel - Re: [PATCH V2] mm/hugetlb: Make __NR_USED

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <d0b45f75-d500-4049-8240-56a0db4038d3@arm.com>
Date: Wed, 4 Dec 2024 08:16:36 +0530
From: Anshuman Khandual <anshuman.khandual@....com>
To: Muchun Song <muchun.song@...ux.dev>
Cc: linux-mm@...ck.org, Andrew Morton <akpm@...ux-foundation.org>,
 Oscar Salvador <osalvador@...e.de>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH V2] mm/hugetlb: Make __NR_USED_SUBPAGE check conditional



On 12/3/24 13:50, Muchun Song wrote:
> 
> 
>> On Dec 3, 2024, at 15:46, Anshuman Khandual <anshuman.khandual@....com> wrote:
>>
>>
>> On 12/3/24 08:59, Muchun Song wrote:
>>>
>>>
>>>> On Dec 3, 2024, at 10:32, Anshuman Khandual <anshuman.khandual@....com> wrote:
>>>>
>>>> The HugeTLB order check against __NR_USED_SUBPAGE is required only when
>>>> HUGETLB_PAGE_OPTIMIZE_VMEMMAP is enabled. Hence BUG_ON() trigger should
>>>> happen only when applicable.
>>>>
>>>> Cc: Muchun Song <muchun.song@...ux.dev>
>>>> Cc: Andrew Morton <akpm@...ux-foundation.org>
>>>> Cc: Oscar Salvador <osalvador@...e.de>
>>>> Cc: linux-mm@...ck.org
>>>> Cc: linux-kernel@...r.kernel.org
>>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@....com>
>>>> ---
>>>> This patch applies on v6.13-rc1
>>>>
>>>> Changes in V2:
>>>>
>>>> - Fixed #ifdef with CONFIG_HUGETLB_PAGE_OPTIMIZE_VMEMMAP per Oscar
>>>>
>>>> Changes in V1:
>>>>
>>>> https://lore.kernel.org/all/20241202090728.78935-1-anshuman.khandual@arm.com/
>>>>
>>>> mm/hugetlb.c | 6 ++++--
>>>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>>>> index ea2ed8e301ef..e6a5b21e3578 100644
>>>> --- a/mm/hugetlb.c
>>>> +++ b/mm/hugetlb.c
>>>> @@ -4513,11 +4513,13 @@ void __init hugetlb_add_hstate(unsigned int order)
>>>> 	struct hstate *h;
>>>> 	unsigned long i;
>>>>
>>>> - 	if (size_to_hstate(PAGE_SIZE << order)) {
>>>> + 	if (size_to_hstate(PAGE_SIZE << order))
>>>> 		return;
>>>> - 	}
>>>> +
>>>> 	BUG_ON(hugetlb_max_hstate >= HUGE_MAX_HSTATE);
>>>> +#ifdef CONFIG_HUGETLB_PAGE_OPTIMIZE_VMEMMAP
>>>> 	BUG_ON(order < order_base_2(__NR_USED_SUBPAGE));
>>>
>>> Hi Anshuman,
>>>
>>> __NR_USED_SUBPAGE indicates how many struct pages are used to store
>>> extra metadata for a HugeTLB page. So we need to make sure the order
>>> of a HugeTLB page should be bigger than or equal to order_base_2(__NR_USED_SUBPAGE).
>>> So It is not related to HVO. I don't think the changes make sense.
>>
>> I did think about that but order_base_2(__NR_USED_SUBPAGE) actually
>> turns out to be just 2 as __NR_USED_SUBPAGE is 3. Would not HugeTLB
>> size be always greater than four base pages in reality, thus making
>> this BUG_ON() check just redundant ?
> 
> Currently, there is no architectures supporting hugetlb page smaller
> than 4 base pages. I think the smallest size is 64KB in arm64, but who
> knows whether if certain architectures supports 8KB in the future or
> we want to uses more struct pages to store metadata for increasing
> __NR_USED_SUBPAGE (e.g. change it to 17). So I tend to keep this
> BUG_ON remain to catch unexpected bugs.

Sure, fair enough.