Received-SPF: pass (google.com: domain of linux-kernel+bounces-76975-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Precedence: bulk
MIME-Version: 1.0
References: <20240221234732.187629-1-vishal.moola@gmail.com>
 <20240221234732.187629-6-vishal.moola@gmail.com> <ZdbFyC6HPr6oKrM-@casper.infradead.org>
In-Reply-To: <ZdbFyC6HPr6oKrM-@casper.infradead.org>
From: Vishal Moola <vishal.moola@gmail.com>
Date: Thu, 22 Feb 2024 09:27:00 -0800
Message-ID: <CAOzc2pyt=3UwRt6a8L-wJGAcQFJr_ZNeFEwiGapx9KktuYC+BQ@mail.gmail.com>
Subject: Re: [PATCH v2 5/5] hugetlb: Allow faults to be handled under the VMA lock
To: Matthew Wilcox <willy@infradead.org>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, 
	akpm@linux-foundation.org, muchun.song@linux.dev, 
	Cyril Hrubis <chrubis@suse.cz>, Jan Stancek <jstancek@redhat.com>, Petr Vorel <pvorel@suse.cz>, 
	ltp@lists.linux.it
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Feb 21, 2024 at 7:55=E2=80=AFPM Matthew Wilcox <willy@infradead.org=
> wrote:
>
> On Wed, Feb 21, 2024 at 03:47:32PM -0800, Vishal Moola (Oracle) wrote:
> > Hugetlb can now safely handle faults under the VMA lock, so allow it to
> > do so.
> >
> > This patch may cause ltp hugemmap10 to "fail". Hugemmap10 tests hugetlb
> > counters, and expects the counters to remain unchanged on failure to
> > handle a fault.
> >
> > In hugetlb_no_page(), vmf_anon_prepare() may bailout with no anon_vma
> > under the VMA lock after allocating a folio for the hugepage. In
> > free_huge_folio(), this folio is completely freed on bailout iff there
> > is a surplus of hugetlb pages. This will remove a folio off the freelis=
t
> > and decrement the number of hugepages while ltp expects these counters
> > to remain unchanged on failure.
> >
> > Originally this could only happen due to OOM failures, but now it may
> > also occur after we allocate a hugetlb folio without a suitable anon_vm=
a
> > under the VMA lock. This should only happen for the first freshly
> > allocated hugepage in this vma.
>
> Hmm, so it's a bug in LTP.  Have you talked to the LTP people about it
> (cc's added)?
>
> Also, did you try moving the anon_vma check befor the folio allocation
> so that you can bail out without disturbing the counters?

Moving the check before folio allocation occurs keeps the folios on the
freelist so the counters remain static as expected.

If we are looking at a shareable vma, hugetlb_no_page() does not need
this check at all, so I left the check where it is. We could definitely pla=
ce
the anon_vma check above allocation, it would just make the code
slightly more complicated - needing another variable (or reassigning
ret in various places) as well as adding another potentially unnecessary
vma flag check.

> > Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
>
> Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>