| Message ID | 20230719063132.37676-1-songmuchun@bytedance.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm: hugetlb_vmemmap: use PageCompound() instead of PageReserved() | expand |
Hi, On 19.7.2023 9.31, Muchun Song wrote: > The ckeck of PageReserved() is easy to be broken in the future, PageCompound() > is more stable to check if the page should be split. > > Signed-off-by: Muchun Song <songmuchun@bytedance.com> > --- > mm/hugetlb_vmemmap.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/mm/hugetlb_vmemmap.c b/mm/hugetlb_vmemmap.c > index 4b9734777f69..8068fe890f52 100644 > --- a/mm/hugetlb_vmemmap.c > +++ b/mm/hugetlb_vmemmap.c > @@ -73,8 +73,10 @@ static int split_vmemmap_huge_pmd(pmd_t *pmd, unsigned long start) > * be treated as indepdenent small pages (as they can be freed > * individually). > */ > - if (!PageReserved(head)) > + if (PageCompound(head)) { > + VM_BUG_ON(compound_order(head) != get_order(PMD_SIZE)); > split_page(head, get_order(PMD_SIZE)); I think vmemmap pages are not compound pages (even order > 0). They would bug on here trying to split_page() : void split_page(struct page *page, unsigned int order) { int i; VM_BUG_ON_PAGE(PageCompound(page), page); > + } > > /* Make pte visible before pmd. See comment in pmd_install(). */ > smp_wmb(); --Mika
> On Jul 19, 2023, at 14:51, Mika Penttilä <mpenttil@redhat.com> wrote: > > Hi, > > > On 19.7.2023 9.31, Muchun Song wrote: >> The ckeck of PageReserved() is easy to be broken in the future, PageCompound() >> is more stable to check if the page should be split. >> Signed-off-by: Muchun Song <songmuchun@bytedance.com> >> --- >> mm/hugetlb_vmemmap.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> diff --git a/mm/hugetlb_vmemmap.c b/mm/hugetlb_vmemmap.c >> index 4b9734777f69..8068fe890f52 100644 >> --- a/mm/hugetlb_vmemmap.c >> +++ b/mm/hugetlb_vmemmap.c >> @@ -73,8 +73,10 @@ static int split_vmemmap_huge_pmd(pmd_t *pmd, unsigned long start) >> * be treated as indepdenent small pages (as they can be freed >> * individually). >> */ >> - if (!PageReserved(head)) >> + if (PageCompound(head)) { >> + VM_BUG_ON(compound_order(head) != get_order(PMD_SIZE)); >> split_page(head, get_order(PMD_SIZE)); > > I think vmemmap pages are not compound pages (even order > 0). > They would bug on here trying to split_page() : You are right. I have missed this. Now I remember why I use PageReserved() instead of PageCompound() when I fist submit the commit 39d35edee453. Thanks for your reminder. Sorry for the noise. Please ignore this patch. Thanks. > > > void split_page(struct page *page, unsigned int order) > { > int i; > > VM_BUG_ON_PAGE(PageCompound(page), page); > > >> + } >> /* Make pte visible before pmd. See comment in pmd_install(). */ >> smp_wmb(); > > --Mika >
diff --git a/mm/hugetlb_vmemmap.c b/mm/hugetlb_vmemmap.c index 4b9734777f69..8068fe890f52 100644 --- a/mm/hugetlb_vmemmap.c +++ b/mm/hugetlb_vmemmap.c @@ -73,8 +73,10 @@ static int split_vmemmap_huge_pmd(pmd_t *pmd, unsigned long start) * be treated as indepdenent small pages (as they can be freed * individually). */ - if (!PageReserved(head)) + if (PageCompound(head)) { + VM_BUG_ON(compound_order(head) != get_order(PMD_SIZE)); split_page(head, get_order(PMD_SIZE)); + } /* Make pte visible before pmd. See comment in pmd_install(). */ smp_wmb();
The ckeck of PageReserved() is easy to be broken in the future, PageCompound() is more stable to check if the page should be split. Signed-off-by: Muchun Song <songmuchun@bytedance.com> --- mm/hugetlb_vmemmap.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)