From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gerald Schaefer Subject: Re: [resend PATCH v2 08/33] dcssblk: add dax_operations support Date: Wed, 19 Apr 2017 17:31:38 +0200 Message-ID: <20170419173138.06621e66@thinkpad> References: <149245612770.10206.15496018295337908594.stgit@dwillia2-desk3.amr.corp.intel.com> <149245617283.10206.846177305206642788.stgit@dwillia2-desk3.amr.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <149245617283.10206.846177305206642788.stgit-p8uTFz9XbKj2zm6wflaqv1nYeNYlB/vhral2JQCrhuEAvxtiuMwx3w@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: linux-nvdimm-bounces-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org Sender: "Linux-nvdimm" To: Dan Williams Cc: linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-block-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, dm-devel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org, linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, hch-jcswGhMUV9g@public.gmane.org List-Id: dm-devel.ids On Mon, 17 Apr 2017 12:09:32 -0700 Dan Williams 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 > Signed-off-by: Dan Williams > --- > 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 > #include > #include > +#include > #include > #include > > @@ -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 > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:60440 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1765626AbdDSPbs (ORCPT ); Wed, 19 Apr 2017 11:31:48 -0400 Received: from pps.filterd (m0098414.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.20/8.16.0.20) with SMTP id v3JFU1ES037271 for ; Wed, 19 Apr 2017 11:31:47 -0400 Received: from e06smtp14.uk.ibm.com (e06smtp14.uk.ibm.com [195.75.94.110]) by mx0b-001b2d01.pphosted.com with ESMTP id 29x75nkdr3-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Wed, 19 Apr 2017 11:31:47 -0400 Received: from localhost by e06smtp14.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 19 Apr 2017 16:31:44 +0100 Date: Wed, 19 Apr 2017 17:31:38 +0200 From: Gerald Schaefer To: Dan Williams Cc: linux-nvdimm@lists.01.org, linux-kernel@vger.kernel.org, linux-block@vger.kernel.org, dm-devel@redhat.com, linux-fsdevel@vger.kernel.org, hch@lst.de Subject: Re: [resend PATCH v2 08/33] dcssblk: add dax_operations support In-Reply-To: <149245617283.10206.846177305206642788.stgit@dwillia2-desk3.amr.corp.intel.com> References: <149245612770.10206.15496018295337908594.stgit@dwillia2-desk3.amr.corp.intel.com> <149245617283.10206.846177305206642788.stgit@dwillia2-desk3.amr.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Message-Id: <20170419173138.06621e66@thinkpad> Sender: linux-block-owner@vger.kernel.org List-Id: linux-block@vger.kernel.org On Mon, 17 Apr 2017 12:09:32 -0700 Dan Williams 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 > Signed-off-by: Dan Williams > --- > 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 > #include > #include > +#include > #include > #include > > @@ -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 > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 393B821954075 for ; Wed, 19 Apr 2017 08:31:49 -0700 (PDT) Received: from pps.filterd (m0098417.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.20/8.16.0.20) with SMTP id v3JFTGWC001135 for ; Wed, 19 Apr 2017 11:31:48 -0400 Received: from e06smtp14.uk.ibm.com (e06smtp14.uk.ibm.com [195.75.94.110]) by mx0a-001b2d01.pphosted.com with ESMTP id 29x77d35fg-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Wed, 19 Apr 2017 11:31:47 -0400 Received: from localhost by e06smtp14.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 19 Apr 2017 16:31:44 +0100 Date: Wed, 19 Apr 2017 17:31:38 +0200 From: Gerald Schaefer Subject: Re: [resend PATCH v2 08/33] dcssblk: add dax_operations support In-Reply-To: <149245617283.10206.846177305206642788.stgit@dwillia2-desk3.amr.corp.intel.com> References: <149245612770.10206.15496018295337908594.stgit@dwillia2-desk3.amr.corp.intel.com> <149245617283.10206.846177305206642788.stgit@dwillia2-desk3.amr.corp.intel.com> Mime-Version: 1.0 Message-Id: <20170419173138.06621e66@thinkpad> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: linux-nvdimm-bounces@lists.01.org Sender: "Linux-nvdimm" To: Dan Williams Cc: linux-nvdimm@lists.01.org, linux-kernel@vger.kernel.org, linux-block@vger.kernel.org, dm-devel@redhat.com, linux-fsdevel@vger.kernel.org, hch@lst.de List-ID: On Mon, 17 Apr 2017 12:09:32 -0700 Dan Williams 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 > Signed-off-by: Dan Williams > --- > 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 > #include > #include > +#include > #include > #include > > @@ -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 > _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1765647AbdDSPcC (ORCPT ); Wed, 19 Apr 2017 11:32:02 -0400 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:55561 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1765629AbdDSPb7 (ORCPT ); Wed, 19 Apr 2017 11:31:59 -0400 Date: Wed, 19 Apr 2017 17:31:38 +0200 From: Gerald Schaefer To: Dan Williams Cc: linux-nvdimm@ml01.01.org, linux-kernel@vger.kernel.org, linux-block@vger.kernel.org, dm-devel@redhat.com, linux-fsdevel@vger.kernel.org, hch@lst.de Subject: Re: [resend PATCH v2 08/33] dcssblk: add dax_operations support In-Reply-To: <149245617283.10206.846177305206642788.stgit@dwillia2-desk3.amr.corp.intel.com> References: <149245612770.10206.15496018295337908594.stgit@dwillia2-desk3.amr.corp.intel.com> <149245617283.10206.846177305206642788.stgit@dwillia2-desk3.amr.corp.intel.com> Organization: IBM Deutschland Research & Development GmbH / Vorsitzende des Aufsichtsrats: Martina Koederitz / Geschaeftsfuehrung: Dirk Wittkopp / Sitz der Gesellschaft: Boeblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294 X-Mailer: Claws Mail 3.9.0 (GTK+ 2.24.23; x86_64-redhat-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 x-cbid: 17041915-0016-0000-0000-00000480DF11 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17041915-0017-0000-0000-0000274F0B13 Message-Id: <20170419173138.06621e66@thinkpad> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-04-19_13:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1703280000 definitions=main-1704190130 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 17 Apr 2017 12:09:32 -0700 Dan Williams 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 > Signed-off-by: Dan Williams > --- > 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 > #include > #include > +#include > #include > #include > > @@ -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 >