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

Alistair Popple
2024-10-11T00:21:56Z

Dan Williams <dan.j.williams@intel.com> 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@intel.com>

Thanks.