Re: [PATCH v2 01/11] hugetlb: set hugetlb page flag before optimizing vmemmap

[Mike Kravetz]

On 10/18/23 10:58, Naoya Horiguchi wrote:
> On Mon, Oct 16, 2023 at 08:21:40PM -0700, Mike Kravetz wrote:
> > On 10/13/23 21:58, Naoya Horiguchi wrote:
> > > On Tue, Sep 05, 2023 at 02:44:00PM -0700, Mike Kravetz wrote:
> > Hi Naoya,
> > 
> > I believe we need to set 'rc = 0' in the !folio_test_hugetlb().  I put
> > together the following patch based on mm-stable.  Please take a look.
> 
> Hi Mike, the patch looks good to me, and I confirmed that my test programs
> showed no new problem on latest mm-stable + this patch.
> 

Thank you for testing!

My patch could be simpler/more elegant by eliminating that else clause
and just returning 0 after update_and_free_hugetlb_folio.  Oh well, it
is functionally the same.
-- 
Mike Kravetz


> Thank you very much.
> Naoya Horiguchi
> 
> > 
> > From f19fbfab324d7d17de4a1e814f95ee655950c58e Mon Sep 17 00:00:00 2001
> > From: Mike Kravetz <mike.kravetz@xxxxxxxxxx>
> > Date: Mon, 16 Oct 2023 19:55:49 -0700
> > Subject: [PATCH] hugetlb: check for hugetlb folio before vmemmap_restore
> > 
> > In commit d8f5f7e445f0 ("hugetlb: set hugetlb page flag before
> > optimizing vmemmap") checks were added to print a warning if
> > hugetlb_vmemmap_restore was called on a non-hugetlb page.  This
> > was mostly due to ordering issues in the hugetlb page set up and
> >  tear down sequencees.  One place missed was the routine
> > dissolve_free_huge_page.  Naoya Horiguchi noted: "I saw that
> > VM_WARN_ON_ONCE() in hugetlb_vmemmap_restore is triggered when
> > memory_failure() is called on a free hugetlb page with vmemmap
> > optimization disabled (the warning is not triggered if vmemmap
> > optimization is enabled).  I think that we need check
> > folio_test_hugetlb() before dissolve_free_huge_page() calls
> > hugetlb_vmemmap_restore_folio()."
> > 
> > Perform the check as suggested by Naoya.
> > 
> > Fixes: d8f5f7e445f0 ("hugetlb: set hugetlb page flag before optimizing vmemmap")
> > Suggested-by: Naoya Horiguchi <naoya.horiguchi@xxxxxxxxx>
> > Signed-off-by: Mike Kravetz <mike.kravetz@xxxxxxxxxx>
> > ---
> >  mm/hugetlb.c | 24 +++++++++++++++---------
> >  1 file changed, 15 insertions(+), 9 deletions(-)
> > 
> > diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> > index 36b40bc9ac25..13736cbb2c19 100644
> > --- a/mm/hugetlb.c
> > +++ b/mm/hugetlb.c
> > @@ -2290,17 +2290,23 @@ int dissolve_free_huge_page(struct page *page)
> >  		 * need to adjust max_huge_pages if the page is not freed.
> >  		 * Attempt to allocate vmemmmap here so that we can take
> >  		 * appropriate action on failure.
> > +		 *
> > +		 * The folio_test_hugetlb check here is because
> > +		 * remove_hugetlb_folio will clear hugetlb folio flag for
> > +		 * non-vmemmap optimized hugetlb folios.
> >  		 */
> > -		rc = hugetlb_vmemmap_restore(h, &folio->page);
> > -		if (!rc) {
> > -			update_and_free_hugetlb_folio(h, folio, false);
> > -		} else {
> > -			spin_lock_irq(&hugetlb_lock);
> > -			add_hugetlb_folio(h, folio, false);
> > -			h->max_huge_pages++;
> > -			spin_unlock_irq(&hugetlb_lock);
> > -		}
> > +		if (folio_test_hugetlb(folio)) {
> > +			rc = hugetlb_vmemmap_restore(h, &folio->page);
> > +			if (rc) {
> > +				spin_lock_irq(&hugetlb_lock);
> > +				add_hugetlb_folio(h, folio, false);
> > +				h->max_huge_pages++;
> > +				goto out;
> > +			}
> > +		} else
> > +			rc = 0;
> >  
> > +		update_and_free_hugetlb_folio(h, folio, false);
> >  		return rc;
> >  	}
> >  out:
> > -- 
> > 2.41.0