Received-SPF: pass (google.com: domain of linux-kernel+bounces-75234-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Wed, 21 Feb 2024 17:55:17 +0000
From: Matthew Wilcox <willy@infradead.org>
To: Vishal Moola <vishal.moola@gmail.com>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	akpm@linux-foundation.org, muchun.song@linux.dev
Subject: Re: [PATCH 2/3] hugetlb: Use vmf_anon_prepare() instead of
 anon_vma_prepare()
Message-ID: <ZdY5BfO3EdcnJM99@casper.infradead.org>
References: <20240220231424.126600-1-vishal.moola@gmail.com>
 <20240220231424.126600-3-vishal.moola@gmail.com>
 <ZdVx_Jr9tm2lV0Ku@casper.infradead.org>
 <CAOzc2pzLbiEpMuBpX7xXhZqPc0S6ZMnziT2uUFsa0tZe6yJQ0g@mail.gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAOzc2pzLbiEpMuBpX7xXhZqPc0S6ZMnziT2uUFsa0tZe6yJQ0g@mail.gmail.com>

On Wed, Feb 21, 2024 at 09:15:51AM -0800, Vishal Moola wrote:
> > >       unsigned long haddr = address & huge_page_mask(h);
> > >       struct mmu_notifier_range range;
> > > +     struct vm_fault vmf = {
> > > +                             .vma = vma,
> > > +                             .address = haddr,
> > > +                             .real_address = address,
> > > +                             .flags = flags,
> > > +     };
> >
> > We don't usually indent quite so far.  One extra tab would be enough.
> >
> > Also, I thought we talked about creating the vmf in hugetlb_fault(),
> > then passing it to hugetlb_wp() hugetlb_no_page() and handle_userfault()?
> > Was there a reason to abandon that idea?
> 
> No I haven't abandoned that idea, I intend to have a separate patchset to go
> on top of this one - just keeping them separate since they are conceptually
> different. I'm converting each function to use struct vm_fault first, then
> shifting it to be passed throughout as an arguement while cleaning up the
> excess variables laying around. In a sense working bottom-up instead
> of top-down.

I think you'll find it less work to create it in hugetlb_fault()
first.  ie patch 2 could be to hoist its creation from half-way down
hugetlb_fault to the top of hugetlb_fault.  Patch 3 could pass it
through hugetlb_no_page() to hugetlb_handle_userfault() and remove its
creation there.  Now you've alreedy got it, and can make use of it in
this patch which would be the new patch 4.

If you want to do a cleanup patch afterwards, you could hoist the vmf
creation all the way to handle_mm_fault() ;-)