On Wed, 13 Apr 2022 22:47:45 +0800 Muchun Song <[email protected]> wrote:
> If the size of "struct page" is not the power of two but with the feature
> of minimizing overhead of struct page associated with each HugeTLB is
> enabled, then the vmemmap pages of HugeTLB will be corrupted after
> remapping (panic is about to happen in theory). But this only exists when
> !CONFIG_MEMCG && !CONFIG_SLUB on x86_64. However, it is not a conventional
> configuration nowadays. So it is not a real word issue, just the result
> of a code review.
The patch does add a whole bunch of tricky junk to address something
which won't happen. How about we simply disable
CONFIG_HUGETLB_PAGE_OPTIMIZE_VMEMMAP if (!CONFIG_MEMCG &&
!CONFIG_SLUB)?
On Wed, Apr 13, 2022 at 12:08:04PM -0700, Andrew Morton wrote:
> On Wed, 13 Apr 2022 22:47:45 +0800 Muchun Song <[email protected]> wrote:
>
> > If the size of "struct page" is not the power of two but with the feature
> > of minimizing overhead of struct page associated with each HugeTLB is
> > enabled, then the vmemmap pages of HugeTLB will be corrupted after
> > remapping (panic is about to happen in theory). But this only exists when
> > !CONFIG_MEMCG && !CONFIG_SLUB on x86_64. However, it is not a conventional
> > configuration nowadays. So it is not a real word issue, just the result
> > of a code review.
>
> The patch does add a whole bunch of tricky junk to address something
> which won't happen. How about we simply disable
> CONFIG_HUGETLB_PAGE_OPTIMIZE_VMEMMAP if (!CONFIG_MEMCG &&
> !CONFIG_SLUB)?
>
I'm afraid not. The size of 'struct page' also depends on
LAST_CPUPID_NOT_IN_PAGE_FLAGS which could be defined
when CONFIG_NODES_SHIFT or CONFIG_KASAN_SW_TAGS
or CONFIG_NR_CPUS is configured with a large value. Then
the size would be more than 64 bytes.
Seems like the approach [1] is more simple and feasible,
which also could prevent the users from doing unexpected
configurations, however, it is objected by Masahiro.
Shall we look back at the approach again?
Thanks.
On Thu, Apr 14, 2022 at 11:10:01AM +0800, Muchun Song wrote:
> On Wed, Apr 13, 2022 at 12:08:04PM -0700, Andrew Morton wrote:
> > On Wed, 13 Apr 2022 22:47:45 +0800 Muchun Song <[email protected]> wrote:
> >
> > > If the size of "struct page" is not the power of two but with the feature
> > > of minimizing overhead of struct page associated with each HugeTLB is
> > > enabled, then the vmemmap pages of HugeTLB will be corrupted after
> > > remapping (panic is about to happen in theory). But this only exists when
> > > !CONFIG_MEMCG && !CONFIG_SLUB on x86_64. However, it is not a conventional
> > > configuration nowadays. So it is not a real word issue, just the result
> > > of a code review.
> >
> > The patch does add a whole bunch of tricky junk to address something
> > which won't happen. How about we simply disable
> > CONFIG_HUGETLB_PAGE_OPTIMIZE_VMEMMAP if (!CONFIG_MEMCG &&
> > !CONFIG_SLUB)?
> >
>
> I'm afraid not. The size of 'struct page' also depends on
> LAST_CPUPID_NOT_IN_PAGE_FLAGS which could be defined
> when CONFIG_NODES_SHIFT or CONFIG_KASAN_SW_TAGS
> or CONFIG_NR_CPUS is configured with a large value. Then
> the size would be more than 64 bytes.
>
> Seems like the approach [1] is more simple and feasible,
Sorry, forgot to post the Link.
[1] https://lore.kernel.org/all/[email protected]/
> which also could prevent the users from doing unexpected
> configurations, however, it is objected by Masahiro.
> Shall we look back at the approach again?
>
Hi all,
Friendly ping.
I have implemented 3 approaches to address this issue.
1) V8 has added a lot of tricky code.
2) V5 has added a feadback from Kbuild to Kconfig, as Masahiro
said, it is terrible.
3) V1 [2] has added a check of is_power_of_2() into hugetlb_vmemmap.c.
Iterated and explored through 8 versions, v1 seems to be the easiest way
to address this. I think reusing v1 may be the best choice now.
What do you think?
[2] https://lore.kernel.org/all/[email protected]/
Thanks.