Re: [PATCH v4 2/2] mm/hmm/test: add self tests for HMM

[Jason Gunthorpe <jgg@mellanox.com>]

On Mon, Nov 18, 2019 at 10:32:18AM -0800, Ralph Campbell wrote:
> 
> On 11/15/19 6:06 AM, Jason Gunthorpe wrote:
> > On Thu, Nov 14, 2019 at 03:06:05PM -0800, Ralph Campbell wrote:
> > > 
> > > On 11/13/19 5:51 AM, Christoph Hellwig wrote:
> > > > On Tue, Nov 12, 2019 at 11:45:52PM +0000, Jason Gunthorpe wrote:
> > > > > > Well, it would mean registering for the whole process address space.
> > > > > > I'll give it a try.
> > > > > 
> > > > > I'm not sure it makes much sense that this testing is essentially
> > > > > modeled after nouveau's usage which is very strange compared to the
> > > > > other drivers.
> > > > 
> > > > Which means we really should make the test cases fit the proper usage.
> > > > Maybe defer the tests for 5.5 and just merge the first patch for now?
> > > > 
> > > 
> > > I think this a good point to discuss.
> > > Some devices will want to register for all changes to the process address
> > > space because there is no requirement to preregister regions that the
> > > device can access verses devices like InfiniBand where a range of addresses
> > > have to be registered before the device can access those addresses.
> > 
> > But this is a very bad idea to register and do HW actions for ranges
> > that can't possibly have any pages registered. It slows down the
> > entire application
> > 
> > I think the ODP approach might be saner, when it mirrors the entire
> > address space it chops it up into VA chunks, and once a page is
> > registered on the HW the VA chunk goes into the interval tree.
> > 
> > Presumably the GPU also has some kind of page table tree and you could
> > set one of the levels as the VA interval when there are populated children
> > 
> > Jason
> 
> I wasn't suggesting that HW invalidates happen in two places.
> I'm suggesting the two styles of invalidates can work together.
> For example, what if a driver calls mmu_notifier_register(mn, mm)
> to register for address space wide invalidations, then some time
> later there is a device page table fault and the driver calls
> mmu_range_notifier_insert() but with a NULL ops.invalidate.

I'm saying drivers shouldn't do that, it is a basically a hack that it
works at all.

> The global invalidate() callback would get the device lock and
> call into mm to update the sequence number of any affected ranges
> instead of having a range invalidate callback, and then do the HW
> invalidations.

No, I just finished eliminating all the range iteration code in the
drivers - and you can't update the sequence number from any place
other than the interval invalidation callback anyhow.

Jason