On Fri, Mar 20, 2020 at 01:49:01PM -0300, Jason Gunthorpe wrote: > +enum { > + NEED_FAULT = 1 << 0, > + NEED_WRITE_FAULT = 1 << 1, > +}; Maybe add a HMM_ prefix? > for (i = 0; i < npages; ++i) { > + required_fault |= > + hmm_pte_need_fault(hmm_vma_walk, pfns[i], cpu_flags); > + if (required_fault == (NEED_FAULT | NEED_WRITE_FAULT)) > + return required_fault; No need for the inner braces. > @@ -532,17 +515,15 @@ static int hmm_vma_walk_test(unsigned long start, unsigned long end, > */ > if ((vma->vm_flags & (VM_IO | VM_PFNMAP | VM_MIXEDMAP)) || > !(vma->vm_flags & VM_READ)) { > - bool fault, write_fault; > - No that there is no need for local variables I'd invert the test and return early: if ((vma->vm_flags & VM_READ) && !(vma->vm_flags & (VM_IO | VM_PFNMAP | VM_MIXEDMAP)) return 0; > /* > * Check to see if a fault is requested for any page in the > * range. > */ > - hmm_range_need_fault(hmm_vma_walk, range->pfns + > - ((start - range->start) >> PAGE_SHIFT), > - (end - start) >> PAGE_SHIFT, > - 0, &fault, &write_fault); > - if (fault || write_fault) > + if (hmm_range_need_fault(hmm_vma_walk, > + range->pfns + > + ((start - range->start) >> > + PAGE_SHIFT), > + (end - start) >> PAGE_SHIFT, 0)) Which should help to make this a little more readable.. _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx