* [PATCH 1/3] nvdimm/pmem: check the validity of the pointer pfn @ 2018-07-04 6:40 Huaisheng Ye 2018-07-04 6:40 ` [PATCH 2/3] drivers/s390/block/dcssblk: " Huaisheng Ye ` (2 more replies) 0 siblings, 3 replies; 10+ messages in thread From: Huaisheng Ye @ 2018-07-04 6:40 UTC (permalink / raw) To: linux-nvdimm Cc: dan.j.williams, ross.zwisler, mawilcox, vishal.l.verma, dave.jiang, schwidefsky, heiko.carstens, viro, martin.petersen, axboe, gregkh, bart.vanassche, jack, chengnt, linux-kernel, linux-s390, linux-fsdevel, Huaisheng Ye From: Huaisheng Ye <yehs1@lenovo.com> Some functions within fs/dax don't need to get gfn from direct_access. Assigning NULL to gfn of dax_direct_access is more intuitive and simple than offering a useless local variable. So direct_access needs to check validity of the pointer pfn For NULL assignment. Signed-off-by: Huaisheng Ye <yehs1@lenovo.com> --- drivers/nvdimm/pmem.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c index 9d71492..018f990 100644 --- a/drivers/nvdimm/pmem.c +++ b/drivers/nvdimm/pmem.c @@ -233,7 +233,8 @@ __weak long __pmem_direct_access(struct pmem_device *pmem, pgoff_t pgoff, PFN_PHYS(nr_pages)))) return -EIO; *kaddr = pmem->virt_addr + offset; - *pfn = phys_to_pfn_t(pmem->phys_addr + offset, pmem->pfn_flags); + if (pfn) + *pfn = phys_to_pfn_t(pmem->phys_addr + offset, pmem->pfn_flags); /* * If badblocks are present, limit known good range to the -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/3] drivers/s390/block/dcssblk: check the validity of the pointer pfn 2018-07-04 6:40 [PATCH 1/3] nvdimm/pmem: check the validity of the pointer pfn Huaisheng Ye @ 2018-07-04 6:40 ` Huaisheng Ye 2018-07-04 6:40 ` [PATCH 3/3] fs/dax: Assigning NULL to gfn of dax_direct_access if useless Huaisheng Ye 2018-07-04 14:40 ` [PATCH 1/3] nvdimm/pmem: check the validity of the pointer pfn Dan Williams 2 siblings, 0 replies; 10+ messages in thread From: Huaisheng Ye @ 2018-07-04 6:40 UTC (permalink / raw) To: linux-nvdimm Cc: dan.j.williams, ross.zwisler, mawilcox, vishal.l.verma, dave.jiang, schwidefsky, heiko.carstens, viro, martin.petersen, axboe, gregkh, bart.vanassche, jack, chengnt, linux-kernel, linux-s390, linux-fsdevel, Huaisheng Ye From: Huaisheng Ye <yehs1@lenovo.com> Some functions within fs/dax don't need to get gfn from direct_access. Assigning NULL to gfn of dax_direct_access is more intuitive and simple than offering a useless local variable. So direct_access needs to check validity of the pointer pfn For NULL assignment. If pfn equals to NULL, it doesn't need to calculate its value. Signed-off-by: Huaisheng Ye <yehs1@lenovo.com> --- drivers/s390/block/dcssblk.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/s390/block/dcssblk.c b/drivers/s390/block/dcssblk.c index 0a312e4..5cdfa02 100644 --- a/drivers/s390/block/dcssblk.c +++ b/drivers/s390/block/dcssblk.c @@ -916,7 +916,8 @@ static DEVICE_ATTR(save, S_IWUSR | S_IRUSR, dcssblk_save_show, 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), + if (pfn) + *pfn = __pfn_to_pfn_t(PFN_DOWN(dev_info->start + offset), PFN_DEV|PFN_SPECIAL); return (dev_sz - offset) / PAGE_SIZE; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/3] fs/dax: Assigning NULL to gfn of dax_direct_access if useless 2018-07-04 6:40 [PATCH 1/3] nvdimm/pmem: check the validity of the pointer pfn Huaisheng Ye 2018-07-04 6:40 ` [PATCH 2/3] drivers/s390/block/dcssblk: " Huaisheng Ye @ 2018-07-04 6:40 ` Huaisheng Ye 2018-07-04 11:30 ` Jan Kara 2018-07-04 14:40 ` [PATCH 1/3] nvdimm/pmem: check the validity of the pointer pfn Dan Williams 2 siblings, 1 reply; 10+ messages in thread From: Huaisheng Ye @ 2018-07-04 6:40 UTC (permalink / raw) To: linux-nvdimm Cc: dan.j.williams, ross.zwisler, mawilcox, vishal.l.verma, dave.jiang, schwidefsky, heiko.carstens, viro, martin.petersen, axboe, gregkh, bart.vanassche, jack, chengnt, linux-kernel, linux-s390, linux-fsdevel, Huaisheng Ye From: Huaisheng Ye <yehs1@lenovo.com> Some functions within fs/dax don't need to get gfn from direct_access. Assigning NULL to gfn of dax_direct_access is more intuitive and simple than offering a useless local variable. Signed-off-by: Huaisheng Ye <yehs1@lenovo.com> --- fs/dax.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/fs/dax.c b/fs/dax.c index aaec72de..aa75dfd 100644 --- a/fs/dax.c +++ b/fs/dax.c @@ -550,7 +550,6 @@ static int copy_user_dax(struct block_device *bdev, struct dax_device *dax_dev, { void *vto, *kaddr; pgoff_t pgoff; - pfn_t pfn; long rc; int id; @@ -559,7 +558,7 @@ static int copy_user_dax(struct block_device *bdev, struct dax_device *dax_dev, return rc; id = dax_read_lock(); - rc = dax_direct_access(dax_dev, pgoff, PHYS_PFN(size), &kaddr, &pfn); + rc = dax_direct_access(dax_dev, pgoff, PHYS_PFN(size), &kaddr, NULL); if (rc < 0) { dax_read_unlock(id); return rc; @@ -961,7 +960,6 @@ int __dax_zero_page_range(struct block_device *bdev, pgoff_t pgoff; long rc, id; void *kaddr; - pfn_t pfn; rc = bdev_dax_pgoff(bdev, sector, PAGE_SIZE, &pgoff); if (rc) @@ -969,7 +967,7 @@ int __dax_zero_page_range(struct block_device *bdev, id = dax_read_lock(); rc = dax_direct_access(dax_dev, pgoff, 1, &kaddr, - &pfn); + NULL); if (rc < 0) { dax_read_unlock(id); return rc; @@ -1024,7 +1022,6 @@ int __dax_zero_page_range(struct block_device *bdev, ssize_t map_len; pgoff_t pgoff; void *kaddr; - pfn_t pfn; if (fatal_signal_pending(current)) { ret = -EINTR; @@ -1036,7 +1033,7 @@ int __dax_zero_page_range(struct block_device *bdev, break; map_len = dax_direct_access(dax_dev, pgoff, PHYS_PFN(size), - &kaddr, &pfn); + &kaddr, NULL); if (map_len < 0) { ret = map_len; break; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] fs/dax: Assigning NULL to gfn of dax_direct_access if useless 2018-07-04 6:40 ` [PATCH 3/3] fs/dax: Assigning NULL to gfn of dax_direct_access if useless Huaisheng Ye @ 2018-07-04 11:30 ` Jan Kara 2018-07-04 13:07 ` Huaisheng Ye 0 siblings, 1 reply; 10+ messages in thread From: Jan Kara @ 2018-07-04 11:30 UTC (permalink / raw) To: Huaisheng Ye Cc: linux-nvdimm, dan.j.williams, ross.zwisler, mawilcox, vishal.l.verma, dave.jiang, schwidefsky, heiko.carstens, viro, martin.petersen, axboe, gregkh, bart.vanassche, jack, chengnt, linux-kernel, linux-s390, linux-fsdevel, Huaisheng Ye On Wed 04-07-18 14:40:58, Huaisheng Ye wrote: > From: Huaisheng Ye <yehs1@lenovo.com> > > Some functions within fs/dax don't need to get gfn from direct_access. > Assigning NULL to gfn of dax_direct_access is more intuitive and simple > than offering a useless local variable. > > Signed-off-by: Huaisheng Ye <yehs1@lenovo.com> I like this. You can add: Reviewed-by: Jan Kara <jack@suse.cz> for the series. Honza > --- > fs/dax.c | 9 +++------ > 1 file changed, 3 insertions(+), 6 deletions(-) > > diff --git a/fs/dax.c b/fs/dax.c > index aaec72de..aa75dfd 100644 > --- a/fs/dax.c > +++ b/fs/dax.c > @@ -550,7 +550,6 @@ static int copy_user_dax(struct block_device *bdev, struct dax_device *dax_dev, > { > void *vto, *kaddr; > pgoff_t pgoff; > - pfn_t pfn; > long rc; > int id; > > @@ -559,7 +558,7 @@ static int copy_user_dax(struct block_device *bdev, struct dax_device *dax_dev, > return rc; > > id = dax_read_lock(); > - rc = dax_direct_access(dax_dev, pgoff, PHYS_PFN(size), &kaddr, &pfn); > + rc = dax_direct_access(dax_dev, pgoff, PHYS_PFN(size), &kaddr, NULL); > if (rc < 0) { > dax_read_unlock(id); > return rc; > @@ -961,7 +960,6 @@ int __dax_zero_page_range(struct block_device *bdev, > pgoff_t pgoff; > long rc, id; > void *kaddr; > - pfn_t pfn; > > rc = bdev_dax_pgoff(bdev, sector, PAGE_SIZE, &pgoff); > if (rc) > @@ -969,7 +967,7 @@ int __dax_zero_page_range(struct block_device *bdev, > > id = dax_read_lock(); > rc = dax_direct_access(dax_dev, pgoff, 1, &kaddr, > - &pfn); > + NULL); > if (rc < 0) { > dax_read_unlock(id); > return rc; > @@ -1024,7 +1022,6 @@ int __dax_zero_page_range(struct block_device *bdev, > ssize_t map_len; > pgoff_t pgoff; > void *kaddr; > - pfn_t pfn; > > if (fatal_signal_pending(current)) { > ret = -EINTR; > @@ -1036,7 +1033,7 @@ int __dax_zero_page_range(struct block_device *bdev, > break; > > map_len = dax_direct_access(dax_dev, pgoff, PHYS_PFN(size), > - &kaddr, &pfn); > + &kaddr, NULL); > if (map_len < 0) { > ret = map_len; > break; > -- > 1.8.3.1 > > > -- Jan Kara <jack@suse.com> SUSE Labs, CR ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] fs/dax: Assigning NULL to gfn of dax_direct_access if useless 2018-07-04 11:30 ` Jan Kara @ 2018-07-04 13:07 ` Huaisheng Ye 2018-07-04 14:37 ` Dan Williams 0 siblings, 1 reply; 10+ messages in thread From: Huaisheng Ye @ 2018-07-04 13:07 UTC (permalink / raw) To: "Jan Kara" Cc: linux-nvdimm, dan.j.williams, ross.zwisler, mawilcox, vishal.l.verma, dave.jiang, schwidefsky, heiko.carstens, viro, martin.petersen, axboe, gregkh, bart.vanassche, jack, chengnt, linux-kernel, linux-s390, linux-fsdevel, "Huaisheng Ye", "colyli" ---- On Wed, 04 Jul 2018 19:30:12 +0800 Jan Kara <jack@suse.cz> wrote ---- > On Wed 04-07-18 14:40:58, Huaisheng Ye wrote: > > From: Huaisheng Ye <yehs1@lenovo.com> > > > > Some functions within fs/dax don't need to get gfn from direct_access. > > Assigning NULL to gfn of dax_direct_access is more intuitive and simple > > than offering a useless local variable. > > > > Signed-off-by: Huaisheng Ye <yehs1@lenovo.com> > > I like this. You can add: > > Reviewed-by: Jan Kara <jack@suse.cz> > > for the series. > > Honza > I am so happy you like them, thank you very much. --- Cheers, Huaisheng ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] fs/dax: Assigning NULL to gfn of dax_direct_access if useless 2018-07-04 13:07 ` Huaisheng Ye @ 2018-07-04 14:37 ` Dan Williams 2018-07-04 14:41 ` Dan Williams 0 siblings, 1 reply; 10+ messages in thread From: Dan Williams @ 2018-07-04 14:37 UTC (permalink / raw) To: yehs2007 Cc: Jan Kara, linux-nvdimm, Ross Zwisler, Matthew Wilcox, Vishal L Verma, Dave Jiang, Martin Schwidefsky, Heiko Carstens, Al Viro, Martin K. Petersen, Jens Axboe, Greg KH, Bart Van Assche, Jan Kara, NingTing Cheng, Linux Kernel Mailing List, linux-s390, linux-fsdevel, Huaisheng Ye, colyli [-- Attachment #1: Type: text/plain, Size: 1007 bytes --] On Wed, Jul 4, 2018 at 6:07 AM, Huaisheng Ye <yehs2007@zoho.com> wrote: > ---- On Wed, 04 Jul 2018 19:30:12 +0800 Jan Kara <jack@suse.cz> wrote ---- > > On Wed 04-07-18 14:40:58, Huaisheng Ye wrote: > > > From: Huaisheng Ye <yehs1@lenovo.com> > > > > > > Some functions within fs/dax don't need to get gfn from direct_access. > > > Assigning NULL to gfn of dax_direct_access is more intuitive and simple > > > than offering a useless local variable. > > > > > > Signed-off-by: Huaisheng Ye <yehs1@lenovo.com> > > > > I like this. You can add: > > > > Reviewed-by: Jan Kara <jack@suse.cz> > > > > for the series. > > > > Honza > > > I am so happy you like them, thank you very much. Yes, I like this too. In fact I have a similar patch in my tree already that I have been preparing to send out. I am using it to delay when we need to have the 'struct page' memmap for dax initialized. Attached is the full patch, but the series is still a work in progress. [-- Attachment #2: 0001-dax-Elide-pfn-lookups.patch --] [-- Type: text/x-patch, Size: 6511 bytes --] From 62e12abebacbafdb5218a92501885fe0e80ea922 Mon Sep 17 00:00:00 2001 From: Dan Williams <dan.j.williams@intel.com> Date: Wed, 27 Jun 2018 22:21:30 -0700 Subject: [PATCH] dax: Elide pfn lookups When not required, allow drivers to skip returning a pfn if one was not requested. In support of asynchronous initialization of 'struct page' there needs to be a sync point at which the memmap is made valid. The latest point where the kernel can reasonably sync is when a pfn is being used to populate userspace page tables. Otherwise, if only the kernel address is needed from dax_direct_access(), typically in-kernel users, then the 'pfn_t *' argument to dax_direct_access() can be NULL. Signed-off-by: Dan Williams <dan.j.williams@intel.com> --- drivers/dax/super.c | 17 +++++++++++------ drivers/nvdimm/pmem.c | 3 ++- drivers/s390/block/dcssblk.c | 5 +++-- fs/dax.c | 10 +++------- tools/testing/nvdimm/pmem-dax.c | 6 ++++-- 5 files changed, 23 insertions(+), 18 deletions(-) diff --git a/drivers/dax/super.c b/drivers/dax/super.c index 3b1fe4b6b00e..82256506e40b 100644 --- a/drivers/dax/super.c +++ b/drivers/dax/super.c @@ -72,7 +72,7 @@ struct dax_device *fs_dax_get_by_bdev(struct block_device *bdev) EXPORT_SYMBOL_GPL(fs_dax_get_by_bdev); #endif -static bool validate_dax_pfn(pfn_t pfn) +static bool validate_dax_pfn(pfn_t *pfn) { bool dax_enabled = false; @@ -84,7 +84,7 @@ static bool validate_dax_pfn(pfn_t pfn) if (!IS_ENABLED(CONFIG_DAX_DRIVER_DEBUG)) return true; - if (IS_ENABLED(CONFIG_FS_DAX_LIMITED) && pfn_t_special(pfn)) { + if (IS_ENABLED(CONFIG_FS_DAX_LIMITED) && pfn_t_special(*pfn)) { /* * An arch that has enabled the pmem api should also * have its drivers support pfn_t_devmap() @@ -95,10 +95,10 @@ static bool validate_dax_pfn(pfn_t pfn) */ WARN_ON(IS_ENABLED(CONFIG_ARCH_HAS_PMEM_API)); dax_enabled = true; - } else if (pfn_t_devmap(pfn)) { + } else if (pfn_t_devmap(*pfn)) { struct dev_pagemap *pgmap; - pgmap = get_dev_pagemap(pfn_t_to_pfn(pfn), NULL); + pgmap = get_dev_pagemap(pfn_t_to_pfn(*pfn), NULL); if (pgmap && pgmap->type == MEMORY_DEVICE_FS_DAX) dax_enabled = true; put_dev_pagemap(pgmap); @@ -120,10 +120,10 @@ static bool validate_dax_pfn(pfn_t pfn) bool __bdev_dax_supported(struct block_device *bdev, int blocksize) { struct dax_device *dax_dev; + pfn_t _pfn, *pfn; pgoff_t pgoff; int err, id; void *kaddr; - pfn_t pfn; long len; char buf[BDEVNAME_SIZE]; @@ -147,8 +147,13 @@ bool __bdev_dax_supported(struct block_device *bdev, int blocksize) return false; } + if (IS_ENABLED(DAX_DRIVER_DEBUG)) + pfn = &_pfn; + else + pfn = NULL; + id = dax_read_lock(); - len = dax_direct_access(dax_dev, pgoff, 1, &kaddr, &pfn); + len = dax_direct_access(dax_dev, pgoff, 1, &kaddr, pfn); dax_read_unlock(id); put_dax(dax_dev); diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c index e8ac6f244d2b..c430536320a5 100644 --- a/drivers/nvdimm/pmem.c +++ b/drivers/nvdimm/pmem.c @@ -228,7 +228,8 @@ __weak long __pmem_direct_access(struct pmem_device *pmem, pgoff_t pgoff, PFN_PHYS(nr_pages)))) return -EIO; *kaddr = pmem->virt_addr + offset; - *pfn = phys_to_pfn_t(pmem->phys_addr + offset, pmem->pfn_flags); + if (pfn) + *pfn = phys_to_pfn_t(pmem->phys_addr + offset, pmem->pfn_flags); /* * If badblocks are present, limit known good range to the diff --git a/drivers/s390/block/dcssblk.c b/drivers/s390/block/dcssblk.c index ed607288e696..a645b2c93c34 100644 --- a/drivers/s390/block/dcssblk.c +++ b/drivers/s390/block/dcssblk.c @@ -923,8 +923,9 @@ __dcssblk_direct_access(struct dcssblk_dev_info *dev_info, pgoff_t pgoff, 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|PFN_SPECIAL); + if (pfn) + *pfn = __pfn_to_pfn_t(PFN_DOWN(dev_info->start + offset), + PFN_DEV|PFN_SPECIAL); return (dev_sz - offset) / PAGE_SIZE; } diff --git a/fs/dax.c b/fs/dax.c index 641192808bb6..28264ff4e343 100644 --- a/fs/dax.c +++ b/fs/dax.c @@ -647,7 +647,6 @@ static int copy_user_dax(struct block_device *bdev, struct dax_device *dax_dev, { void *vto, *kaddr; pgoff_t pgoff; - pfn_t pfn; long rc; int id; @@ -656,7 +655,7 @@ static int copy_user_dax(struct block_device *bdev, struct dax_device *dax_dev, return rc; id = dax_read_lock(); - rc = dax_direct_access(dax_dev, pgoff, PHYS_PFN(size), &kaddr, &pfn); + rc = dax_direct_access(dax_dev, pgoff, PHYS_PFN(size), &kaddr, NULL); if (rc < 0) { dax_read_unlock(id); return rc; @@ -1052,15 +1051,13 @@ int __dax_zero_page_range(struct block_device *bdev, pgoff_t pgoff; long rc, id; void *kaddr; - pfn_t pfn; rc = bdev_dax_pgoff(bdev, sector, PAGE_SIZE, &pgoff); if (rc) return rc; id = dax_read_lock(); - rc = dax_direct_access(dax_dev, pgoff, 1, &kaddr, - &pfn); + rc = dax_direct_access(dax_dev, pgoff, 1, &kaddr, NULL); if (rc < 0) { dax_read_unlock(id); return rc; @@ -1116,7 +1113,6 @@ dax_iomap_actor(struct inode *inode, loff_t pos, loff_t length, void *data, ssize_t map_len; pgoff_t pgoff; void *kaddr; - pfn_t pfn; if (fatal_signal_pending(current)) { ret = -EINTR; @@ -1128,7 +1124,7 @@ dax_iomap_actor(struct inode *inode, loff_t pos, loff_t length, void *data, break; map_len = dax_direct_access(dax_dev, pgoff, PHYS_PFN(size), - &kaddr, &pfn); + &kaddr, NULL); if (map_len < 0) { ret = map_len; break; diff --git a/tools/testing/nvdimm/pmem-dax.c b/tools/testing/nvdimm/pmem-dax.c index b53596ad601b..d4cb5281b30e 100644 --- a/tools/testing/nvdimm/pmem-dax.c +++ b/tools/testing/nvdimm/pmem-dax.c @@ -33,7 +33,8 @@ long __pmem_direct_access(struct pmem_device *pmem, pgoff_t pgoff, *kaddr = pmem->virt_addr + offset; page = vmalloc_to_page(pmem->virt_addr + offset); - *pfn = page_to_pfn_t(page); + if (pfn) + *pfn = page_to_pfn_t(page); pr_debug_ratelimited("%s: pmem: %p pgoff: %#lx pfn: %#lx\n", __func__, pmem, pgoff, page_to_pfn(page)); @@ -41,7 +42,8 @@ long __pmem_direct_access(struct pmem_device *pmem, pgoff_t pgoff, } *kaddr = pmem->virt_addr + offset; - *pfn = phys_to_pfn_t(pmem->phys_addr + offset, pmem->pfn_flags); + if (pfn) + *pfn = phys_to_pfn_t(pmem->phys_addr + offset, pmem->pfn_flags); /* * If badblocks are present, limit known good range to the -- 2.13.6 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] fs/dax: Assigning NULL to gfn of dax_direct_access if useless 2018-07-04 14:37 ` Dan Williams @ 2018-07-04 14:41 ` Dan Williams 2018-07-04 15:35 ` [External] " Huaisheng HS1 Ye 0 siblings, 1 reply; 10+ messages in thread From: Dan Williams @ 2018-07-04 14:41 UTC (permalink / raw) To: Huaisheng Ye Cc: Jan Kara, linux-nvdimm, Ross Zwisler, Matthew Wilcox, Vishal L Verma, Dave Jiang, Martin Schwidefsky, Heiko Carstens, Al Viro, Martin K. Petersen, Jens Axboe, Greg KH, Bart Van Assche, Jan Kara, NingTing Cheng, Linux Kernel Mailing List, linux-s390, linux-fsdevel, Huaisheng Ye, colyli On Wed, Jul 4, 2018 at 7:37 AM, Dan Williams <dan.j.williams@intel.com> wrote: > On Wed, Jul 4, 2018 at 6:07 AM, Huaisheng Ye <yehs2007@zoho.com> wrote: >> ---- On Wed, 04 Jul 2018 19:30:12 +0800 Jan Kara <jack@suse.cz> wrote ---- >> > On Wed 04-07-18 14:40:58, Huaisheng Ye wrote: >> > > From: Huaisheng Ye <yehs1@lenovo.com> >> > > >> > > Some functions within fs/dax don't need to get gfn from direct_access. >> > > Assigning NULL to gfn of dax_direct_access is more intuitive and simple >> > > than offering a useless local variable. >> > > >> > > Signed-off-by: Huaisheng Ye <yehs1@lenovo.com> >> > >> > I like this. You can add: >> > >> > Reviewed-by: Jan Kara <jack@suse.cz> >> > >> > for the series. >> > >> > Honza >> > >> I am so happy you like them, thank you very much. > > Yes, I like this too. In fact I have a similar patch in my tree > already that I have been preparing to send out. I am using it to delay > when we need to have the 'struct page' memmap for dax initialized. > Attached is the full patch, but the series is still a work in > progress. Btw, I'll drop my version and apply your series since you got it posted first and it can stand alone as its own cleanup. ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [External] Re: [PATCH 3/3] fs/dax: Assigning NULL to gfn of dax_direct_access if useless 2018-07-04 14:41 ` Dan Williams @ 2018-07-04 15:35 ` Huaisheng HS1 Ye 0 siblings, 0 replies; 10+ messages in thread From: Huaisheng HS1 Ye @ 2018-07-04 15:35 UTC (permalink / raw) To: Dan Williams Cc: Jan Kara, linux-nvdimm, Ross Zwisler, Matthew Wilcox, Vishal L Verma, Dave Jiang, Martin Schwidefsky, Heiko Carstens, Al Viro, Martin K. Petersen, Jens Axboe, Greg KH, Bart Van Assche, Jan Kara, NingTing Cheng, Linux Kernel Mailing List, linux-s390, linux-fsdevel, colyli, Huaisheng Ye From: Dan Williams [mailto:dan.j.williams@intel.com] Sent: Wednesday, July 04, 2018 10:42 PM > > On Wed, Jul 4, 2018 at 7:37 AM, Dan Williams <dan.j.williams@intel.com> wrote: > > On Wed, Jul 4, 2018 at 6:07 AM, Huaisheng Ye <yehs2007@zoho.com> wrote: > >> ---- On Wed, 04 Jul 2018 19:30:12 +0800 Jan Kara <jack@suse.cz> wrote ---- > >> > On Wed 04-07-18 14:40:58, Huaisheng Ye wrote: > >> > > From: Huaisheng Ye <yehs1@lenovo.com> > >> > > > >> > > Some functions within fs/dax don't need to get gfn from direct_access. > >> > > Assigning NULL to gfn of dax_direct_access is more intuitive and simple > >> > > than offering a useless local variable. > >> > > > >> > > Signed-off-by: Huaisheng Ye <yehs1@lenovo.com> > >> > > >> > I like this. You can add: > >> > > >> > Reviewed-by: Jan Kara <jack@suse.cz> > >> > > >> > for the series. > >> > > >> > Honza > >> > > >> I am so happy you like them, thank you very much. > > > > Yes, I like this too. In fact I have a similar patch in my tree > > already that I have been preparing to send out. I am using it to delay > > when we need to have the 'struct page' memmap for dax initialized. > > Attached is the full patch, but the series is still a work in > > progress. > > Btw, I'll drop my version and apply your series since you got it > posted first and it can stand alone as its own cleanup. Hi Dan, I will resend the series later, and thanks for your help. Cheers, Huaisheng Ye ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] nvdimm/pmem: check the validity of the pointer pfn 2018-07-04 6:40 [PATCH 1/3] nvdimm/pmem: check the validity of the pointer pfn Huaisheng Ye 2018-07-04 6:40 ` [PATCH 2/3] drivers/s390/block/dcssblk: " Huaisheng Ye 2018-07-04 6:40 ` [PATCH 3/3] fs/dax: Assigning NULL to gfn of dax_direct_access if useless Huaisheng Ye @ 2018-07-04 14:40 ` Dan Williams 2018-07-04 15:35 ` [External] " Huaisheng HS1 Ye 2 siblings, 1 reply; 10+ messages in thread From: Dan Williams @ 2018-07-04 14:40 UTC (permalink / raw) To: Huaisheng Ye Cc: linux-nvdimm, Ross Zwisler, Matthew Wilcox, Vishal L Verma, Dave Jiang, Martin Schwidefsky, Heiko Carstens, Al Viro, Martin K. Petersen, Jens Axboe, Greg KH, Bart Van Assche, Jan Kara, NingTing Cheng, Linux Kernel Mailing List, linux-s390, linux-fsdevel, Huaisheng Ye On Tue, Jul 3, 2018 at 11:40 PM, Huaisheng Ye <yehs2007@zoho.com> wrote: > From: Huaisheng Ye <yehs1@lenovo.com> > > Some functions within fs/dax don't need to get gfn from direct_access. > Assigning NULL to gfn of dax_direct_access is more intuitive and simple > than offering a useless local variable. > > So direct_access needs to check validity of the pointer pfn For NULL > assignment. > > Signed-off-by: Huaisheng Ye <yehs1@lenovo.com> > --- > drivers/nvdimm/pmem.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c > index 9d71492..018f990 100644 > --- a/drivers/nvdimm/pmem.c > +++ b/drivers/nvdimm/pmem.c > @@ -233,7 +233,8 @@ __weak long __pmem_direct_access(struct pmem_device *pmem, pgoff_t pgoff, > PFN_PHYS(nr_pages)))) > return -EIO; > *kaddr = pmem->virt_addr + offset; > - *pfn = phys_to_pfn_t(pmem->phys_addr + offset, pmem->pfn_flags); > + if (pfn) > + *pfn = phys_to_pfn_t(pmem->phys_addr + offset, pmem->pfn_flags); > > /* > * If badblocks are present, limit known good range to the Looks good. You also need to update the unit test infrastructure version of this operation in: tools/testing/nvdimm/pmem-dax.c ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [External] Re: [PATCH 1/3] nvdimm/pmem: check the validity of the pointer pfn 2018-07-04 14:40 ` [PATCH 1/3] nvdimm/pmem: check the validity of the pointer pfn Dan Williams @ 2018-07-04 15:35 ` Huaisheng HS1 Ye 0 siblings, 0 replies; 10+ messages in thread From: Huaisheng HS1 Ye @ 2018-07-04 15:35 UTC (permalink / raw) To: Dan Williams Cc: linux-nvdimm, Ross Zwisler, Matthew Wilcox, Vishal L Verma, Dave Jiang, Martin Schwidefsky, Heiko Carstens, Al Viro, Martin K. Petersen, Jens Axboe, Greg KH, Bart Van Assche, Jan Kara, NingTing Cheng, Linux Kernel Mailing List, linux-s390, linux-fsdevel, Huaisheng Ye > From: Dan Williams [mailto:dan.j.williams@intel.com] > Sent: Wednesday, July 04, 2018 10:40 PM > On Tue, Jul 3, 2018 at 11:40 PM, Huaisheng Ye <yehs2007@zoho.com> wrote: > > From: Huaisheng Ye <yehs1@lenovo.com> > > > > Some functions within fs/dax don't need to get gfn from direct_access. > > Assigning NULL to gfn of dax_direct_access is more intuitive and simple > > than offering a useless local variable. > > > > So direct_access needs to check validity of the pointer pfn For NULL > > assignment. > > > > Signed-off-by: Huaisheng Ye <yehs1@lenovo.com> > > --- > > drivers/nvdimm/pmem.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c > > index 9d71492..018f990 100644 > > --- a/drivers/nvdimm/pmem.c > > +++ b/drivers/nvdimm/pmem.c > > @@ -233,7 +233,8 @@ __weak long __pmem_direct_access(struct pmem_device *pmem, > pgoff_t pgoff, > > PFN_PHYS(nr_pages)))) > > return -EIO; > > *kaddr = pmem->virt_addr + offset; > > - *pfn = phys_to_pfn_t(pmem->phys_addr + offset, pmem->pfn_flags); > > + if (pfn) > > + *pfn = phys_to_pfn_t(pmem->phys_addr + offset, > pmem->pfn_flags); > > > > /* > > * If badblocks are present, limit known good range to the > > Looks good. You also need to update the unit test infrastructure > version of this operation in: > > tools/testing/nvdimm/pmem-dax.c Yes, you are right. Cheers, Huaisheng Ye ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-07-04 15:36 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-07-04 6:40 [PATCH 1/3] nvdimm/pmem: check the validity of the pointer pfn Huaisheng Ye 2018-07-04 6:40 ` [PATCH 2/3] drivers/s390/block/dcssblk: " Huaisheng Ye 2018-07-04 6:40 ` [PATCH 3/3] fs/dax: Assigning NULL to gfn of dax_direct_access if useless Huaisheng Ye 2018-07-04 11:30 ` Jan Kara 2018-07-04 13:07 ` Huaisheng Ye 2018-07-04 14:37 ` Dan Williams 2018-07-04 14:41 ` Dan Williams 2018-07-04 15:35 ` [External] " Huaisheng HS1 Ye 2018-07-04 14:40 ` [PATCH 1/3] nvdimm/pmem: check the validity of the pointer pfn Dan Williams 2018-07-04 15:35 ` [External] " Huaisheng HS1 Ye
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).