Re: [PATCH v6 22/26] device/dax: Properly refcount device dax pages when mapping

Dan Williams
2025-01-14T06:14:25Z

Alistair Popple wrote:
> Device DAX pages are currently not reference counted when mapped,
> instead relying on the devmap PTE bit to ensure mapping code will not
> get/put references. This requires special handling in various page
> table walkers, particularly GUP, to manage references on the
> underlying pgmap to ensure the pages remain valid.
>
> However there is no reason these pages can't be refcounted properly at
> map time. Doning so eliminates the need for the devmap PTE bit,
> freeing up a precious PTE bit. It also simplifies GUP as it no longer
> needs to manage the special pgmap references and can instead just
> treat the pages normally as defined by vm_normal_page().
>
> Signed-off-by: Alistair Popple <apopple@nvidia.com>
> ---
> drivers/dax/device.c | 15 +++++++++------
> mm/memremap.c | 13 ++++++-------
> 2 files changed, 15 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/dax/device.c b/drivers/dax/device.c
> index 6d74e62..fd22dbf 100644
> --- a/drivers/dax/device.c
> +++ b/drivers/dax/device.c
> @@ -126,11 +126,12 @@ static vm_fault_t __dev_dax_pte_fault(struct dev_dax *dev_dax,
> return VM_FAULT_SIGBUS;
> }
>
> - pfn = phys_to_pfn_t(phys, PFN_DEV|PFN_MAP);
> + pfn = phys_to_pfn_t(phys, 0);
>
> dax_set_mapping(vmf, pfn, fault_size);
>
> - return vmf_insert_mixed(vmf->vma, vmf->address, pfn);
> + return vmf_insert_page_mkwrite(vmf, pfn_t_to_page(pfn),
> + vmf->flags & FAULT_FLAG_WRITE);
> }
>
> static vm_fault_t __dev_dax_pmd_fault(struct dev_dax *dev_dax,
> @@ -169,11 +170,12 @@ static vm_fault_t __dev_dax_pmd_fault(struct dev_dax *dev_dax,
> return VM_FAULT_SIGBUS;
> }
>
> - pfn = phys_to_pfn_t(phys, PFN_DEV|PFN_MAP);
> + pfn = phys_to_pfn_t(phys, 0);
>
> dax_set_mapping(vmf, pfn, fault_size);
>
> - return vmf_insert_pfn_pmd(vmf, pfn, vmf->flags & FAULT_FLAG_WRITE);
> + return vmf_insert_folio_pmd(vmf, page_folio(pfn_t_to_page(pfn)),
> + vmf->flags & FAULT_FLAG_WRITE);

This looks suspect without initializing the compound page metadata.

This might be getting compound pages by default with
CONFIG_ARCH_WANT_OPTIMIZE_DAX_VMEMMAP. The device-dax unit tests are ok
so far, but that is not super comforting until I can think about this a
bit more... but not tonight.

Might as well fix up device-dax refcounts in this series too, but I
won't ask you to do that, will send you something to include.