Re: [PATCH v6 21/26] fs/dax: Properly refcount fs dax pages

Alistair Popple
2025-02-07T05:31:56Z

On Mon, Jan 13, 2025 at 07:35:07PM -0800, Dan Williams wrote:
> Alistair Popple wrote:

[...]

> ...and here is that aformentioned patch:

This patch is different from what you originally posted here:
https://yhbt.net/lore/linux-s390/172721874675.497781.3277495908107141898.stgit@dwillia2-xfh.jf.intel.com/

> -- 8< --
> Subject: dcssblk: Mark DAX broken, remove FS_DAX_LIMITED support
>
> From: Dan Williams <dan.j.williams@intel.com>
>
> The dcssblk driver has long needed special case supoprt to enable
> limited dax operation, so called CONFIG_FS_DAX_LIMITED. This mode
> works around the incomplete support for ZONE_DEVICE on s390 by forgoing
> the ability of dax-mapped pages to support GUP.
>
> Now, pending cleanups to fsdax that fix its reference counting [1] depend on
> the ability of all dax drivers to supply ZONE_DEVICE pages.
>
> To allow that work to move forward, dax support needs to be paused for
> dcssblk until ZONE_DEVICE support arrives. That work has been known for
> a few years [2], and the removal of "pte_devmap" requirements [3] makes the
> conversion easier.
>
> For now, place the support behind CONFIG_BROKEN, and remove PFN_SPECIAL
> (dcssblk was the only user).

Specifically it no longer removes PFN_SPECIAL. Was this intentional? Or should I
really have picked up the original patch from the mailing list?

- Alistair

> Link: http://lore.kernel.org/cover.9f0e45d52f5cff58807831b6b867084d0b14b61c.1725941415.git-series.apopple@nvidia.com [1]
> Link: http://lore.kernel.org/20210820210318.187742e8@thinkpad/ [2]
> Link: http://lore.kernel.org/4511465a4f8429f45e2ac70d2e65dc5e1df1eb47.1725941415.git-series.apopple@nvidia.com [3]
> Reviewed-by: Gerald Schaefer <gerald.schaefer@linux.ibm.com>
> Tested-by: Alexander Gordeev <agordeev@linux.ibm.com>
> Acked-by: David Hildenbrand <david@redhat.com>
> Cc: Heiko Carstens <hca@linux.ibm.com>
> Cc: Vasily Gorbik <gor@linux.ibm.com>
> Cc: Christian Borntraeger <borntraeger@linux.ibm.com>
> Cc: Sven Schnelle <svens@linux.ibm.com>
> Cc: Jan Kara <jack@suse.cz>
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Alistair Popple <apopple@nvidia.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
> Documentation/filesystems/dax.rst | 1 -
> drivers/s390/block/Kconfig | 12 ++++++++++--
> drivers/s390/block/dcssblk.c | 27 +++++++++++++++++----------
> 3 files changed, 27 insertions(+), 13 deletions(-)
>
> diff --git a/Documentation/filesystems/dax.rst b/Documentation/filesystems/dax.rst
> index 719e90f1988e..08dd5e254cc5 100644
> --- a/Documentation/filesystems/dax.rst
> +++ b/Documentation/filesystems/dax.rst
> @@ -207,7 +207,6 @@ implement direct_access.
>
> These block devices may be used for inspiration:
> - brd: RAM backed block device driver
> -- dcssblk: s390 dcss block device driver
> - pmem: NVDIMM persistent memory driver
>
>
> diff --git a/drivers/s390/block/Kconfig b/drivers/s390/block/Kconfig
> index e3710a762aba..4bfe469c04aa 100644
> --- a/drivers/s390/block/Kconfig
> +++ b/drivers/s390/block/Kconfig
> @@ -4,13 +4,21 @@ comment "S/390 block device drivers"
>
> config DCSSBLK
> def_tristate m
> - select FS_DAX_LIMITED
> - select DAX
> prompt "DCSSBLK support"
> depends on S390 && BLOCK
> help
> Support for dcss block device
>
> +config DCSSBLK_DAX
> + def_bool y
> + depends on DCSSBLK
> + # requires S390 ZONE_DEVICE support
> + depends on BROKEN
> + select DAX
> + prompt "DCSSBLK DAX support"
> + help
> + Enable DAX operation for the dcss block device
> +
> config DASD
> def_tristate y
> prompt "Support for DASD devices"
> diff --git a/drivers/s390/block/dcssblk.c b/drivers/s390/block/dcssblk.c
> index 0f14d279d30b..7248e547fefb 100644
> --- a/drivers/s390/block/dcssblk.c
> +++ b/drivers/s390/block/dcssblk.c
> @@ -534,6 +534,21 @@ static const struct attribute_group *dcssblk_dev_attr_groups[] = {
> NULL,
> };
>
> +static int dcssblk_setup_dax(struct dcssblk_dev_info *dev_info)
> +{
> + struct dax_device *dax_dev;
> +
> + if (!IS_ENABLED(CONFIG_DCSSBLK_DAX))
> + return 0;
> +
> + dax_dev = alloc_dax(dev_info, &dcssblk_dax_ops);
> + if (IS_ERR(dax_dev))
> + return PTR_ERR(dax_dev);
> + set_dax_synchronous(dax_dev);
> + dev_info->dax_dev = dax_dev;
> + return dax_add_host(dev_info->dax_dev, dev_info->gd);
> +}
> +
> /*
> * device attribute for adding devices
> */
> @@ -547,7 +562,6 @@ dcssblk_add_store(struct device *dev, struct device_attribute *attr, const char
> int rc, i, j, num_of_segments;
> struct dcssblk_dev_info *dev_info;
> struct segment_info *seg_info, *temp;
> - struct dax_device *dax_dev;
> char *local_buf;
> unsigned long seg_byte_size;
>
> @@ -674,14 +688,7 @@ dcssblk_add_store(struct device *dev, struct device_attribute *attr, const char
> if (rc)
> goto put_dev;
>
> - dax_dev = alloc_dax(dev_info, &dcssblk_dax_ops);
> - if (IS_ERR(dax_dev)) {
> - rc = PTR_ERR(dax_dev);
> - goto put_dev;
> - }
> - set_dax_synchronous(dax_dev);
> - dev_info->dax_dev = dax_dev;
> - rc = dax_add_host(dev_info->dax_dev, dev_info->gd);
> + rc = dcssblk_setup_dax(dev_info);
> if (rc)
> goto out_dax;
>
> @@ -917,7 +924,7 @@ __dcssblk_direct_access(struct dcssblk_dev_info *dev_info, pgoff_t pgoff,
> *kaddr = __va(dev_info->start + offset);
> if (pfn)
> *pfn = __pfn_to_pfn_t(PFN_DOWN(dev_info->start + offset),
> - PFN_DEV|PFN_SPECIAL);
> + PFN_DEV);
>
> return (dev_sz - offset) / PAGE_SIZE;
> }