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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to