If the size of "struct page" is not the power of two and this
feature 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.
But we cannot prevent anyone from configuring that combined
configure. This feature should be disable in this case to fix
this issue.
Signed-off-by: Muchun Song <[email protected]>
---
mm/hugetlb_vmemmap.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/mm/hugetlb_vmemmap.c b/mm/hugetlb_vmemmap.c
index b3118dba0518..49bc7f845438 100644
--- a/mm/hugetlb_vmemmap.c
+++ b/mm/hugetlb_vmemmap.c
@@ -121,6 +121,18 @@ void __init hugetlb_vmemmap_init(struct hstate *h)
if (!hugetlb_free_vmemmap_enabled())
return;
+ if (IS_ENABLED(CONFIG_HUGETLB_PAGE_FREE_VMEMMAP_DEFAULT_ON) &&
+ !is_power_of_2(sizeof(struct page))) {
+ /*
+ * The hugetlb_free_vmemmap_enabled_key can be enabled when
+ * CONFIG_HUGETLB_PAGE_FREE_VMEMMAP_DEFAULT_ON. It should
+ * be disabled if "struct page" crosses page boundaries.
+ */
+ pr_warn_once("cannot free vmemmap pages because \"struct page\" crosses page boundaries\n");
+ static_branch_disable(&hugetlb_free_vmemmap_enabled_key);
+ return;
+ }
+
vmemmap_pages = (nr_pages * sizeof(struct page)) >> PAGE_SHIFT;
/*
* The head page is not to be freed to buddy allocator, the other tail
--
2.11.0
On Mon, Mar 07, 2022 at 09:07:05PM +0800, Muchun Song wrote:
> If the size of "struct page" is not the power of two and this
> feature is enabled, then the vmemmap pages of HugeTLB will be
> corrupted after remapping (panic is about to happen in theory).
Huh what? If a panic is possible best we prevent this in kconfig
all together. I'd instead just put some work into this instead of
adding all this run time hacks.
Can you try to add kconfig magic to detect if a PAGE_SIZE is PO2?
Luis
On Tue, Mar 8, 2022 at 1:03 AM Muchun Song <[email protected]> wrote:
>
> On Tue, Mar 8, 2022 at 12:35 AM Luis Chamberlain <[email protected]> wrote:
> >
> > On Mon, Mar 07, 2022 at 09:07:05PM +0800, Muchun Song wrote:
> > > If the size of "struct page" is not the power of two and this
> > > feature is enabled, then the vmemmap pages of HugeTLB will be
> > > corrupted after remapping (panic is about to happen in theory).
> >
> > Huh what? If a panic is possible best we prevent this in kconfig
> > all together. I'd instead just put some work into this instead of
> > adding all this run time hacks.
>
> If the size of `struct page` is not power of 2, then those lines added
> by this patch will be optimized away by the compiler, therefore there
> is going to be no extra overhead to detect this.
>
> >
> > Can you try to add kconfig magic to detect if a PAGE_SIZE is PO2?
> >
>
> I agree with you that it is better if we can move this check
> into Kconfig. I tried this a few months ago. It is not easy to
> do this. How to check if a `struct page size` is PO2 in
> Kconfig? If you have any thoughts please let me know.
>
> Thanks.
Here is a discussion [1] from a few months ago.
[1] https://lore.kernel.org/all/CAMZfGtWfz8DcwKBLdf3j0x9Dt6ZvOd+MvjX6yXrAoKDeXxW95w@mail.gmail.com/
On Tue, Mar 8, 2022 at 12:35 AM Luis Chamberlain <[email protected]> wrote:
>
> On Mon, Mar 07, 2022 at 09:07:05PM +0800, Muchun Song wrote:
> > If the size of "struct page" is not the power of two and this
> > feature is enabled, then the vmemmap pages of HugeTLB will be
> > corrupted after remapping (panic is about to happen in theory).
>
> Huh what? If a panic is possible best we prevent this in kconfig
> all together. I'd instead just put some work into this instead of
> adding all this run time hacks.
If the size of `struct page` is not power of 2, then those lines added
by this patch will be optimized away by the compiler, therefore there
is going to be no extra overhead to detect this.
>
> Can you try to add kconfig magic to detect if a PAGE_SIZE is PO2?
>
I agree with you that it is better if we can move this check
into Kconfig. I tried this a few months ago. It is not easy to
do this. How to check if a `struct page size` is PO2 in
Kconfig? If you have any thoughts please let me know.
Thanks.
On Fri, Mar 11, 2022 at 5:31 AM Luis Chamberlain <[email protected]> wrote:
>
> On Tue, Mar 08, 2022 at 01:03:08AM +0800, Muchun Song wrote:
> > On Tue, Mar 8, 2022 at 12:35 AM Luis Chamberlain <[email protected]> wrote:
> > >
> > > On Mon, Mar 07, 2022 at 09:07:05PM +0800, Muchun Song wrote:
> > > > If the size of "struct page" is not the power of two and this
> > > > feature is enabled, then the vmemmap pages of HugeTLB will be
> > > > corrupted after remapping (panic is about to happen in theory).
> > >
> > > Huh what? If a panic is possible best we prevent this in kconfig
> > > all together. I'd instead just put some work into this instead of
> > > adding all this run time hacks.
> >
> > If the size of `struct page` is not power of 2, then those lines added
> > by this patch will be optimized away by the compiler, therefore there
> > is going to be no extra overhead to detect this.
> >
> > >
> > > Can you try to add kconfig magic to detect if a PAGE_SIZE is PO2?
> > >
> >
> > I agree with you that it is better if we can move this check
> > into Kconfig. I tried this a few months ago. It is not easy to
> > do this. How to check if a `struct page size` is PO2 in
> > Kconfig? If you have any thoughts please let me know.
>
> Can you query this with a script?
>
> config HAS_PAGE_SIZE_PO2
> bool
> default $(shell, scripts/check_po2_page_size.sh arguments_are_allowed)
>
Excellent. I'll try this approach.
Thanks very much.
On Tue, Mar 08, 2022 at 01:03:08AM +0800, Muchun Song wrote:
> On Tue, Mar 8, 2022 at 12:35 AM Luis Chamberlain <[email protected]> wrote:
> >
> > On Mon, Mar 07, 2022 at 09:07:05PM +0800, Muchun Song wrote:
> > > If the size of "struct page" is not the power of two and this
> > > feature is enabled, then the vmemmap pages of HugeTLB will be
> > > corrupted after remapping (panic is about to happen in theory).
> >
> > Huh what? If a panic is possible best we prevent this in kconfig
> > all together. I'd instead just put some work into this instead of
> > adding all this run time hacks.
>
> If the size of `struct page` is not power of 2, then those lines added
> by this patch will be optimized away by the compiler, therefore there
> is going to be no extra overhead to detect this.
>
> >
> > Can you try to add kconfig magic to detect if a PAGE_SIZE is PO2?
> >
>
> I agree with you that it is better if we can move this check
> into Kconfig. I tried this a few months ago. It is not easy to
> do this. How to check if a `struct page size` is PO2 in
> Kconfig? If you have any thoughts please let me know.
Can you query this with a script?
config HAS_PAGE_SIZE_PO2
bool
default $(shell, scripts/check_po2_page_size.sh arguments_are_allowed)
Luis