On 12/3/24 13:50, Muchun Song wrote: > > >> On Dec 3, 2024, at 15:46, Anshuman Khandual <anshuman.khandual@xxxxxxx> wrote: >> >> >> On 12/3/24 08:59, Muchun Song wrote: >>> >>> >>>> On Dec 3, 2024, at 10:32, Anshuman Khandual <anshuman.khandual@xxxxxxx> 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@xxxxxxxxx> >>>> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> >>>> Cc: Oscar Salvador <osalvador@xxxxxxx> >>>> Cc: linux-mm@xxxxxxxxx >>>> Cc: linux-kernel@xxxxxxxxxxxxxxx >>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@xxxxxxx> >>>> --- >>>> 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@xxxxxxx/ >>>> >>>> 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.
