On 2020-05-01 11:20, Jason Gunthorpe wrote:
From: Jason Gunthorpe jgg@mellanox.com
hmm_vma_walk->last is supposed to be updated after every write to the pfns, so that it can be returned by hmm_range_fault(). However, this is not done consistently. Fortunately nothing checks the return code of hmm_range_fault() for anything other than error.
More importantly last must be set before returning -EBUSY as it is used to prevent reading an output pfn as an input flags when the loop restarts.
For clarity and simplicity make hmm_range_fault() return 0 or -ERRNO. Only set last when returning -EBUSY.
Yes, this is also a nice simplification.
... @@ -590,10 +580,13 @@ long hmm_range_fault(struct hmm_range *range) return -EBUSY; ret = walk_page_range(mm, hmm_vma_walk.last, range->end, &hmm_walk_ops, &hmm_vma_walk);
/** When -EBUSY is returned the loop restarts with* hmm_vma_walk.last set to an address that has not been stored* in pfns. All entries < last in the pfn array are set to their* output, and all >= are still at their input values.*/
I'm glad you added that comment. This is much easier to figure out with that in place. After poking around this patch and eventually understanding the .last handling, I wondered if you might like this slightly tweaked wording instead:
/* * Each of the hmm_walk_ops routines returns -EBUSY if and only * hmm_vma_walk.last has been set to an address that has not yet * been stored in pfns. All entries < last in the pfn array are * set to their output, and all >= are still at their input * values. */
Either way,
Reviewed-by: John Hubbard jhubbard@nvidia.com
thanks,