[PATCH hmm 2/6] mm/hmm: return the fault type from hmm_pte_need_fault()

[Christoph Hellwig]

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..