nvdimm.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* dax_supported() related cleanups v2
@ 2021-09-22 17:34 Christoph Hellwig
  2021-09-22 17:34 ` [PATCH 1/3] nvdimm/pmem: fix creating the dax group Christoph Hellwig
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Christoph Hellwig @ 2021-09-22 17:34 UTC (permalink / raw)
  To: Dan Williams, Vishal Verma, Dave Jiang
  Cc: Mike Snitzer, Matthew Wilcox, linux-xfs, nvdimm, linux-fsdevel,
	linux-ext4

Hi all,

this series first clarifies how to use fsdax in the Kconfig help a bit,
and then untangles the code path that checks if fsdax is supported.

Changes since v1:
 - improve the FS_DAX Kconfig help text further
 - write a proper commit log for a patch missing it

Diffstat
 drivers/dax/super.c   |  191 +++++++++++++++++++-------------------------------
 drivers/md/dm-table.c |    9 --
 drivers/md/dm.c       |    2 
 fs/Kconfig            |   21 ++++-
 fs/ext2/super.c       |    3 
 fs/ext4/super.c       |    3 
 fs/xfs/xfs_super.c    |   16 +++-
 include/linux/dax.h   |   41 +---------
 8 files changed, 117 insertions(+), 169 deletions(-)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/3] nvdimm/pmem: fix creating the dax group
  2021-09-22 17:34 dax_supported() related cleanups v2 Christoph Hellwig
@ 2021-09-22 17:34 ` Christoph Hellwig
  2021-09-22 17:34 ` [PATCH 2/3] nvdimm/pmem: move dax_attribute_group from dax to pmem Christoph Hellwig
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 14+ messages in thread
From: Christoph Hellwig @ 2021-09-22 17:34 UTC (permalink / raw)
  To: Dan Williams, Vishal Verma, Dave Jiang
  Cc: Mike Snitzer, Matthew Wilcox, linux-xfs, nvdimm, linux-fsdevel,
	linux-ext4, Ira Weiny

The recent block layer refactoring broke the way how the pmem driver
abused device_add_disk.  Fix this by properly passing the attribute groups
to device_add_disk.

Fixes: 52b85909f85d ("block: fold register_disk into device_add_disk")
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Ira Weiny <ira.weiny@intel.com>
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/nvdimm/pmem.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index 72de88ff0d30d..ef4950f808326 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -380,7 +380,6 @@ static int pmem_attach_disk(struct device *dev,
 	struct nd_pfn_sb *pfn_sb;
 	struct pmem_device *pmem;
 	struct request_queue *q;
-	struct device *gendev;
 	struct gendisk *disk;
 	void *addr;
 	int rc;
@@ -489,10 +488,8 @@ static int pmem_attach_disk(struct device *dev,
 	}
 	dax_write_cache(dax_dev, nvdimm_has_cache(nd_region));
 	pmem->dax_dev = dax_dev;
-	gendev = disk_to_dev(disk);
-	gendev->groups = pmem_attribute_groups;
 
-	device_add_disk(dev, disk, NULL);
+	device_add_disk(dev, disk, pmem_attribute_groups);
 	if (devm_add_action_or_reset(dev, pmem_release_disk, pmem))
 		return -ENOMEM;
 
-- 
2.30.2


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 2/3] nvdimm/pmem: move dax_attribute_group from dax to pmem
  2021-09-22 17:34 dax_supported() related cleanups v2 Christoph Hellwig
  2021-09-22 17:34 ` [PATCH 1/3] nvdimm/pmem: fix creating the dax group Christoph Hellwig
@ 2021-09-22 17:34 ` Christoph Hellwig
  2021-09-22 17:34 ` [PATCH 3/3] block: warn if ->groups is set when calling add_disk Christoph Hellwig
  2021-09-22 17:55 ` dax_supported() related cleanups v2 Dan Williams
  3 siblings, 0 replies; 14+ messages in thread
From: Christoph Hellwig @ 2021-09-22 17:34 UTC (permalink / raw)
  To: Dan Williams, Vishal Verma, Dave Jiang
  Cc: Mike Snitzer, Matthew Wilcox, linux-xfs, nvdimm, linux-fsdevel,
	linux-ext4

dax_attribute_group is only used by the pmem driver, and can avoid the
completely pointless lookup by the disk name if moved there.  This
leaves just a single caller of dax_get_by_host, so move dax_get_by_host
into the same ifdef block as that caller.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/dax/super.c   | 100 ++++++++----------------------------------
 drivers/nvdimm/pmem.c |  43 ++++++++++++++++++
 include/linux/dax.h   |   2 -
 3 files changed, 61 insertions(+), 84 deletions(-)

diff --git a/drivers/dax/super.c b/drivers/dax/super.c
index fc89e91beea7c..b882cf8106ea3 100644
--- a/drivers/dax/super.c
+++ b/drivers/dax/super.c
@@ -63,6 +63,24 @@ static int dax_host_hash(const char *host)
 	return hashlen_hash(hashlen_string("DAX", host)) % DAX_HASH_SIZE;
 }
 
+#ifdef CONFIG_BLOCK
+#include <linux/blkdev.h>
+
+int bdev_dax_pgoff(struct block_device *bdev, sector_t sector, size_t size,
+		pgoff_t *pgoff)
+{
+	sector_t start_sect = bdev ? get_start_sect(bdev) : 0;
+	phys_addr_t phys_off = (start_sect + sector) * 512;
+
+	if (pgoff)
+		*pgoff = PHYS_PFN(phys_off);
+	if (phys_off % PAGE_SIZE || size % PAGE_SIZE)
+		return -EINVAL;
+	return 0;
+}
+EXPORT_SYMBOL(bdev_dax_pgoff);
+
+#if IS_ENABLED(CONFIG_FS_DAX)
 /**
  * dax_get_by_host() - temporary lookup mechanism for filesystem-dax
  * @host: alternate name for the device registered by a dax driver
@@ -94,24 +112,6 @@ static struct dax_device *dax_get_by_host(const char *host)
 	return found;
 }
 
-#ifdef CONFIG_BLOCK
-#include <linux/blkdev.h>
-
-int bdev_dax_pgoff(struct block_device *bdev, sector_t sector, size_t size,
-		pgoff_t *pgoff)
-{
-	sector_t start_sect = bdev ? get_start_sect(bdev) : 0;
-	phys_addr_t phys_off = (start_sect + sector) * 512;
-
-	if (pgoff)
-		*pgoff = PHYS_PFN(phys_off);
-	if (phys_off % PAGE_SIZE || size % PAGE_SIZE)
-		return -EINVAL;
-	return 0;
-}
-EXPORT_SYMBOL(bdev_dax_pgoff);
-
-#if IS_ENABLED(CONFIG_FS_DAX)
 struct dax_device *fs_dax_get_by_bdev(struct block_device *bdev)
 {
 	if (!blk_queue_dax(bdev->bd_disk->queue))
@@ -231,70 +231,6 @@ enum dax_device_flags {
 	DAXDEV_SYNC,
 };
 
-static ssize_t write_cache_show(struct device *dev,
-		struct device_attribute *attr, char *buf)
-{
-	struct dax_device *dax_dev = dax_get_by_host(dev_name(dev));
-	ssize_t rc;
-
-	WARN_ON_ONCE(!dax_dev);
-	if (!dax_dev)
-		return -ENXIO;
-
-	rc = sprintf(buf, "%d\n", !!dax_write_cache_enabled(dax_dev));
-	put_dax(dax_dev);
-	return rc;
-}
-
-static ssize_t write_cache_store(struct device *dev,
-		struct device_attribute *attr, const char *buf, size_t len)
-{
-	bool write_cache;
-	int rc = strtobool(buf, &write_cache);
-	struct dax_device *dax_dev = dax_get_by_host(dev_name(dev));
-
-	WARN_ON_ONCE(!dax_dev);
-	if (!dax_dev)
-		return -ENXIO;
-
-	if (rc)
-		len = rc;
-	else
-		dax_write_cache(dax_dev, write_cache);
-
-	put_dax(dax_dev);
-	return len;
-}
-static DEVICE_ATTR_RW(write_cache);
-
-static umode_t dax_visible(struct kobject *kobj, struct attribute *a, int n)
-{
-	struct device *dev = container_of(kobj, typeof(*dev), kobj);
-	struct dax_device *dax_dev = dax_get_by_host(dev_name(dev));
-
-	WARN_ON_ONCE(!dax_dev);
-	if (!dax_dev)
-		return 0;
-
-#ifndef CONFIG_ARCH_HAS_PMEM_API
-	if (a == &dev_attr_write_cache.attr)
-		return 0;
-#endif
-	return a->mode;
-}
-
-static struct attribute *dax_attributes[] = {
-	&dev_attr_write_cache.attr,
-	NULL,
-};
-
-struct attribute_group dax_attribute_group = {
-	.name = "dax",
-	.attrs = dax_attributes,
-	.is_visible = dax_visible,
-};
-EXPORT_SYMBOL_GPL(dax_attribute_group);
-
 /**
  * dax_direct_access() - translate a device pgoff to an absolute pfn
  * @dax_dev: a dax_device instance representing the logical memory range
diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index ef4950f808326..bbeb3f46db157 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -328,6 +328,49 @@ static const struct dax_operations pmem_dax_ops = {
 	.zero_page_range = pmem_dax_zero_page_range,
 };
 
+static ssize_t write_cache_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct pmem_device *pmem = dev_to_disk(dev)->private_data;
+
+	return sprintf(buf, "%d\n", !!dax_write_cache_enabled(pmem->dax_dev));
+}
+
+static ssize_t write_cache_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t len)
+{
+	struct pmem_device *pmem = dev_to_disk(dev)->private_data;
+	bool write_cache;
+	int rc;
+
+	rc = strtobool(buf, &write_cache);
+	if (rc)
+		return rc;
+	dax_write_cache(pmem->dax_dev, write_cache);
+	return len;
+}
+static DEVICE_ATTR_RW(write_cache);
+
+static umode_t dax_visible(struct kobject *kobj, struct attribute *a, int n)
+{
+#ifndef CONFIG_ARCH_HAS_PMEM_API
+	if (a == &dev_attr_write_cache.attr)
+		return 0;
+#endif
+	return a->mode;
+}
+
+static struct attribute *dax_attributes[] = {
+	&dev_attr_write_cache.attr,
+	NULL,
+};
+
+static const struct attribute_group dax_attribute_group = {
+	.name		= "dax",
+	.attrs		= dax_attributes,
+	.is_visible	= dax_visible,
+};
+
 static const struct attribute_group *pmem_attribute_groups[] = {
 	&dax_attribute_group,
 	NULL,
diff --git a/include/linux/dax.h b/include/linux/dax.h
index 2619d94c308d4..8623caa673889 100644
--- a/include/linux/dax.h
+++ b/include/linux/dax.h
@@ -38,8 +38,6 @@ struct dax_operations {
 	int (*zero_page_range)(struct dax_device *, pgoff_t, size_t);
 };
 
-extern struct attribute_group dax_attribute_group;
-
 #if IS_ENABLED(CONFIG_DAX)
 struct dax_device *alloc_dax(void *private, const char *host,
 		const struct dax_operations *ops, unsigned long flags);
-- 
2.30.2


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 3/3] block: warn if ->groups is set when calling add_disk
  2021-09-22 17:34 dax_supported() related cleanups v2 Christoph Hellwig
  2021-09-22 17:34 ` [PATCH 1/3] nvdimm/pmem: fix creating the dax group Christoph Hellwig
  2021-09-22 17:34 ` [PATCH 2/3] nvdimm/pmem: move dax_attribute_group from dax to pmem Christoph Hellwig
@ 2021-09-22 17:34 ` Christoph Hellwig
  2021-09-22 17:55 ` dax_supported() related cleanups v2 Dan Williams
  3 siblings, 0 replies; 14+ messages in thread
From: Christoph Hellwig @ 2021-09-22 17:34 UTC (permalink / raw)
  To: Dan Williams, Vishal Verma, Dave Jiang
  Cc: Mike Snitzer, Matthew Wilcox, linux-xfs, nvdimm, linux-fsdevel,
	linux-ext4, Ira Weiny

The proper API is to pass the groups to device_add_disk, but the code
used to also allow groups being set before calling *add_disk.  Warn
about that but keep the group pointer intact for now so that it can
be removed again after a grace period.

Fixes: 52b85909f85d ("block: fold register_disk into device_add_disk")
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Ira Weiny <ira.weiny@intel.com>
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
---
 block/genhd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/genhd.c b/block/genhd.c
index 7b6e5e1cf9564..409cf608cc5bd 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -439,7 +439,8 @@ int device_add_disk(struct device *parent, struct gendisk *disk,
 	dev_set_uevent_suppress(ddev, 1);
 
 	ddev->parent = parent;
-	ddev->groups = groups;
+	if (!WARN_ON_ONCE(ddev->groups))
+		ddev->groups = groups;
 	dev_set_name(ddev, "%s", disk->disk_name);
 	if (!(disk->flags & GENHD_FL_HIDDEN))
 		ddev->devt = MKDEV(disk->major, disk->first_minor);
-- 
2.30.2


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: dax_supported() related cleanups v2
  2021-09-22 17:34 dax_supported() related cleanups v2 Christoph Hellwig
                   ` (2 preceding siblings ...)
  2021-09-22 17:34 ` [PATCH 3/3] block: warn if ->groups is set when calling add_disk Christoph Hellwig
@ 2021-09-22 17:55 ` Dan Williams
  2021-09-22 18:33   ` Christoph Hellwig
  3 siblings, 1 reply; 14+ messages in thread
From: Dan Williams @ 2021-09-22 17:55 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Vishal Verma, Dave Jiang, Mike Snitzer, Matthew Wilcox,
	linux-xfs, Linux NVDIMM, linux-fsdevel, linux-ext4

On Wed, Sep 22, 2021 at 10:37 AM Christoph Hellwig <hch@lst.de> wrote:
>
> Hi all,
>
> this series first clarifies how to use fsdax in the Kconfig help a bit,
> and then untangles the code path that checks if fsdax is supported.
>
> Changes since v1:
>  - improve the FS_DAX Kconfig help text further
>  - write a proper commit log for a patch missing it
>

This looks like your send script picked up the wrong cover letter?

> Diffstat
>  drivers/dax/super.c   |  191 +++++++++++++++++++-------------------------------
>  drivers/md/dm-table.c |    9 --
>  drivers/md/dm.c       |    2
>  fs/Kconfig            |   21 ++++-
>  fs/ext2/super.c       |    3
>  fs/ext4/super.c       |    3
>  fs/xfs/xfs_super.c    |   16 +++-
>  include/linux/dax.h   |   41 +---------
>  8 files changed, 117 insertions(+), 169 deletions(-)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: dax_supported() related cleanups v2
  2021-09-22 17:55 ` dax_supported() related cleanups v2 Dan Williams
@ 2021-09-22 18:33   ` Christoph Hellwig
  0 siblings, 0 replies; 14+ messages in thread
From: Christoph Hellwig @ 2021-09-22 18:33 UTC (permalink / raw)
  To: Dan Williams
  Cc: Christoph Hellwig, Vishal Verma, Dave Jiang, Mike Snitzer,
	Matthew Wilcox, linux-xfs, Linux NVDIMM, linux-fsdevel,
	linux-ext4

On Wed, Sep 22, 2021 at 10:55:01AM -0700, Dan Williams wrote:
> This looks like your send script picked up the wrong cover letter?

Yes.  Or the human running the script to be exact :)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/3] nvdimm/pmem: move dax_attribute_group from dax to pmem
  2021-09-22 18:33 ` [PATCH 2/3] nvdimm/pmem: move dax_attribute_group from dax to pmem Christoph Hellwig
@ 2021-09-22 21:40   ` Ira Weiny
  0 siblings, 0 replies; 14+ messages in thread
From: Ira Weiny @ 2021-09-22 21:40 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Dan Williams, Jens Axboe, Vishal Verma, Dave Jiang, linux-block, nvdimm

On Wed, Sep 22, 2021 at 08:33:30PM +0200, Christoph Hellwig wrote:
> dax_attribute_group is only used by the pmem driver, and can avoid the
> completely pointless lookup by the disk name if moved there.  This
> leaves just a single caller of dax_get_by_host, so move dax_get_by_host
> into the same ifdef block as that caller.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

> ---
>  drivers/dax/super.c   | 100 ++++++++----------------------------------
>  drivers/nvdimm/pmem.c |  43 ++++++++++++++++++
>  include/linux/dax.h   |   2 -
>  3 files changed, 61 insertions(+), 84 deletions(-)
> 
> diff --git a/drivers/dax/super.c b/drivers/dax/super.c
> index fc89e91beea7c..b882cf8106ea3 100644
> --- a/drivers/dax/super.c
> +++ b/drivers/dax/super.c
> @@ -63,6 +63,24 @@ static int dax_host_hash(const char *host)
>  	return hashlen_hash(hashlen_string("DAX", host)) % DAX_HASH_SIZE;
>  }
>  
> +#ifdef CONFIG_BLOCK
> +#include <linux/blkdev.h>
> +
> +int bdev_dax_pgoff(struct block_device *bdev, sector_t sector, size_t size,
> +		pgoff_t *pgoff)
> +{
> +	sector_t start_sect = bdev ? get_start_sect(bdev) : 0;
> +	phys_addr_t phys_off = (start_sect + sector) * 512;
> +
> +	if (pgoff)
> +		*pgoff = PHYS_PFN(phys_off);
> +	if (phys_off % PAGE_SIZE || size % PAGE_SIZE)
> +		return -EINVAL;
> +	return 0;
> +}
> +EXPORT_SYMBOL(bdev_dax_pgoff);
> +
> +#if IS_ENABLED(CONFIG_FS_DAX)
>  /**
>   * dax_get_by_host() - temporary lookup mechanism for filesystem-dax
>   * @host: alternate name for the device registered by a dax driver
> @@ -94,24 +112,6 @@ static struct dax_device *dax_get_by_host(const char *host)
>  	return found;
>  }
>  
> -#ifdef CONFIG_BLOCK
> -#include <linux/blkdev.h>
> -
> -int bdev_dax_pgoff(struct block_device *bdev, sector_t sector, size_t size,
> -		pgoff_t *pgoff)
> -{
> -	sector_t start_sect = bdev ? get_start_sect(bdev) : 0;
> -	phys_addr_t phys_off = (start_sect + sector) * 512;
> -
> -	if (pgoff)
> -		*pgoff = PHYS_PFN(phys_off);
> -	if (phys_off % PAGE_SIZE || size % PAGE_SIZE)
> -		return -EINVAL;
> -	return 0;
> -}
> -EXPORT_SYMBOL(bdev_dax_pgoff);
> -
> -#if IS_ENABLED(CONFIG_FS_DAX)
>  struct dax_device *fs_dax_get_by_bdev(struct block_device *bdev)
>  {
>  	if (!blk_queue_dax(bdev->bd_disk->queue))
> @@ -231,70 +231,6 @@ enum dax_device_flags {
>  	DAXDEV_SYNC,
>  };
>  
> -static ssize_t write_cache_show(struct device *dev,
> -		struct device_attribute *attr, char *buf)
> -{
> -	struct dax_device *dax_dev = dax_get_by_host(dev_name(dev));
> -	ssize_t rc;
> -
> -	WARN_ON_ONCE(!dax_dev);
> -	if (!dax_dev)
> -		return -ENXIO;
> -
> -	rc = sprintf(buf, "%d\n", !!dax_write_cache_enabled(dax_dev));
> -	put_dax(dax_dev);
> -	return rc;
> -}
> -
> -static ssize_t write_cache_store(struct device *dev,
> -		struct device_attribute *attr, const char *buf, size_t len)
> -{
> -	bool write_cache;
> -	int rc = strtobool(buf, &write_cache);
> -	struct dax_device *dax_dev = dax_get_by_host(dev_name(dev));
> -
> -	WARN_ON_ONCE(!dax_dev);
> -	if (!dax_dev)
> -		return -ENXIO;
> -
> -	if (rc)
> -		len = rc;
> -	else
> -		dax_write_cache(dax_dev, write_cache);
> -
> -	put_dax(dax_dev);
> -	return len;
> -}
> -static DEVICE_ATTR_RW(write_cache);
> -
> -static umode_t dax_visible(struct kobject *kobj, struct attribute *a, int n)
> -{
> -	struct device *dev = container_of(kobj, typeof(*dev), kobj);
> -	struct dax_device *dax_dev = dax_get_by_host(dev_name(dev));
> -
> -	WARN_ON_ONCE(!dax_dev);
> -	if (!dax_dev)
> -		return 0;
> -
> -#ifndef CONFIG_ARCH_HAS_PMEM_API
> -	if (a == &dev_attr_write_cache.attr)
> -		return 0;
> -#endif
> -	return a->mode;
> -}
> -
> -static struct attribute *dax_attributes[] = {
> -	&dev_attr_write_cache.attr,
> -	NULL,
> -};
> -
> -struct attribute_group dax_attribute_group = {
> -	.name = "dax",
> -	.attrs = dax_attributes,
> -	.is_visible = dax_visible,
> -};
> -EXPORT_SYMBOL_GPL(dax_attribute_group);
> -
>  /**
>   * dax_direct_access() - translate a device pgoff to an absolute pfn
>   * @dax_dev: a dax_device instance representing the logical memory range
> diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
> index ef4950f808326..bbeb3f46db157 100644
> --- a/drivers/nvdimm/pmem.c
> +++ b/drivers/nvdimm/pmem.c
> @@ -328,6 +328,49 @@ static const struct dax_operations pmem_dax_ops = {
>  	.zero_page_range = pmem_dax_zero_page_range,
>  };
>  
> +static ssize_t write_cache_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct pmem_device *pmem = dev_to_disk(dev)->private_data;
> +
> +	return sprintf(buf, "%d\n", !!dax_write_cache_enabled(pmem->dax_dev));
> +}
> +
> +static ssize_t write_cache_store(struct device *dev,
> +		struct device_attribute *attr, const char *buf, size_t len)
> +{
> +	struct pmem_device *pmem = dev_to_disk(dev)->private_data;
> +	bool write_cache;
> +	int rc;
> +
> +	rc = strtobool(buf, &write_cache);
> +	if (rc)
> +		return rc;
> +	dax_write_cache(pmem->dax_dev, write_cache);
> +	return len;
> +}
> +static DEVICE_ATTR_RW(write_cache);
> +
> +static umode_t dax_visible(struct kobject *kobj, struct attribute *a, int n)
> +{
> +#ifndef CONFIG_ARCH_HAS_PMEM_API
> +	if (a == &dev_attr_write_cache.attr)
> +		return 0;
> +#endif
> +	return a->mode;
> +}
> +
> +static struct attribute *dax_attributes[] = {
> +	&dev_attr_write_cache.attr,
> +	NULL,
> +};
> +
> +static const struct attribute_group dax_attribute_group = {
> +	.name		= "dax",
> +	.attrs		= dax_attributes,
> +	.is_visible	= dax_visible,
> +};
> +
>  static const struct attribute_group *pmem_attribute_groups[] = {
>  	&dax_attribute_group,
>  	NULL,
> diff --git a/include/linux/dax.h b/include/linux/dax.h
> index 2619d94c308d4..8623caa673889 100644
> --- a/include/linux/dax.h
> +++ b/include/linux/dax.h
> @@ -38,8 +38,6 @@ struct dax_operations {
>  	int (*zero_page_range)(struct dax_device *, pgoff_t, size_t);
>  };
>  
> -extern struct attribute_group dax_attribute_group;
> -
>  #if IS_ENABLED(CONFIG_DAX)
>  struct dax_device *alloc_dax(void *private, const char *host,
>  		const struct dax_operations *ops, unsigned long flags);
> -- 
> 2.30.2
> 
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 2/3] nvdimm/pmem: move dax_attribute_group from dax to pmem
  2021-09-22 18:33 fix a dax/block device attribute registration regression Christoph Hellwig
@ 2021-09-22 18:33 ` Christoph Hellwig
  2021-09-22 21:40   ` Ira Weiny
  0 siblings, 1 reply; 14+ messages in thread
From: Christoph Hellwig @ 2021-09-22 18:33 UTC (permalink / raw)
  To: Dan Williams, Jens Axboe
  Cc: Vishal Verma, Dave Jiang, Ira Weiny, linux-block, nvdimm

dax_attribute_group is only used by the pmem driver, and can avoid the
completely pointless lookup by the disk name if moved there.  This
leaves just a single caller of dax_get_by_host, so move dax_get_by_host
into the same ifdef block as that caller.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/dax/super.c   | 100 ++++++++----------------------------------
 drivers/nvdimm/pmem.c |  43 ++++++++++++++++++
 include/linux/dax.h   |   2 -
 3 files changed, 61 insertions(+), 84 deletions(-)

diff --git a/drivers/dax/super.c b/drivers/dax/super.c
index fc89e91beea7c..b882cf8106ea3 100644
--- a/drivers/dax/super.c
+++ b/drivers/dax/super.c
@@ -63,6 +63,24 @@ static int dax_host_hash(const char *host)
 	return hashlen_hash(hashlen_string("DAX", host)) % DAX_HASH_SIZE;
 }
 
+#ifdef CONFIG_BLOCK
+#include <linux/blkdev.h>
+
+int bdev_dax_pgoff(struct block_device *bdev, sector_t sector, size_t size,
+		pgoff_t *pgoff)
+{
+	sector_t start_sect = bdev ? get_start_sect(bdev) : 0;
+	phys_addr_t phys_off = (start_sect + sector) * 512;
+
+	if (pgoff)
+		*pgoff = PHYS_PFN(phys_off);
+	if (phys_off % PAGE_SIZE || size % PAGE_SIZE)
+		return -EINVAL;
+	return 0;
+}
+EXPORT_SYMBOL(bdev_dax_pgoff);
+
+#if IS_ENABLED(CONFIG_FS_DAX)
 /**
  * dax_get_by_host() - temporary lookup mechanism for filesystem-dax
  * @host: alternate name for the device registered by a dax driver
@@ -94,24 +112,6 @@ static struct dax_device *dax_get_by_host(const char *host)
 	return found;
 }
 
-#ifdef CONFIG_BLOCK
-#include <linux/blkdev.h>
-
-int bdev_dax_pgoff(struct block_device *bdev, sector_t sector, size_t size,
-		pgoff_t *pgoff)
-{
-	sector_t start_sect = bdev ? get_start_sect(bdev) : 0;
-	phys_addr_t phys_off = (start_sect + sector) * 512;
-
-	if (pgoff)
-		*pgoff = PHYS_PFN(phys_off);
-	if (phys_off % PAGE_SIZE || size % PAGE_SIZE)
-		return -EINVAL;
-	return 0;
-}
-EXPORT_SYMBOL(bdev_dax_pgoff);
-
-#if IS_ENABLED(CONFIG_FS_DAX)
 struct dax_device *fs_dax_get_by_bdev(struct block_device *bdev)
 {
 	if (!blk_queue_dax(bdev->bd_disk->queue))
@@ -231,70 +231,6 @@ enum dax_device_flags {
 	DAXDEV_SYNC,
 };
 
-static ssize_t write_cache_show(struct device *dev,
-		struct device_attribute *attr, char *buf)
-{
-	struct dax_device *dax_dev = dax_get_by_host(dev_name(dev));
-	ssize_t rc;
-
-	WARN_ON_ONCE(!dax_dev);
-	if (!dax_dev)
-		return -ENXIO;
-
-	rc = sprintf(buf, "%d\n", !!dax_write_cache_enabled(dax_dev));
-	put_dax(dax_dev);
-	return rc;
-}
-
-static ssize_t write_cache_store(struct device *dev,
-		struct device_attribute *attr, const char *buf, size_t len)
-{
-	bool write_cache;
-	int rc = strtobool(buf, &write_cache);
-	struct dax_device *dax_dev = dax_get_by_host(dev_name(dev));
-
-	WARN_ON_ONCE(!dax_dev);
-	if (!dax_dev)
-		return -ENXIO;
-
-	if (rc)
-		len = rc;
-	else
-		dax_write_cache(dax_dev, write_cache);
-
-	put_dax(dax_dev);
-	return len;
-}
-static DEVICE_ATTR_RW(write_cache);
-
-static umode_t dax_visible(struct kobject *kobj, struct attribute *a, int n)
-{
-	struct device *dev = container_of(kobj, typeof(*dev), kobj);
-	struct dax_device *dax_dev = dax_get_by_host(dev_name(dev));
-
-	WARN_ON_ONCE(!dax_dev);
-	if (!dax_dev)
-		return 0;
-
-#ifndef CONFIG_ARCH_HAS_PMEM_API
-	if (a == &dev_attr_write_cache.attr)
-		return 0;
-#endif
-	return a->mode;
-}
-
-static struct attribute *dax_attributes[] = {
-	&dev_attr_write_cache.attr,
-	NULL,
-};
-
-struct attribute_group dax_attribute_group = {
-	.name = "dax",
-	.attrs = dax_attributes,
-	.is_visible = dax_visible,
-};
-EXPORT_SYMBOL_GPL(dax_attribute_group);
-
 /**
  * dax_direct_access() - translate a device pgoff to an absolute pfn
  * @dax_dev: a dax_device instance representing the logical memory range
diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index ef4950f808326..bbeb3f46db157 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -328,6 +328,49 @@ static const struct dax_operations pmem_dax_ops = {
 	.zero_page_range = pmem_dax_zero_page_range,
 };
 
+static ssize_t write_cache_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct pmem_device *pmem = dev_to_disk(dev)->private_data;
+
+	return sprintf(buf, "%d\n", !!dax_write_cache_enabled(pmem->dax_dev));
+}
+
+static ssize_t write_cache_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t len)
+{
+	struct pmem_device *pmem = dev_to_disk(dev)->private_data;
+	bool write_cache;
+	int rc;
+
+	rc = strtobool(buf, &write_cache);
+	if (rc)
+		return rc;
+	dax_write_cache(pmem->dax_dev, write_cache);
+	return len;
+}
+static DEVICE_ATTR_RW(write_cache);
+
+static umode_t dax_visible(struct kobject *kobj, struct attribute *a, int n)
+{
+#ifndef CONFIG_ARCH_HAS_PMEM_API
+	if (a == &dev_attr_write_cache.attr)
+		return 0;
+#endif
+	return a->mode;
+}
+
+static struct attribute *dax_attributes[] = {
+	&dev_attr_write_cache.attr,
+	NULL,
+};
+
+static const struct attribute_group dax_attribute_group = {
+	.name		= "dax",
+	.attrs		= dax_attributes,
+	.is_visible	= dax_visible,
+};
+
 static const struct attribute_group *pmem_attribute_groups[] = {
 	&dax_attribute_group,
 	NULL,
diff --git a/include/linux/dax.h b/include/linux/dax.h
index 2619d94c308d4..8623caa673889 100644
--- a/include/linux/dax.h
+++ b/include/linux/dax.h
@@ -38,8 +38,6 @@ struct dax_operations {
 	int (*zero_page_range)(struct dax_device *, pgoff_t, size_t);
 };
 
-extern struct attribute_group dax_attribute_group;
-
 #if IS_ENABLED(CONFIG_DAX)
 struct dax_device *alloc_dax(void *private, const char *host,
 		const struct dax_operations *ops, unsigned long flags);
-- 
2.30.2


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/3] nvdimm/pmem: move dax_attribute_group from dax to pmem
  2021-09-20  7:27 ` [PATCH 2/3] nvdimm/pmem: move dax_attribute_group from dax to pmem Christoph Hellwig
  2021-09-20 18:00   ` kernel test robot
  2021-09-20 22:51   ` Ira Weiny
@ 2021-09-20 23:37   ` Dan Williams
  2 siblings, 0 replies; 14+ messages in thread
From: Dan Williams @ 2021-09-20 23:37 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jens Axboe, Vishal Verma, Dave Jiang, Ira Weiny, linux-block,
	Linux NVDIMM

On Mon, Sep 20, 2021 at 12:29 AM Christoph Hellwig <hch@lst.de> wrote:
>
> dax_attribute_group is only used by the pmem driver, and can avoid the
> completely pointless lookup by the disk name if moved there.

After the additional fixups that 0day-robot found, you can add:

Reviewed-by: Dan Williams <dan.j.williams@intel.com>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/3] nvdimm/pmem: move dax_attribute_group from dax to pmem
  2021-09-20 22:51   ` Ira Weiny
@ 2021-09-20 23:36     ` Dan Williams
  0 siblings, 0 replies; 14+ messages in thread
From: Dan Williams @ 2021-09-20 23:36 UTC (permalink / raw)
  To: Ira Weiny
  Cc: Christoph Hellwig, Jens Axboe, Vishal Verma, Dave Jiang,
	linux-block, Linux NVDIMM

On Mon, Sep 20, 2021 at 3:51 PM Ira Weiny <ira.weiny@intel.com> wrote:
>
> On Mon, Sep 20, 2021 at 09:27:25AM +0200, Christoph Hellwig wrote:
>
> ...
>
> > diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
> > index ef4950f808326..bbeb3f46db157 100644
> > --- a/drivers/nvdimm/pmem.c
> > +++ b/drivers/nvdimm/pmem.c
> > @@ -328,6 +328,49 @@ static const struct dax_operations pmem_dax_ops = {
> >       .zero_page_range = pmem_dax_zero_page_range,
> >  };
> >
> > +static ssize_t write_cache_show(struct device *dev,
> > +             struct device_attribute *attr, char *buf)
> > +{
> > +     struct pmem_device *pmem = dev_to_disk(dev)->private_data;
>
> I want to say this should be dax_get_private()...  However, looking at the use

No, this wants to do from @dev to @dax_dev. dax_get_private() assumes
that @dax_dev is already known. Also, in this case @dev is the gendisk
device, so this is a gendisk-to-dax-device with special knowledge that
the gendisk is for a pmem device.

> of dax_get_private() not a single caller checks for NULL!  :-(

All the callers are correctly assuming that their usage is before kill_dax().

>
> So now I wonder why dax_get_private() exists...  :-/

It exists so that the definition of 'struct dax_device' can remain
private, as no one should be directly mucking with dax_device
internals outside of the provided APIs.

> A quick history search does not make anything apparent.  When the DAXDEV_ALIVE
> check was added to dax_get_private() no callers were changed to account for a
> potential NULL return.
>
> Dan?

I double checked, but this all looks ok to me.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/3] nvdimm/pmem: move dax_attribute_group from dax to pmem
  2021-09-20 18:00   ` kernel test robot
@ 2021-09-20 23:29     ` Dan Williams
  0 siblings, 0 replies; 14+ messages in thread
From: Dan Williams @ 2021-09-20 23:29 UTC (permalink / raw)
  To: kernel test robot
  Cc: Christoph Hellwig, Jens Axboe, kbuild-all, Vishal Verma,
	Dave Jiang, Ira Weiny, linux-block, Linux NVDIMM

On Mon, Sep 20, 2021 at 11:01 AM kernel test robot <lkp@intel.com> wrote:
>
> Hi Christoph,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on axboe-block/for-next]
> [also build test ERROR on linus/master v5.15-rc2 next-20210920]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url:    https://github.com/0day-ci/linux/commits/Christoph-Hellwig/nvdimm-pmem-fix-creating-the-dax-group/20210920-162804
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
> config: riscv-buildonly-randconfig-r006-20210920 (attached as .config)
> compiler: riscv64-linux-gcc (GCC) 11.2.0
> reproduce (this is a W=1 build):
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # https://github.com/0day-ci/linux/commit/1a16d0f32f70bcd159a9f8cf32794f4024e8711e
>         git remote add linux-review https://github.com/0day-ci/linux
>         git fetch --no-tags linux-review Christoph-Hellwig/nvdimm-pmem-fix-creating-the-dax-group/20210920-162804
>         git checkout 1a16d0f32f70bcd159a9f8cf32794f4024e8711e
>         # save the attached .config to linux build tree
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=riscv
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All errors (new ones prefixed by >>):
>
>    drivers/dax/super.c:375:6: error: no previous prototype for 'run_dax' [-Werror=missing-prototypes]
>      375 | void run_dax(struct dax_device *dax_dev)
>          |      ^~~~~~~

In this new -Werror world drivers/dax/bus.c needs:

#include "bus.h"

> >> drivers/dax/super.c:70:27: error: 'dax_get_by_host' defined but not used [-Werror=unused-function]
>       70 | static struct dax_device *dax_get_by_host(const char *host)
>          |                           ^~~~~~~~~~~~~~~
>    cc1: all warnings being treated as errors

Nice additional cleanup that now fs_dax_get_by_bdev() is the only
caller of dax_get_by_host().

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/3] nvdimm/pmem: move dax_attribute_group from dax to pmem
  2021-09-20  7:27 ` [PATCH 2/3] nvdimm/pmem: move dax_attribute_group from dax to pmem Christoph Hellwig
  2021-09-20 18:00   ` kernel test robot
@ 2021-09-20 22:51   ` Ira Weiny
  2021-09-20 23:36     ` Dan Williams
  2021-09-20 23:37   ` Dan Williams
  2 siblings, 1 reply; 14+ messages in thread
From: Ira Weiny @ 2021-09-20 22:51 UTC (permalink / raw)
  To: Christoph Hellwig, Dan Williams
  Cc: Dan Williams, Jens Axboe, Vishal Verma, Dave Jiang, linux-block, nvdimm

On Mon, Sep 20, 2021 at 09:27:25AM +0200, Christoph Hellwig wrote:

...

> diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
> index ef4950f808326..bbeb3f46db157 100644
> --- a/drivers/nvdimm/pmem.c
> +++ b/drivers/nvdimm/pmem.c
> @@ -328,6 +328,49 @@ static const struct dax_operations pmem_dax_ops = {
>  	.zero_page_range = pmem_dax_zero_page_range,
>  };
>  
> +static ssize_t write_cache_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct pmem_device *pmem = dev_to_disk(dev)->private_data;

I want to say this should be dax_get_private()...  However, looking at the use
of dax_get_private() not a single caller checks for NULL!  :-(

So now I wonder why dax_get_private() exists...  :-/

A quick history search does not make anything apparent.  When the DAXDEV_ALIVE
check was added to dax_get_private() no callers were changed to account for a
potential NULL return.

Dan?

> +
> +	return sprintf(buf, "%d\n", !!dax_write_cache_enabled(pmem->dax_dev));
> +}
> +
> +static ssize_t write_cache_store(struct device *dev,
> +		struct device_attribute *attr, const char *buf, size_t len)
> +{
> +	struct pmem_device *pmem = dev_to_disk(dev)->private_data;

Same here...

Ira


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/3] nvdimm/pmem: move dax_attribute_group from dax to pmem
  2021-09-20  7:27 ` [PATCH 2/3] nvdimm/pmem: move dax_attribute_group from dax to pmem Christoph Hellwig
@ 2021-09-20 18:00   ` kernel test robot
  2021-09-20 23:29     ` Dan Williams
  2021-09-20 22:51   ` Ira Weiny
  2021-09-20 23:37   ` Dan Williams
  2 siblings, 1 reply; 14+ messages in thread
From: kernel test robot @ 2021-09-20 18:00 UTC (permalink / raw)
  To: Christoph Hellwig, Dan Williams, Jens Axboe
  Cc: kbuild-all, Vishal Verma, Dave Jiang, Ira Weiny, linux-block, nvdimm

[-- Attachment #1: Type: text/plain, Size: 4375 bytes --]

Hi Christoph,

I love your patch! Yet something to improve:

[auto build test ERROR on axboe-block/for-next]
[also build test ERROR on linus/master v5.15-rc2 next-20210920]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Christoph-Hellwig/nvdimm-pmem-fix-creating-the-dax-group/20210920-162804
base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
config: riscv-buildonly-randconfig-r006-20210920 (attached as .config)
compiler: riscv64-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/1a16d0f32f70bcd159a9f8cf32794f4024e8711e
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Christoph-Hellwig/nvdimm-pmem-fix-creating-the-dax-group/20210920-162804
        git checkout 1a16d0f32f70bcd159a9f8cf32794f4024e8711e
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=riscv 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/dax/super.c:375:6: error: no previous prototype for 'run_dax' [-Werror=missing-prototypes]
     375 | void run_dax(struct dax_device *dax_dev)
         |      ^~~~~~~
>> drivers/dax/super.c:70:27: error: 'dax_get_by_host' defined but not used [-Werror=unused-function]
      70 | static struct dax_device *dax_get_by_host(const char *host)
         |                           ^~~~~~~~~~~~~~~
   cc1: all warnings being treated as errors


vim +/dax_get_by_host +70 drivers/dax/super.c

1b7646014e0d838b Christoph Hellwig 2021-08-26  65  
1b7646014e0d838b Christoph Hellwig 2021-08-26  66  /**
1b7646014e0d838b Christoph Hellwig 2021-08-26  67   * dax_get_by_host() - temporary lookup mechanism for filesystem-dax
1b7646014e0d838b Christoph Hellwig 2021-08-26  68   * @host: alternate name for the device registered by a dax driver
1b7646014e0d838b Christoph Hellwig 2021-08-26  69   */
1b7646014e0d838b Christoph Hellwig 2021-08-26 @70  static struct dax_device *dax_get_by_host(const char *host)
1b7646014e0d838b Christoph Hellwig 2021-08-26  71  {
1b7646014e0d838b Christoph Hellwig 2021-08-26  72  	struct dax_device *dax_dev, *found = NULL;
1b7646014e0d838b Christoph Hellwig 2021-08-26  73  	int hash, id;
1b7646014e0d838b Christoph Hellwig 2021-08-26  74  
1b7646014e0d838b Christoph Hellwig 2021-08-26  75  	if (!host)
1b7646014e0d838b Christoph Hellwig 2021-08-26  76  		return NULL;
1b7646014e0d838b Christoph Hellwig 2021-08-26  77  
1b7646014e0d838b Christoph Hellwig 2021-08-26  78  	hash = dax_host_hash(host);
1b7646014e0d838b Christoph Hellwig 2021-08-26  79  
1b7646014e0d838b Christoph Hellwig 2021-08-26  80  	id = dax_read_lock();
1b7646014e0d838b Christoph Hellwig 2021-08-26  81  	spin_lock(&dax_host_lock);
1b7646014e0d838b Christoph Hellwig 2021-08-26  82  	hlist_for_each_entry(dax_dev, &dax_host_list[hash], list) {
1b7646014e0d838b Christoph Hellwig 2021-08-26  83  		if (!dax_alive(dax_dev)
1b7646014e0d838b Christoph Hellwig 2021-08-26  84  				|| strcmp(host, dax_dev->host) != 0)
1b7646014e0d838b Christoph Hellwig 2021-08-26  85  			continue;
1b7646014e0d838b Christoph Hellwig 2021-08-26  86  
1b7646014e0d838b Christoph Hellwig 2021-08-26  87  		if (igrab(&dax_dev->inode))
1b7646014e0d838b Christoph Hellwig 2021-08-26  88  			found = dax_dev;
1b7646014e0d838b Christoph Hellwig 2021-08-26  89  		break;
1b7646014e0d838b Christoph Hellwig 2021-08-26  90  	}
1b7646014e0d838b Christoph Hellwig 2021-08-26  91  	spin_unlock(&dax_host_lock);
1b7646014e0d838b Christoph Hellwig 2021-08-26  92  	dax_read_unlock(id);
1b7646014e0d838b Christoph Hellwig 2021-08-26  93  
1b7646014e0d838b Christoph Hellwig 2021-08-26  94  	return found;
1b7646014e0d838b Christoph Hellwig 2021-08-26  95  }
1b7646014e0d838b Christoph Hellwig 2021-08-26  96  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28763 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 2/3] nvdimm/pmem: move dax_attribute_group from dax to pmem
  2021-09-20  7:27 fix a dax/block device attribute registration regression Christoph Hellwig
@ 2021-09-20  7:27 ` Christoph Hellwig
  2021-09-20 18:00   ` kernel test robot
                     ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Christoph Hellwig @ 2021-09-20  7:27 UTC (permalink / raw)
  To: Dan Williams, Jens Axboe
  Cc: Vishal Verma, Dave Jiang, Ira Weiny, linux-block, nvdimm

dax_attribute_group is only used by the pmem driver, and can avoid the
completely pointless lookup by the disk name if moved there.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/dax/super.c   | 64 -------------------------------------------
 drivers/nvdimm/pmem.c | 43 +++++++++++++++++++++++++++++
 include/linux/dax.h   |  2 --
 3 files changed, 43 insertions(+), 66 deletions(-)

diff --git a/drivers/dax/super.c b/drivers/dax/super.c
index fc89e91beea7c..e03d94bdc0449 100644
--- a/drivers/dax/super.c
+++ b/drivers/dax/super.c
@@ -231,70 +231,6 @@ enum dax_device_flags {
 	DAXDEV_SYNC,
 };
 
-static ssize_t write_cache_show(struct device *dev,
-		struct device_attribute *attr, char *buf)
-{
-	struct dax_device *dax_dev = dax_get_by_host(dev_name(dev));
-	ssize_t rc;
-
-	WARN_ON_ONCE(!dax_dev);
-	if (!dax_dev)
-		return -ENXIO;
-
-	rc = sprintf(buf, "%d\n", !!dax_write_cache_enabled(dax_dev));
-	put_dax(dax_dev);
-	return rc;
-}
-
-static ssize_t write_cache_store(struct device *dev,
-		struct device_attribute *attr, const char *buf, size_t len)
-{
-	bool write_cache;
-	int rc = strtobool(buf, &write_cache);
-	struct dax_device *dax_dev = dax_get_by_host(dev_name(dev));
-
-	WARN_ON_ONCE(!dax_dev);
-	if (!dax_dev)
-		return -ENXIO;
-
-	if (rc)
-		len = rc;
-	else
-		dax_write_cache(dax_dev, write_cache);
-
-	put_dax(dax_dev);
-	return len;
-}
-static DEVICE_ATTR_RW(write_cache);
-
-static umode_t dax_visible(struct kobject *kobj, struct attribute *a, int n)
-{
-	struct device *dev = container_of(kobj, typeof(*dev), kobj);
-	struct dax_device *dax_dev = dax_get_by_host(dev_name(dev));
-
-	WARN_ON_ONCE(!dax_dev);
-	if (!dax_dev)
-		return 0;
-
-#ifndef CONFIG_ARCH_HAS_PMEM_API
-	if (a == &dev_attr_write_cache.attr)
-		return 0;
-#endif
-	return a->mode;
-}
-
-static struct attribute *dax_attributes[] = {
-	&dev_attr_write_cache.attr,
-	NULL,
-};
-
-struct attribute_group dax_attribute_group = {
-	.name = "dax",
-	.attrs = dax_attributes,
-	.is_visible = dax_visible,
-};
-EXPORT_SYMBOL_GPL(dax_attribute_group);
-
 /**
  * dax_direct_access() - translate a device pgoff to an absolute pfn
  * @dax_dev: a dax_device instance representing the logical memory range
diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index ef4950f808326..bbeb3f46db157 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -328,6 +328,49 @@ static const struct dax_operations pmem_dax_ops = {
 	.zero_page_range = pmem_dax_zero_page_range,
 };
 
+static ssize_t write_cache_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct pmem_device *pmem = dev_to_disk(dev)->private_data;
+
+	return sprintf(buf, "%d\n", !!dax_write_cache_enabled(pmem->dax_dev));
+}
+
+static ssize_t write_cache_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t len)
+{
+	struct pmem_device *pmem = dev_to_disk(dev)->private_data;
+	bool write_cache;
+	int rc;
+
+	rc = strtobool(buf, &write_cache);
+	if (rc)
+		return rc;
+	dax_write_cache(pmem->dax_dev, write_cache);
+	return len;
+}
+static DEVICE_ATTR_RW(write_cache);
+
+static umode_t dax_visible(struct kobject *kobj, struct attribute *a, int n)
+{
+#ifndef CONFIG_ARCH_HAS_PMEM_API
+	if (a == &dev_attr_write_cache.attr)
+		return 0;
+#endif
+	return a->mode;
+}
+
+static struct attribute *dax_attributes[] = {
+	&dev_attr_write_cache.attr,
+	NULL,
+};
+
+static const struct attribute_group dax_attribute_group = {
+	.name		= "dax",
+	.attrs		= dax_attributes,
+	.is_visible	= dax_visible,
+};
+
 static const struct attribute_group *pmem_attribute_groups[] = {
 	&dax_attribute_group,
 	NULL,
diff --git a/include/linux/dax.h b/include/linux/dax.h
index 2619d94c308d4..8623caa673889 100644
--- a/include/linux/dax.h
+++ b/include/linux/dax.h
@@ -38,8 +38,6 @@ struct dax_operations {
 	int (*zero_page_range)(struct dax_device *, pgoff_t, size_t);
 };
 
-extern struct attribute_group dax_attribute_group;
-
 #if IS_ENABLED(CONFIG_DAX)
 struct dax_device *alloc_dax(void *private, const char *host,
 		const struct dax_operations *ops, unsigned long flags);
-- 
2.30.2


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2021-09-22 21:40 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-22 17:34 dax_supported() related cleanups v2 Christoph Hellwig
2021-09-22 17:34 ` [PATCH 1/3] nvdimm/pmem: fix creating the dax group Christoph Hellwig
2021-09-22 17:34 ` [PATCH 2/3] nvdimm/pmem: move dax_attribute_group from dax to pmem Christoph Hellwig
2021-09-22 17:34 ` [PATCH 3/3] block: warn if ->groups is set when calling add_disk Christoph Hellwig
2021-09-22 17:55 ` dax_supported() related cleanups v2 Dan Williams
2021-09-22 18:33   ` Christoph Hellwig
  -- strict thread matches above, loose matches on Subject: below --
2021-09-22 18:33 fix a dax/block device attribute registration regression Christoph Hellwig
2021-09-22 18:33 ` [PATCH 2/3] nvdimm/pmem: move dax_attribute_group from dax to pmem Christoph Hellwig
2021-09-22 21:40   ` Ira Weiny
2021-09-20  7:27 fix a dax/block device attribute registration regression Christoph Hellwig
2021-09-20  7:27 ` [PATCH 2/3] nvdimm/pmem: move dax_attribute_group from dax to pmem Christoph Hellwig
2021-09-20 18:00   ` kernel test robot
2021-09-20 23:29     ` Dan Williams
2021-09-20 22:51   ` Ira Weiny
2021-09-20 23:36     ` Dan Williams
2021-09-20 23:37   ` Dan Williams

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).