Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1624826 > unrolled thread

[resend PATCH v2 08/33] dcssblk: add dax_operations support

Started byDan Williams <dan.j.williams@intel.com>
First post2017-04-17 21:20 +0200
Last post2017-04-19 17:50 +0200
Articles 3 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [resend PATCH v2 08/33] dcssblk: add dax_operations support Dan Williams <dan.j.williams@intel.com> - 2017-04-17 21:20 +0200
    Re: [resend PATCH v2 08/33] dcssblk: add dax_operations support Gerald Schaefer <gerald.schaefer@de.ibm.com> - 2017-04-19 17:40 +0200
      Re: [resend PATCH v2 08/33] dcssblk: add dax_operations support Dan Williams <dan.j.williams@intel.com> - 2017-04-19 17:50 +0200

#1624826 — [resend PATCH v2 08/33] dcssblk: add dax_operations support

FromDan Williams <dan.j.williams@intel.com>
Date2017-04-17 21:20 +0200
Subject[resend PATCH v2 08/33] dcssblk: add dax_operations support
Message-ID<txklQ-4zD-23@gated-at.bofh.it>
Setup a dax_dev to have the same lifetime as the dcssblk block device
and add a ->direct_access() method that is equivalent to
dcssblk_direct_access(). Once fs/dax.c has been converted to use
dax_operations the old dcssblk_direct_access() will be removed.

Cc: Gerald Schaefer <gerald.schaefer@de.ibm.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/s390/block/Kconfig   |    1 +
 drivers/s390/block/dcssblk.c |   54 +++++++++++++++++++++++++++++++++++-------
 2 files changed, 46 insertions(+), 9 deletions(-)

diff --git a/drivers/s390/block/Kconfig b/drivers/s390/block/Kconfig
index 4a3b62326183..0acb8c2f9475 100644
--- a/drivers/s390/block/Kconfig
+++ b/drivers/s390/block/Kconfig
@@ -14,6 +14,7 @@ config BLK_DEV_XPRAM
 
 config DCSSBLK
 	def_tristate m
+	select DAX
 	prompt "DCSSBLK support"
 	depends on S390 && BLOCK
 	help
diff --git a/drivers/s390/block/dcssblk.c b/drivers/s390/block/dcssblk.c
index 415d10a67b7a..682a9eb4934d 100644
--- a/drivers/s390/block/dcssblk.c
+++ b/drivers/s390/block/dcssblk.c
@@ -18,6 +18,7 @@
 #include <linux/interrupt.h>
 #include <linux/platform_device.h>
 #include <linux/pfn_t.h>
+#include <linux/dax.h>
 #include <asm/extmem.h>
 #include <asm/io.h>
 
@@ -30,8 +31,10 @@ static int dcssblk_open(struct block_device *bdev, fmode_t mode);
 static void dcssblk_release(struct gendisk *disk, fmode_t mode);
 static blk_qc_t dcssblk_make_request(struct request_queue *q,
 						struct bio *bio);
-static long dcssblk_direct_access(struct block_device *bdev, sector_t secnum,
+static long dcssblk_blk_direct_access(struct block_device *bdev, sector_t secnum,
 			 void **kaddr, pfn_t *pfn, long size);
+static long dcssblk_dax_direct_access(struct dax_device *dax_dev, pgoff_t pgoff,
+		long nr_pages, void **kaddr, pfn_t *pfn);
 
 static char dcssblk_segments[DCSSBLK_PARM_LEN] = "\0";
 
@@ -40,7 +43,11 @@ static const struct block_device_operations dcssblk_devops = {
 	.owner   	= THIS_MODULE,
 	.open    	= dcssblk_open,
 	.release 	= dcssblk_release,
-	.direct_access 	= dcssblk_direct_access,
+	.direct_access 	= dcssblk_blk_direct_access,
+};
+
+static const struct dax_operations dcssblk_dax_ops = {
+	.direct_access = dcssblk_dax_direct_access,
 };
 
 struct dcssblk_dev_info {
@@ -57,6 +64,7 @@ struct dcssblk_dev_info {
 	struct request_queue *dcssblk_queue;
 	int num_of_segments;
 	struct list_head seg_list;
+	struct dax_device *dax_dev;
 };
 
 struct segment_info {
@@ -389,6 +397,8 @@ dcssblk_shared_store(struct device *dev, struct device_attribute *attr, const ch
 	}
 	list_del(&dev_info->lh);
 
+	kill_dax(dev_info->dax_dev);
+	put_dax(dev_info->dax_dev);
 	del_gendisk(dev_info->gd);
 	blk_cleanup_queue(dev_info->dcssblk_queue);
 	dev_info->gd->queue = NULL;
@@ -525,6 +535,7 @@ 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;
 
@@ -654,6 +665,11 @@ dcssblk_add_store(struct device *dev, struct device_attribute *attr, const char
 	if (rc)
 		goto put_dev;
 
+	dax_dev = alloc_dax(dev_info, dev_info->gd->disk_name,
+			&dcssblk_dax_ops);
+	if (!dax_dev)
+		goto put_dev;
+
 	get_device(&dev_info->dev);
 	device_add_disk(&dev_info->dev, dev_info->gd);
 
@@ -752,6 +768,8 @@ dcssblk_remove_store(struct device *dev, struct device_attribute *attr, const ch
 	}
 
 	list_del(&dev_info->lh);
+	kill_dax(dev_info->dax_dev);
+	put_dax(dev_info->dax_dev);
 	del_gendisk(dev_info->gd);
 	blk_cleanup_queue(dev_info->dcssblk_queue);
 	dev_info->gd->queue = NULL;
@@ -883,21 +901,39 @@ dcssblk_make_request(struct request_queue *q, struct bio *bio)
 }
 
 static long
-dcssblk_direct_access (struct block_device *bdev, sector_t secnum,
+__dcssblk_direct_access(struct dcssblk_dev_info *dev_info, pgoff_t pgoff,
+		long nr_pages, void **kaddr, pfn_t *pfn)
+{
+	resource_size_t offset = pgoff * PAGE_SIZE;
+	unsigned long dev_sz;
+
+	dev_sz = dev_info->end - dev_info->start + 1;
+	*kaddr = (void *) dev_info->start + offset;
+	*pfn = __pfn_to_pfn_t(PFN_DOWN(dev_info->start + offset), PFN_DEV);
+
+	return (dev_sz - offset) / PAGE_SIZE;
+}
+
+static long
+dcssblk_blk_direct_access(struct block_device *bdev, sector_t secnum,
 			void **kaddr, pfn_t *pfn, long size)
 {
 	struct dcssblk_dev_info *dev_info;
-	unsigned long offset, dev_sz;
 
 	dev_info = bdev->bd_disk->private_data;
 	if (!dev_info)
 		return -ENODEV;
-	dev_sz = dev_info->end - dev_info->start + 1;
-	offset = secnum * 512;
-	*kaddr = (void *) dev_info->start + offset;
-	*pfn = __pfn_to_pfn_t(PFN_DOWN(dev_info->start + offset), PFN_DEV);
+	return __dcssblk_direct_access(dev_info, PHYS_PFN(secnum * 512),
+			PHYS_PFN(size), kaddr, pfn) * PAGE_SIZE;
+}
+
+static long
+dcssblk_dax_direct_access(struct dax_device *dax_dev, pgoff_t pgoff,
+		long nr_pages, void **kaddr, pfn_t *pfn)
+{
+	struct dcssblk_dev_info *dev_info = dax_get_private(dax_dev);
 
-	return dev_sz - offset;
+	return __dcssblk_direct_access(dev_info, pgoff, nr_pages, kaddr, pfn);
 }
 
 static void

[toc] | [next] | [standalone]

#1626407

FromGerald Schaefer <gerald.schaefer@de.ibm.com>
Date2017-04-19 17:40 +0200
Message-ID<txZS3-59Y-51@gated-at.bofh.it>
In reply to#1624826
On Mon, 17 Apr 2017 12:09:32 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> Setup a dax_dev to have the same lifetime as the dcssblk block device
> and add a ->direct_access() method that is equivalent to
> dcssblk_direct_access(). Once fs/dax.c has been converted to use
> dax_operations the old dcssblk_direct_access() will be removed.
> 
> Cc: Gerald Schaefer <gerald.schaefer@de.ibm.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  drivers/s390/block/Kconfig   |    1 +
>  drivers/s390/block/dcssblk.c |   54 +++++++++++++++++++++++++++++++++++-------
>  2 files changed, 46 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/s390/block/Kconfig b/drivers/s390/block/Kconfig
> index 4a3b62326183..0acb8c2f9475 100644
> --- a/drivers/s390/block/Kconfig
> +++ b/drivers/s390/block/Kconfig
> @@ -14,6 +14,7 @@ config BLK_DEV_XPRAM
> 
>  config DCSSBLK
>  	def_tristate m
> +	select DAX
>  	prompt "DCSSBLK support"
>  	depends on S390 && BLOCK
>  	help
> diff --git a/drivers/s390/block/dcssblk.c b/drivers/s390/block/dcssblk.c
> index 415d10a67b7a..682a9eb4934d 100644
> --- a/drivers/s390/block/dcssblk.c
> +++ b/drivers/s390/block/dcssblk.c
> @@ -18,6 +18,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/platform_device.h>
>  #include <linux/pfn_t.h>
> +#include <linux/dax.h>
>  #include <asm/extmem.h>
>  #include <asm/io.h>
> 
> @@ -30,8 +31,10 @@ static int dcssblk_open(struct block_device *bdev, fmode_t mode);
>  static void dcssblk_release(struct gendisk *disk, fmode_t mode);
>  static blk_qc_t dcssblk_make_request(struct request_queue *q,
>  						struct bio *bio);
> -static long dcssblk_direct_access(struct block_device *bdev, sector_t secnum,
> +static long dcssblk_blk_direct_access(struct block_device *bdev, sector_t secnum,
>  			 void **kaddr, pfn_t *pfn, long size);
> +static long dcssblk_dax_direct_access(struct dax_device *dax_dev, pgoff_t pgoff,
> +		long nr_pages, void **kaddr, pfn_t *pfn);
> 
>  static char dcssblk_segments[DCSSBLK_PARM_LEN] = "\0";
> 
> @@ -40,7 +43,11 @@ static const struct block_device_operations dcssblk_devops = {
>  	.owner   	= THIS_MODULE,
>  	.open    	= dcssblk_open,
>  	.release 	= dcssblk_release,
> -	.direct_access 	= dcssblk_direct_access,
> +	.direct_access 	= dcssblk_blk_direct_access,
> +};
> +
> +static const struct dax_operations dcssblk_dax_ops = {
> +	.direct_access = dcssblk_dax_direct_access,
>  };
> 
>  struct dcssblk_dev_info {
> @@ -57,6 +64,7 @@ struct dcssblk_dev_info {
>  	struct request_queue *dcssblk_queue;
>  	int num_of_segments;
>  	struct list_head seg_list;
> +	struct dax_device *dax_dev;
>  };
> 
>  struct segment_info {
> @@ -389,6 +397,8 @@ dcssblk_shared_store(struct device *dev, struct device_attribute *attr, const ch
>  	}
>  	list_del(&dev_info->lh);
> 
> +	kill_dax(dev_info->dax_dev);
> +	put_dax(dev_info->dax_dev);
>  	del_gendisk(dev_info->gd);
>  	blk_cleanup_queue(dev_info->dcssblk_queue);
>  	dev_info->gd->queue = NULL;
> @@ -525,6 +535,7 @@ 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;
> 
> @@ -654,6 +665,11 @@ dcssblk_add_store(struct device *dev, struct device_attribute *attr, const char
>  	if (rc)
>  		goto put_dev;
> 
> +	dax_dev = alloc_dax(dev_info, dev_info->gd->disk_name,
> +			&dcssblk_dax_ops);
> +	if (!dax_dev)
> +		goto put_dev;
> +

The returned dax_dev should be stored into dev_info->dax_dev, for later use
by kill/put_dax(). This can also be done directly, so that we don't need the
local dax_dev variable here.

Also, in the error case, a proper rc should be set before going to put_dev,
probably -ENOMEM.

I took a quick look at the patches for the other affected drivers, and it
looks like axonram also has the "missing rc" issue, and brd the "missing
brd->dax_dev init" issue, pmem seems to be fine.

>  	get_device(&dev_info->dev);
>  	device_add_disk(&dev_info->dev, dev_info->gd);
> 
> @@ -752,6 +768,8 @@ dcssblk_remove_store(struct device *dev, struct device_attribute *attr, const ch
>  	}
> 
>  	list_del(&dev_info->lh);
> +	kill_dax(dev_info->dax_dev);
> +	put_dax(dev_info->dax_dev);
>  	del_gendisk(dev_info->gd);
>  	blk_cleanup_queue(dev_info->dcssblk_queue);
>  	dev_info->gd->queue = NULL;
> @@ -883,21 +901,39 @@ dcssblk_make_request(struct request_queue *q, struct bio *bio)
>  }
> 
>  static long
> -dcssblk_direct_access (struct block_device *bdev, sector_t secnum,
> +__dcssblk_direct_access(struct dcssblk_dev_info *dev_info, pgoff_t pgoff,
> +		long nr_pages, void **kaddr, pfn_t *pfn)
> +{
> +	resource_size_t offset = pgoff * PAGE_SIZE;
> +	unsigned long dev_sz;
> +
> +	dev_sz = dev_info->end - dev_info->start + 1;
> +	*kaddr = (void *) dev_info->start + offset;
> +	*pfn = __pfn_to_pfn_t(PFN_DOWN(dev_info->start + offset), PFN_DEV);
> +
> +	return (dev_sz - offset) / PAGE_SIZE;
> +}
> +
> +static long
> +dcssblk_blk_direct_access(struct block_device *bdev, sector_t secnum,
>  			void **kaddr, pfn_t *pfn, long size)
>  {
>  	struct dcssblk_dev_info *dev_info;
> -	unsigned long offset, dev_sz;
> 
>  	dev_info = bdev->bd_disk->private_data;
>  	if (!dev_info)
>  		return -ENODEV;
> -	dev_sz = dev_info->end - dev_info->start + 1;
> -	offset = secnum * 512;
> -	*kaddr = (void *) dev_info->start + offset;
> -	*pfn = __pfn_to_pfn_t(PFN_DOWN(dev_info->start + offset), PFN_DEV);
> +	return __dcssblk_direct_access(dev_info, PHYS_PFN(secnum * 512),
> +			PHYS_PFN(size), kaddr, pfn) * PAGE_SIZE;
> +}
> +
> +static long
> +dcssblk_dax_direct_access(struct dax_device *dax_dev, pgoff_t pgoff,
> +		long nr_pages, void **kaddr, pfn_t *pfn)
> +{
> +	struct dcssblk_dev_info *dev_info = dax_get_private(dax_dev);
> 
> -	return dev_sz - offset;
> +	return __dcssblk_direct_access(dev_info, pgoff, nr_pages, kaddr, pfn);
>  }
> 
>  static void
>

[toc] | [prev] | [next] | [standalone]

#1626415

FromDan Williams <dan.j.williams@intel.com>
Date2017-04-19 17:50 +0200
Message-ID<ty01H-5db-23@gated-at.bofh.it>
In reply to#1626407
On Wed, Apr 19, 2017 at 8:31 AM, Gerald Schaefer
<gerald.schaefer@de.ibm.com> wrote:
> On Mon, 17 Apr 2017 12:09:32 -0700
> Dan Williams <dan.j.williams@intel.com> wrote:
>
>> Setup a dax_dev to have the same lifetime as the dcssblk block device
>> and add a ->direct_access() method that is equivalent to
>> dcssblk_direct_access(). Once fs/dax.c has been converted to use
>> dax_operations the old dcssblk_direct_access() will be removed.
>>
>> Cc: Gerald Schaefer <gerald.schaefer@de.ibm.com>
>> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>> ---
>>  drivers/s390/block/Kconfig   |    1 +
>>  drivers/s390/block/dcssblk.c |   54 +++++++++++++++++++++++++++++++++++-------
>>  2 files changed, 46 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/s390/block/Kconfig b/drivers/s390/block/Kconfig
>> index 4a3b62326183..0acb8c2f9475 100644
>> --- a/drivers/s390/block/Kconfig
>> +++ b/drivers/s390/block/Kconfig
>> @@ -14,6 +14,7 @@ config BLK_DEV_XPRAM
>>
>>  config DCSSBLK
>>       def_tristate m
>> +     select DAX
>>       prompt "DCSSBLK support"
>>       depends on S390 && BLOCK
>>       help
>> diff --git a/drivers/s390/block/dcssblk.c b/drivers/s390/block/dcssblk.c
>> index 415d10a67b7a..682a9eb4934d 100644
>> --- a/drivers/s390/block/dcssblk.c
>> +++ b/drivers/s390/block/dcssblk.c
>> @@ -18,6 +18,7 @@
>>  #include <linux/interrupt.h>
>>  #include <linux/platform_device.h>
>>  #include <linux/pfn_t.h>
>> +#include <linux/dax.h>
>>  #include <asm/extmem.h>
>>  #include <asm/io.h>
>>
>> @@ -30,8 +31,10 @@ static int dcssblk_open(struct block_device *bdev, fmode_t mode);
>>  static void dcssblk_release(struct gendisk *disk, fmode_t mode);
>>  static blk_qc_t dcssblk_make_request(struct request_queue *q,
>>                                               struct bio *bio);
>> -static long dcssblk_direct_access(struct block_device *bdev, sector_t secnum,
>> +static long dcssblk_blk_direct_access(struct block_device *bdev, sector_t secnum,
>>                        void **kaddr, pfn_t *pfn, long size);
>> +static long dcssblk_dax_direct_access(struct dax_device *dax_dev, pgoff_t pgoff,
>> +             long nr_pages, void **kaddr, pfn_t *pfn);
>>
>>  static char dcssblk_segments[DCSSBLK_PARM_LEN] = "\0";
>>
>> @@ -40,7 +43,11 @@ static const struct block_device_operations dcssblk_devops = {
>>       .owner          = THIS_MODULE,
>>       .open           = dcssblk_open,
>>       .release        = dcssblk_release,
>> -     .direct_access  = dcssblk_direct_access,
>> +     .direct_access  = dcssblk_blk_direct_access,
>> +};
>> +
>> +static const struct dax_operations dcssblk_dax_ops = {
>> +     .direct_access = dcssblk_dax_direct_access,
>>  };
>>
>>  struct dcssblk_dev_info {
>> @@ -57,6 +64,7 @@ struct dcssblk_dev_info {
>>       struct request_queue *dcssblk_queue;
>>       int num_of_segments;
>>       struct list_head seg_list;
>> +     struct dax_device *dax_dev;
>>  };
>>
>>  struct segment_info {
>> @@ -389,6 +397,8 @@ dcssblk_shared_store(struct device *dev, struct device_attribute *attr, const ch
>>       }
>>       list_del(&dev_info->lh);
>>
>> +     kill_dax(dev_info->dax_dev);
>> +     put_dax(dev_info->dax_dev);
>>       del_gendisk(dev_info->gd);
>>       blk_cleanup_queue(dev_info->dcssblk_queue);
>>       dev_info->gd->queue = NULL;
>> @@ -525,6 +535,7 @@ 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;
>>
>> @@ -654,6 +665,11 @@ dcssblk_add_store(struct device *dev, struct device_attribute *attr, const char
>>       if (rc)
>>               goto put_dev;
>>
>> +     dax_dev = alloc_dax(dev_info, dev_info->gd->disk_name,
>> +                     &dcssblk_dax_ops);
>> +     if (!dax_dev)
>> +             goto put_dev;
>> +
>
> The returned dax_dev should be stored into dev_info->dax_dev, for later use
> by kill/put_dax(). This can also be done directly, so that we don't need the
> local dax_dev variable here.
>
> Also, in the error case, a proper rc should be set before going to put_dev,
> probably -ENOMEM.
>
> I took a quick look at the patches for the other affected drivers, and it
> looks like axonram also has the "missing rc" issue, and brd the "missing
> brd->dax_dev init" issue, pmem seems to be fine.

Thank you for taking a look. I'll get this fixed up.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web