Re: [PATCH v2 2/3] mm/hmm: allow snapshot of the special zero page

[Jerome Glisse <jglisse@xxxxxxxxxx>]

On Tue, Oct 15, 2019 at 01:48:13PM -0700, Ralph Campbell wrote:
> Allow hmm_range_fault() to return success (0) when the CPU pagetable
> entry points to the special shared zero page.
> The caller can then handle the zero page by possibly clearing device
> private memory instead of DMAing a zero page.

I do not understand why you are talking about DMA. GPU can work
on main memory and migrating to GPU memory is optional and should
not involve this function at all.

> 
> Signed-off-by: Ralph Campbell <rcampbell@xxxxxxxxxx>
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
> Cc: "Jérôme Glisse" <jglisse@xxxxxxxxxx>
> Cc: Jason Gunthorpe <jgg@xxxxxxxxxxxx>

NAK please keep semantic or change it fully. See the alternative
below.

> ---
>  mm/hmm.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/hmm.c b/mm/hmm.c
> index 5df0dbf77e89..f62b119722a3 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -530,7 +530,9 @@ static int hmm_vma_handle_pte(struct mm_walk *walk, unsigned long addr,
>  			return -EBUSY;
>  	} else if (IS_ENABLED(CONFIG_ARCH_HAS_PTE_SPECIAL) && pte_special(pte)) {
>  		*pfn = range->values[HMM_PFN_SPECIAL];
> -		return -EFAULT;
> +		if (!is_zero_pfn(pte_pfn(pte)))
> +			return -EFAULT;
> +		return 0;

An acceptable change would be to turn the branch into:
	} else if (IS_ENABLED(CONFIG_ARCH_HAS_PTE_SPECIAL) && pte_special(pte)) {
		if (!is_zero_pfn(pte_pfn(pte))) {
			*pfn = range->values[HMM_PFN_SPECIAL];
			return -EFAULT;
		}
		/* Fall-through for zero pfn (if write was needed the above
		 * hmm_pte_need_faul() would had catched it).
		 */
	}