On Mon, Mar 16, 2020 at 02:53:09PM +0100, Christoph Hellwig wrote: > There is just a single caller using hmm_vma_walk_hole_ for the non-fault > case. Use hmm_pfns_fill to fill the whole pfn array with zeroes in only > caller for the non-fault case and remove the non-fault path from > hmm_vma_walk_hole_. > > Also rename the function to hmm_vma_fault to better describe what it > does. > > Signed-off-by: Christoph Hellwig <hch@xxxxxx> > --- > mm/hmm.c | 38 +++++++++++++++++++------------------- > 1 file changed, 19 insertions(+), 19 deletions(-) Ah, I did nearly the same in my the next series, I'm cooking: https://github.com/jgunthorpe/linux/commit/078e10ca5919f2c263c245784fb5fe63ddbb61f4 But this can go first, I was probably going to break mine up anyhow. > + pfns[i] = range->values[HMM_PFN_NONE]; Since the function always returns -EBUSY now, it should not set an output. This is an existing bug. Since there is no purpose now that the caller does the fill, this patch should drop the above and have the fixes line. > static inline void hmm_pte_need_fault(const struct hmm_vma_walk *hmm_vma_walk, > @@ -193,7 +190,10 @@ static int hmm_vma_walk_hole(unsigned long addr, unsigned long end, > pfns = &range->pfns[i]; > hmm_range_need_fault(hmm_vma_walk, pfns, npages, > 0, &fault, &write_fault); > - return hmm_vma_walk_hole_(addr, end, fault, write_fault, walk); > + if (fault || write_fault) > + return hmm_vma_fault(addr, end, fault, write_fault, walk); > + hmm_vma_walk->last = addr; > + return hmm_pfns_fill(addr, end, range, HMM_PFN_NONE); This is also the only caller of hmm_vma_fault() that could have walk->vma == NULL, and NULL vma + a fault == -EFAULT Jason
