| Message ID | 20241203023207.123416-1-anshuman.khandual@arm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [V2] mm/hugetlb: Make __NR_USED_SUBPAGE check conditional | expand |
> On Dec 3, 2024, at 10:32, Anshuman Khandual <anshuman.khandual@arm.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@linux.dev> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Oscar Salvador <osalvador@suse.de> > Cc: linux-mm@kvack.org > Cc: linux-kernel@vger.kernel.org > Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.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. Thanks. > +#endif > h = &hstates[hugetlb_max_hstate++]; > __mutex_init(&h->resize_lock, "resize mutex", &h->resize_key); > h->order = order; > -- > 2.30.2 >
On 12/3/24 08:59, Muchun Song wrote: > > >> On Dec 3, 2024, at 10:32, Anshuman Khandual <anshuman.khandual@arm.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@linux.dev> >> Cc: Andrew Morton <akpm@linux-foundation.org> >> Cc: Oscar Salvador <osalvador@suse.de> >> Cc: linux-mm@kvack.org >> Cc: linux-kernel@vger.kernel.org >> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.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 ? > > Thanks. > >> +#endif >> h = &hstates[hugetlb_max_hstate++]; >> __mutex_init(&h->resize_lock, "resize mutex", &h->resize_key); >> h->order = order; >> -- >> 2.30.2 >> >
> On Dec 3, 2024, at 15:46, Anshuman Khandual <anshuman.khandual@arm.com> wrote: > > > On 12/3/24 08:59, Muchun Song wrote: >> >> >>> On Dec 3, 2024, at 10:32, Anshuman Khandual <anshuman.khandual@arm.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@linux.dev> >>> Cc: Andrew Morton <akpm@linux-foundation.org> >>> Cc: Oscar Salvador <osalvador@suse.de> >>> Cc: linux-mm@kvack.org >>> Cc: linux-kernel@vger.kernel.org >>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.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. Thanks. > >> >> Thanks. >> >>> +#endif >>> h = &hstates[hugetlb_max_hstate++]; >>> __mutex_init(&h->resize_lock, "resize mutex", &h->resize_key); >>> h->order = order; >>> -- >>> 2.30.2 >>> >>
On 12/3/24 13:50, Muchun Song wrote: > > >> On Dec 3, 2024, at 15:46, Anshuman Khandual <anshuman.khandual@arm.com> wrote: >> >> >> On 12/3/24 08:59, Muchun Song wrote: >>> >>> >>>> On Dec 3, 2024, at 10:32, Anshuman Khandual <anshuman.khandual@arm.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@linux.dev> >>>> Cc: Andrew Morton <akpm@linux-foundation.org> >>>> Cc: Oscar Salvador <osalvador@suse.de> >>>> Cc: linux-mm@kvack.org >>>> Cc: linux-kernel@vger.kernel.org >>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.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.
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)); +#endif h = &hstates[hugetlb_max_hstate++]; __mutex_init(&h->resize_lock, "resize mutex", &h->resize_key); h->order = order;
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@linux.dev> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Oscar Salvador <osalvador@suse.de> Cc: linux-mm@kvack.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.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(-)