Re: [PATCH mm-unstable 6/8] mm/hugetlb: convert alloc_migrate_huge_page to folios

Mike Kravetz
2023-01-09T18:25:12Z

On 01/09/23 10:26, Sidhartha Kumar wrote:
> On 1/6/23 6:54 PM, Mike Kravetz wrote:
> > On 01/03/23 13:13, Sidhartha Kumar wrote:
> > > Change alloc_huge_page_nodemask() to alloc_hugetlb_folio_nodemask() and
> > > alloc_migrate_huge_page() to alloc_migrate_hugetlb_folio(). Both functions
> > > now return a folio rather than a page.
> >
> > > /* mempolicy aware migration callback */
> > > @@ -2357,16 +2357,16 @@ struct page *alloc_huge_page_vma(struct hstate *h, struct vm_area_struct *vma,
> > > {
> > > struct mempolicy *mpol;
> > > nodemask_t *nodemask;
> > > - struct page *page;
> > > + struct folio *folio;
> > > gfp_t gfp_mask;
> > > int node;
> > > gfp_mask = htlb_alloc_mask(h);
> > > node = huge_node(vma, address, gfp_mask, &mpol, &nodemask);
> > > - page = alloc_huge_page_nodemask(h, node, nodemask, gfp_mask);
> > > + folio = alloc_hugetlb_folio_nodemask(h, node, nodemask, gfp_mask);
> > > mpol_cond_put(mpol);
> > > - return page;
> > > + return &folio->page;
> >
> > Is it possible that folio could be NULL here and cause addressing exception?
> >
> > > diff --git a/mm/migrate.c b/mm/migrate.c
> > > index 6932b3d5a9dd..fab706b78be1 100644
> > > --- a/mm/migrate.c
> > > +++ b/mm/migrate.c
> > > @@ -1622,6 +1622,7 @@ struct page *alloc_migration_target(struct page *page, unsigned long private)
> > > struct migration_target_control *mtc;
> > > gfp_t gfp_mask;
> > > unsigned int order = 0;
> > > + struct folio *hugetlb_folio = NULL;
> > > struct folio *new_folio = NULL;
> > > int nid;
> > > int zidx;
> > > @@ -1636,7 +1637,9 @@ struct page *alloc_migration_target(struct page *page, unsigned long private)
> > > struct hstate *h = folio_hstate(folio);
> > > gfp_mask = htlb_modify_alloc_mask(h, gfp_mask);
> > > - return alloc_huge_page_nodemask(h, nid, mtc->nmask, gfp_mask);
> > > + hugetlb_folio = alloc_hugetlb_folio_nodemask(h, nid,
> > > + mtc->nmask, gfp_mask);
> > > + return &hugetlb_folio->page;
> >
> > and, here as well?
>
> Hi Mike,
>
> It is possible that the folio could be null but I believe these instances
> would not cause an addressing exception because as described in [1],
> &folio->page is safe even if the folio is NULL as the page offset is at 0.
>
>
> [1] https://lore.kernel.org/lkml/Y7h4jsv6jl0XSIsk@casper.infradead.org/T/

Thanks!
I did not follow that thread and did not look closely as to whether
&folio->page was safe with folio == NULL.

I must say that it is going to take me some time to not pause and think
when coming upon &folio->page. Perhaps that is good.

Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
--
Mike Kravetz