On 08/25/22 15:31, Mike Kravetz wrote:
> On 08/25/22 15:35, Joao Martins wrote:
> > On 8/12/22 06:36, Muchun Song wrote:
> > >> On Aug 11, 2022, at 06:38, Mike Kravetz <mike.kravetz@xxxxxxxxxx> wrote:
> > >> On 08/10/22 14:20, Muchun Song wrote:
> > >>>> On Aug 9, 2022, at 05:28, Mike Kravetz <mike.kravetz@xxxxxxxxxx> wrote:
> <snip>
> > >>>> There have been proposals to change at least the buddy allocator to
> > >>>> return frozen pages as described at [3]. If such a change is made, it
> > >>>> can be employed by the hugetlb code. However, as mentioned above
> > >>>> hugetlb uses several low level allocators so each would need to be
> > >>>> modified to return frozen pages. For now, we can manually freeze the
> > >>>> returned pages. This is done in two places:
> > >>>> 1) alloc_buddy_huge_page, only the returned head page is ref counted.
> > >>>> We freeze the head page, retrying once in the VERY rare case where
> > >>>> there may be an inflated ref count.
> > >>>> 2) prep_compound_gigantic_page, for gigantic pages the current code
> > >>>> freezes all pages except the head page. New code will simply freeze
> > >>>> the head page as well.
> <snip>
> > >>>> /*
> > >>>> * By default we always try hard to allocate the page with
> > >>>> @@ -1933,7 +1934,21 @@ static struct page *alloc_buddy_huge_page(struct hstate *h,
> > >>>> gfp_mask |= __GFP_RETRY_MAYFAIL;
> > >>>> if (nid == NUMA_NO_NODE)
> > >>>> nid = numa_mem_id();
> > >>>> +retry:
> > >>>> page = __alloc_pages(gfp_mask, order, nid, nmask);
> > >>>> +
> > >>>> + /* Freeze head page */
> > >>>> + if (!page_ref_freeze(page, 1)) {
> > >>>
> > >>> Hi Mike,
> > >>>
> > >>> I saw Mattew has introduced a new helper alloc_frozen_pages() in thread [1] to allocate a
> > >>> frozen page. Then we do not need to handle an unexpected refcount case, which
> > >>> should be easy. Is there any consideration why we do not choose alloc_frozen_pages()?
> > >>
> > >> I asked Matthew about these efforts before creating this patch. At the
> > >> time, there were issues with the first version of his patch series and
> > >> he wasn't sure when he would get around to looking into those issues.
> > >>
> > >> I then decided to proceed with manually freezing pages after allocation.
> > >> The thought was that alloc_frozen_pages() could be added when it became
> > >> available. The 'downstream changes' to deal with pages that have zero
> > >> ref count should remain the same. IMO, these downstream changes are the
> > >> more important parts of this patch.
> > >>
> > >> Shortly after sending this patch, Matthew took another look at his
> > >> series and discovered the source of issues. He then sent this v2
> > >> series. Matthew will correct me if this is not accurate.
> > >
> > > Thanks Mike, it is really clear to me.
> > >
> > >>
> > >>>
> > >>> [1] https://lore.kernel.org/linux-mm/20220809171854.3725722-15-willy@xxxxxxxxxxxxx/T/#u
> > >>>
> > >>
> > >> I am happy to wait until Matthew's series moves forward, and then use
> > >> the new interface.
> > >
> > > I agree. Let’s wait together.
> >
> > Maybe this is a bit of bad suggestion, but considering that the enterity of kernels
> > supporting hugetlb vmemmap optimization are using the old interface (the new one isn't yet
> > settled it seems?) would it be better to go with something this patch, and then have a
> > second patch (simpler one) that just switches to the new interface?
> >
>
> My thoughts.
> Right now, the earliest any of this code would be merged is in 6.1. If
> the alloc_frozen_pages interface gors into 6.1, then it would make sense
> to just make this patch/change use it. If not, we can manually freeze as
> done here, and switch when alloc_frozen_pages becomes available. In either
> case, this could/should go into 6.1.
>
> We still have a bit of time to see if alloc_frozen_pages will make 6.1.
> Ideally, we would wait and see. Ideally, I (we) would help with Matthew's
> series. However, I am a bit concerned that we may forget about pushing this
> forward and miss the window. And, since Joao's optimization depends on this,
> that would miss the window as well.
>
> Matthew, any thoughts on the likelihood of alloc_frozen_pages going into 6.1?
Unless someone objects, my plan is to send an updated verstion of this
patch (fixing __prep_compound_gigantic_page issue, thanks Miaohe!). We
can then switch to using alloc_frozen_pages when it becomes available.
With this patch in place, Joao should be able to send an updated version of
the patch "mm/hugetlb_vmemmap: remap head page to newly allocated page".
--
Mike Kravetz
