Re: [PATCH 02/12] pci/p2pdma: Don't initialise page refcount to one

[Alistair Popple]

Dan Williams <dan.j.williams@xxxxxxxxx> writes:

> Alistair Popple wrote:

[...]

>> diff --git a/mm/memremap.c b/mm/memremap.c
>> index 40d4547..07bbe0e 100644
>> --- a/mm/memremap.c
>> +++ b/mm/memremap.c
>> @@ -488,15 +488,24 @@ void free_zone_device_folio(struct folio *folio)
>>  	folio->mapping = NULL;
>>  	folio->page.pgmap->ops->page_free(folio_page(folio, 0));
>>  
>> -	if (folio->page.pgmap->type != MEMORY_DEVICE_PRIVATE &&
>> -	    folio->page.pgmap->type != MEMORY_DEVICE_COHERENT)
>> +	switch (folio->page.pgmap->type) {
>> +	case MEMORY_DEVICE_PRIVATE:
>> +	case MEMORY_DEVICE_COHERENT:
>> +		put_dev_pagemap(folio->page.pgmap);
>> +		break;
>> +
>> +	case MEMORY_DEVICE_FS_DAX:
>> +	case MEMORY_DEVICE_GENERIC:
>>  		/*
>>  		 * Reset the refcount to 1 to prepare for handing out the page
>>  		 * again.
>>  		 */
>>  		folio_set_count(folio, 1);
>> -	else
>> -		put_dev_pagemap(folio->page.pgmap);
>> +		break;
>> +
>> +	case MEMORY_DEVICE_PCI_P2PDMA:
>> +		break;
>
> A follow on cleanup is that either all implementations should be
> put_dev_pagemap(), or none of them. Put the onus on the implementation
> to track how many pages it has handed out in the implementation
> allocator.

Agreed. I've ignored the get/put_dev_pagemap() calls for this clean up
but am planning to do a follow up to clean those up too, probably by
removing them entirely as you suggest.

[...]

> For this one:
>
> Reviewed-by: Dan Williams <dan.j.williams@xxxxxxxxx>

Thanks.