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 Content-Length: 5993 Lines: 170 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 >