| Message ID | 20200203200029.4592-6-vgoyal@redhat.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Mike Snitzer |
| Headers | show |
| Series | dax, pmem: Provide a dax operation to zero range of memory | expand |
Hi Vivek,
I love your patch! Yet something to improve:
[auto build test ERROR on dm/for-next]
[also build test ERROR on s390/features xfs-linux/for-next linus/master linux-nvdimm/libnvdimm-for-next v5.5 next-20200203]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Vivek-Goyal/dax-pmem-Provide-a-dax-operation-to-zero-range-of-memory/20200204-082750
base: https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git for-next
config: s390-alldefconfig (attached as .config)
compiler: s390-linux-gcc (GCC) 7.5.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.5.0 make.cross ARCH=s390
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> drivers/s390/block/dcssblk.c:65:21: error: 'dcssblk_dax_zero_page_range' undeclared here (not in a function); did you mean 'generic_dax_zero_page_range'?
.zero_page_range = dcssblk_dax_zero_page_range,
^~~~~~~~~~~~~~~~~~~~~~~~~~~
generic_dax_zero_page_range
drivers/s390/block/dcssblk.c:945:12: warning: 'dcssblk_dax_zero_page_range' defined but not used [-Wunused-function]
static int dcssblk_dax_zero_page_range(struct dax_device *dax_dev,pgoff_t pgoff,
^~~~~~~~~~~~~~~~~~~~~~~~~~~
vim +65 drivers/s390/block/dcssblk.c
b3a9a0c36e1f7b Dan Williams 2018-05-02 59
7a2765f6e82063 Dan Williams 2017-01-26 60 static const struct dax_operations dcssblk_dax_ops = {
7a2765f6e82063 Dan Williams 2017-01-26 61 .direct_access = dcssblk_dax_direct_access,
7bf7eac8d64805 Dan Williams 2019-05-16 62 .dax_supported = generic_fsdax_supported,
5d61e43b3975c0 Dan Williams 2017-06-27 63 .copy_from_iter = dcssblk_dax_copy_from_iter,
b3a9a0c36e1f7b Dan Williams 2018-05-02 64 .copy_to_iter = dcssblk_dax_copy_to_iter,
c5cb636194a0d8 Vivek Goyal 2020-02-03 @65 .zero_page_range = dcssblk_dax_zero_page_range,
^1da177e4c3f41 Linus Torvalds 2005-04-16 66 };
^1da177e4c3f41 Linus Torvalds 2005-04-16 67
:::::: The code at line 65 was first introduced by commit
:::::: c5cb636194a0d8d33d549903c92189385db48406 s390,dax: Add dax zero_page_range operation to dcssblk driver
:::::: TO: Vivek Goyal <vgoyal@redhat.com>
:::::: CC: 0day robot <lkp@intel.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
> +int dax_iomap_zero(loff_t pos, unsigned offset, unsigned size, > + struct iomap *iomap) > { > pgoff_t pgoff; > long rc, id; > + sector_t sector = iomap_sector(iomap, pos & PAGE_MASK); > > - rc = bdev_dax_pgoff(bdev, sector, PAGE_SIZE, &pgoff); > + rc = bdev_dax_pgoff(iomap->bdev, sector, PAGE_SIZE, &pgoff); > if (rc) > return rc; > > id = dax_read_lock(); > - rc = dax_zero_page_range(dax_dev, pgoff, offset, size); > + rc = dax_zero_page_range(iomap->dax_dev, pgoff, offset, size); > dax_read_unlock(id); > return rc; > } > -EXPORT_SYMBOL_GPL(__dax_zero_page_range); > +EXPORT_SYMBOL_GPL(dax_iomap_zero); This function is only used by fs/iomap/buffered-io.c, so no need to export it. > #ifdef CONFIG_FS_DAX > -int __dax_zero_page_range(struct block_device *bdev, > - struct dax_device *dax_dev, sector_t sector, > - unsigned int offset, unsigned int length); > +int dax_iomap_zero(loff_t pos, unsigned offset, unsigned size, > + struct iomap *iomap); > #else > -static inline int __dax_zero_page_range(struct block_device *bdev, > - struct dax_device *dax_dev, sector_t sector, > - unsigned int offset, unsigned int length) > +static inline int dax_iomap_zero(loff_t pos, unsigned offset, unsigned size, > + struct iomap *iomap) > { > return -ENXIO; > } Given that the only caller is under an IS_DAX() check you could just declare the function unconditionally and let the compiler optimize away the guaranteed dead call for the !CONFIG_FS_DAX case, like we do with various other functions. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
On Wed, Feb 05, 2020 at 10:36:09AM -0800, Christoph Hellwig wrote: > > +int dax_iomap_zero(loff_t pos, unsigned offset, unsigned size, > > + struct iomap *iomap) > > { > > pgoff_t pgoff; > > long rc, id; > > + sector_t sector = iomap_sector(iomap, pos & PAGE_MASK); > > > > - rc = bdev_dax_pgoff(bdev, sector, PAGE_SIZE, &pgoff); > > + rc = bdev_dax_pgoff(iomap->bdev, sector, PAGE_SIZE, &pgoff); > > if (rc) > > return rc; > > > > id = dax_read_lock(); > > - rc = dax_zero_page_range(dax_dev, pgoff, offset, size); > > + rc = dax_zero_page_range(iomap->dax_dev, pgoff, offset, size); > > dax_read_unlock(id); > > return rc; > > } > > -EXPORT_SYMBOL_GPL(__dax_zero_page_range); > > +EXPORT_SYMBOL_GPL(dax_iomap_zero); > > This function is only used by fs/iomap/buffered-io.c, so no need to > export it. Will do. > > > #ifdef CONFIG_FS_DAX > > -int __dax_zero_page_range(struct block_device *bdev, > > - struct dax_device *dax_dev, sector_t sector, > > - unsigned int offset, unsigned int length); > > +int dax_iomap_zero(loff_t pos, unsigned offset, unsigned size, > > + struct iomap *iomap); > > #else > > -static inline int __dax_zero_page_range(struct block_device *bdev, > > - struct dax_device *dax_dev, sector_t sector, > > - unsigned int offset, unsigned int length) > > +static inline int dax_iomap_zero(loff_t pos, unsigned offset, unsigned size, > > + struct iomap *iomap) > > { > > return -ENXIO; > > } > > Given that the only caller is under an IS_DAX() check you could just > declare the function unconditionally and let the compiler optimize > away the guaranteed dead call for the !CONFIG_FS_DAX case, like we > do with various other functions. Sure, will do. Vivek -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
diff --git a/fs/dax.c b/fs/dax.c index 1b9ba6b59cdb..63303e274221 100644 --- a/fs/dax.c +++ b/fs/dax.c @@ -1059,23 +1059,23 @@ int generic_dax_zero_page_range(struct dax_device *dax_dev, pgoff_t pgoff, } EXPORT_SYMBOL_GPL(generic_dax_zero_page_range); -int __dax_zero_page_range(struct block_device *bdev, - struct dax_device *dax_dev, sector_t sector, - unsigned int offset, unsigned int size) +int dax_iomap_zero(loff_t pos, unsigned offset, unsigned size, + struct iomap *iomap) { pgoff_t pgoff; long rc, id; + sector_t sector = iomap_sector(iomap, pos & PAGE_MASK); - rc = bdev_dax_pgoff(bdev, sector, PAGE_SIZE, &pgoff); + rc = bdev_dax_pgoff(iomap->bdev, sector, PAGE_SIZE, &pgoff); if (rc) return rc; id = dax_read_lock(); - rc = dax_zero_page_range(dax_dev, pgoff, offset, size); + rc = dax_zero_page_range(iomap->dax_dev, pgoff, offset, size); dax_read_unlock(id); return rc; } -EXPORT_SYMBOL_GPL(__dax_zero_page_range); +EXPORT_SYMBOL_GPL(dax_iomap_zero); static loff_t dax_iomap_actor(struct inode *inode, loff_t pos, loff_t length, void *data, diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c index 828444e14d09..5a5d784a110e 100644 --- a/fs/iomap/buffered-io.c +++ b/fs/iomap/buffered-io.c @@ -974,13 +974,6 @@ static int iomap_zero(struct inode *inode, loff_t pos, unsigned offset, return iomap_write_end(inode, pos, bytes, bytes, page, iomap, srcmap); } -static int iomap_dax_zero(loff_t pos, unsigned offset, unsigned bytes, - struct iomap *iomap) -{ - return __dax_zero_page_range(iomap->bdev, iomap->dax_dev, - iomap_sector(iomap, pos & PAGE_MASK), offset, bytes); -} - static loff_t iomap_zero_range_actor(struct inode *inode, loff_t pos, loff_t count, void *data, struct iomap *iomap, struct iomap *srcmap) @@ -1000,7 +993,7 @@ iomap_zero_range_actor(struct inode *inode, loff_t pos, loff_t count, bytes = min_t(loff_t, PAGE_SIZE - offset, count); if (IS_DAX(inode)) - status = iomap_dax_zero(pos, offset, bytes, iomap); + status = dax_iomap_zero(pos, offset, bytes, iomap); else status = iomap_zero(inode, pos, offset, bytes, iomap, srcmap); diff --git a/include/linux/dax.h b/include/linux/dax.h index 3356b874c55d..ffaaa12f8ca9 100644 --- a/include/linux/dax.h +++ b/include/linux/dax.h @@ -13,6 +13,7 @@ typedef unsigned long dax_entry_t; struct iomap_ops; +struct iomap; struct dax_device; struct dax_operations { /* @@ -228,13 +229,11 @@ int dax_invalidate_mapping_entry_sync(struct address_space *mapping, pgoff_t index); #ifdef CONFIG_FS_DAX -int __dax_zero_page_range(struct block_device *bdev, - struct dax_device *dax_dev, sector_t sector, - unsigned int offset, unsigned int length); +int dax_iomap_zero(loff_t pos, unsigned offset, unsigned size, + struct iomap *iomap); #else -static inline int __dax_zero_page_range(struct block_device *bdev, - struct dax_device *dax_dev, sector_t sector, - unsigned int offset, unsigned int length) +static inline int dax_iomap_zero(loff_t pos, unsigned offset, unsigned size, + struct iomap *iomap) { return -ENXIO; }
Add a helper dax_ioamp_zero() to zero a range. This patch basically merges __dax_zero_page_range() and iomap_dax_zero(). Suggested-by: Christoph Hellwig <hch@infradead.org> Signed-off-by: Vivek Goyal <vgoyal@redhat.com> --- fs/dax.c | 12 ++++++------ fs/iomap/buffered-io.c | 9 +-------- include/linux/dax.h | 11 +++++------ 3 files changed, 12 insertions(+), 20 deletions(-)