Re: [PATCH hmm 8/8] mm/hmm: add missing call to hmm_pte_need_fault in HMM_PFN_SPECIAL handling

From: Jason Gunthorpe <jgg@mellanox.com>
To: Christoph Hellwig <hch@lst.de>
Cc: Philip Yang <Philip.Yang@amd.com>,
	Ralph Campbell <rcampbell@nvidia.com>,
	John Hubbard <jhubbard@nvidia.com>,
	Felix.Kuehling@amd.com, dri-devel@lists.freedesktop.org,
	linux-mm@kvack.org, Jerome Glisse <jglisse@redhat.com>,
	amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH  hmm 8/8] mm/hmm: add missing call to hmm_pte_need_fault in HMM_PFN_SPECIAL handling
Date: Tue, 17 Mar 2020 09:53:17 -0300	[thread overview]
Message-ID: <20200317125317.GT13183@mellanox.com> (raw)
In-Reply-To: <20200317123210.GA12058@lst.de>

On Tue, Mar 17, 2020 at 01:32:10PM +0100, Christoph Hellwig wrote:
> On Mon, Mar 16, 2020 at 02:12:01PM +0100, Christoph Hellwig wrote:
> > On Mon, Mar 16, 2020 at 10:04:58AM -0300, Jason Gunthorpe wrote:
> > > > Ok.  I had some cleanups like this based of older trees, but if you are
> > > > active in this area I think I'll let you handle it.
> > > 
> > > You once said you wanted to loose the weird pfn flags scheme, so
> > > before putting hmm_range_fault in ODP I planned to do that.
> > > 
> > > If you have your series someplace send me a URL and I'll look on it
> > 
> > I have a local branch I just started hacking on, but it is rather broken
> > based on various discussions we had.  But for a basic one I'd suggest
> > something like:
> > 
> >  - kill HMM_PFN_SPECIAL as it serves no purpose
> >  - split the ->pfns array into an input flags (optional) and an output
> >    pfn (mandtory) one, using new flags for the input side
> >  - replace the output flags/values indirection with a bunch of values
> >    encoded in the high bits of a u64, with the rest used for the pfn
> 
> Thinking out loud a bit more:
> 
>  - do we really need HMM_PFN_ERROR, or is just a return value from
>    hmm_range_fault enough?

I'm not totally clear on this. The only use for ERROR is to signal to a
non-faulting hmm_range_fault (ie shapshot) that the page should generate a 
device fault (ie device SIGSEGV).

We can also handle ERROR by having the device take the fault to CPU,
then fail during a faulting hmm_range_fault and then dropping the
ERROR indication toward the device.

If we already know the page cannot be faulted when we read it it feels
natural to return that too.

I have a patch, that now needs rebasing, which removes the PFN_ERROR
from the faulting cases. So only non-faulting hmm_range_fault users
will see it. faulting users will always see an errno return.

>  - because if it is we don't need output flags at all, and the output
>    array could be struct pages, which would make for a much easier
>    to use API

valid and write is required for the non-faulting case, I don't
think flags can go away.

Being able to do a non-faulting hmm_range_fault is going to be a big
performance win for ODP, this is my interest here.

Jason
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel