| Message ID | 20220826092422.39591-9-linmiaohe@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | A few cleanup patches for hugetlb | expand |
> On Aug 26, 2022, at 17:24, Miaohe Lin <linmiaohe@huawei.com> wrote: > > If code reaches here, it's guaranteed that HPageVmemmapOptimized is set > for the hugetlb page (or VM_BUG_ON_PAGE() will complain about it). It's Hi Miaohe, Right. > unnecessary to set it again. No, I suppose you didn’t test this patch since it does not work. The HPageVmemmapOptimized is cleared in the above code of line (set_page_private(page, 0)). So NAck. Thanks, Muchun > > Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> > --- > mm/hugetlb.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > index 7934188bbed0..b432a00061e3 100644 > --- a/mm/hugetlb.c > +++ b/mm/hugetlb.c > @@ -1520,7 +1520,6 @@ static void add_hugetlb_page(struct hstate *h, struct page *page, > > set_compound_page_dtor(page, HUGETLB_PAGE_DTOR); > set_page_private(page, 0); > - SetHPageVmemmapOptimized(page); > > /* > * This page is about to be managed by the hugetlb allocator and > -- > 2.23.0 > >
On 2022/8/27 9:35, Muchun Song wrote: > > >> On Aug 26, 2022, at 17:24, Miaohe Lin <linmiaohe@huawei.com> wrote: >> >> If code reaches here, it's guaranteed that HPageVmemmapOptimized is set >> for the hugetlb page (or VM_BUG_ON_PAGE() will complain about it). It's > > Hi Miaohe, > > Right. > >> unnecessary to set it again. > > No, I suppose you didn’t test this patch since it does not work. The > HPageVmemmapOptimized is cleared in the above code of line > (set_page_private(page, 0)). So NAck. Sorry, I missed that and thanks for pointing this out. Does this code deserves a comment like below? diff --git a/mm/hugetlb.c b/mm/hugetlb.c index d0617d64d718..3374c1d2b52e 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -1522,6 +1522,10 @@ static void add_hugetlb_page(struct hstate *h, struct page *page, set_compound_page_dtor(page, HUGETLB_PAGE_DTOR); set_page_private(page, 0); + /* + * We have to set HPageVmemmapOptimized again as above + * set_page_private(page, 0) cleared it. + */ SetHPageVmemmapOptimized(page); /* Thanks, Miaohe Lin
> On Aug 27, 2022, at 10:18, Miaohe Lin <linmiaohe@huawei.com> wrote: > > On 2022/8/27 9:35, Muchun Song wrote: >> >> >>> On Aug 26, 2022, at 17:24, Miaohe Lin <linmiaohe@huawei.com> wrote: >>> >>> If code reaches here, it's guaranteed that HPageVmemmapOptimized is set >>> for the hugetlb page (or VM_BUG_ON_PAGE() will complain about it). It's >> >> Hi Miaohe, >> >> Right. >> >>> unnecessary to set it again. >> >> No, I suppose you didn’t test this patch since it does not work. The >> HPageVmemmapOptimized is cleared in the above code of line >> (set_page_private(page, 0)). So NAck. > > Sorry, I missed that and thanks for pointing this out. Does this code deserves a > comment like below? > > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > index d0617d64d718..3374c1d2b52e 100644 > --- a/mm/hugetlb.c > +++ b/mm/hugetlb.c > @@ -1522,6 +1522,10 @@ static void add_hugetlb_page(struct hstate *h, struct page *page, > > set_compound_page_dtor(page, HUGETLB_PAGE_DTOR); > set_page_private(page, 0); > + /* > + * We have to set HPageVmemmapOptimized again as above > + * set_page_private(page, 0) cleared it. > + */ > SetHPageVmemmapOptimized(page); I’m fine with this comment. Reviewed-by: Muchun Song <songmuchun@bytedance.com> > > /* > > > Thanks, > Miaohe Lin
diff --git a/mm/hugetlb.c b/mm/hugetlb.c index 7934188bbed0..b432a00061e3 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -1520,7 +1520,6 @@ static void add_hugetlb_page(struct hstate *h, struct page *page, set_compound_page_dtor(page, HUGETLB_PAGE_DTOR); set_page_private(page, 0); - SetHPageVmemmapOptimized(page); /* * This page is about to be managed by the hugetlb allocator and
If code reaches here, it's guaranteed that HPageVmemmapOptimized is set for the hugetlb page (or VM_BUG_ON_PAGE() will complain about it). It's unnecessary to set it again. Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> --- mm/hugetlb.c | 1 - 1 file changed, 1 deletion(-)