Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965449AbdDSPo1 (ORCPT ); Wed, 19 Apr 2017 11:44:27 -0400 Received: from mail-yw0-f181.google.com ([209.85.161.181]:32976 "EHLO mail-yw0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S937583AbdDSPoZ (ORCPT ); Wed, 19 Apr 2017 11:44:25 -0400 MIME-Version: 1.0 In-Reply-To: <20170419173138.06621e66@thinkpad> References: <149245612770.10206.15496018295337908594.stgit@dwillia2-desk3.amr.corp.intel.com> <149245617283.10206.846177305206642788.stgit@dwillia2-desk3.amr.corp.intel.com> <20170419173138.06621e66@thinkpad> From: Dan Williams Date: Wed, 19 Apr 2017 08:44:23 -0700 Message-ID: Subject: Re: [resend PATCH v2 08/33] dcssblk: add dax_operations support To: Gerald Schaefer Cc: "linux-nvdimm@lists.01.org" , "linux-kernel@vger.kernel.org" , linux-block@vger.kernel.org, dm-devel@redhat.com, linux-fsdevel , Christoph Hellwig Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4521 Lines: 113 On Wed, Apr 19, 2017 at 8:31 AM, Gerald Schaefer wrote: > 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. Thank you for taking a look. I'll get this fixed up.