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

[Ralph Campbell <rcampbell@nvidia.com>]

On 10/21/19 11:08 AM, Jason Gunthorpe wrote:
> 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.
>>
>> Signed-off-by: Ralph Campbell <rcampbell@nvidia.com>
>> Reviewed-by: Christoph Hellwig <hch@lst.de>
>> Cc: "Jérôme Glisse" <jglisse@redhat.com>
>> Cc: Jason Gunthorpe <jgg@mellanox.com>
>>   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
>> +++ 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;
> 
> Does it make sense to return HMM_PFN_SPECIAL in this case? Does the
> zero pfn have a struct page? Does it need mandatory special treatment?

The zero pfn does not have a struct page so it needs special treatment:
see nouveau_dmem_convert_pfn() where it calls hmm_device_entry_to_page().

If HMM ever ends up supporting VM_PFNMAP
there would need to be a way to distinguish pfns with and without a
backing struct page too.

> ie the base behavior without any driver code should be to dma from the
> zero memory. A fancy driver should be able to detect the zero and do
> something else.

Correct.

> I'm not clear what the two existing users do with PFN_SPECIAL? Nouveau
> looks like it is the same value as error, can't guess what amdgpu does
> with its magic constant
> 
> Jason

I doubt the zero pfn case is being handled correctly in amd/nouveau.
I made the change above when explicitly testing for it in the patch
adding HMM tests.