Linux-XFS Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/9 V6] New ->fiemap infrastructure and ->bmap removal
@ 2019-09-11 13:43 Carlos Maiolino
  2019-09-11 13:43 ` [PATCH 1/9] fs: Enable bmap() function to properly return errors Carlos Maiolino
                   ` (8 more replies)
  0 siblings, 9 replies; 24+ messages in thread
From: Carlos Maiolino @ 2019-09-11 13:43 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: hch, adilger, darrick.wong, linux-xfs

Hi.

This is the 6th version of the complete series with the goal to deprecate and
eventually remove ->bmap() interface, in lieu if FIEMAP.

This V6, compared with the previous one, is rebased agains next-20190904, and
addresses a few issues found by kbuild test robot, and other points discussed in
previous version.

Detailed information are in each patch description, but the biggest change
in this version is the removal of FIEMAP_KERNEL_FIBMAP flag in patch 8, so,
reducing patch's complexity and avoiding any specific filesystem modification.

The impact of such change is further detailed in patch 8.

Carlos Maiolino (9):
  fs: Enable bmap() function to properly return errors
  cachefiles: drop direct usage of ->bmap method.
  ecryptfs: drop direct calls to ->bmap
  fibmap: Use bmap instead of ->bmap method in ioctl_fibmap
  fs: Move start and length fiemap fields into fiemap_extent_info
  iomap: Remove length and start fields from iomap_fiemap
  fiemap: Use a callback to fill fiemap extents
  Use FIEMAP for FIBMAP calls
  xfs: Get rid of ->bmap

 drivers/md/md-bitmap.c |  16 +++---
 fs/bad_inode.c         |   3 +-
 fs/btrfs/inode.c       |   5 +-
 fs/cachefiles/rdwr.c   |  27 +++++-----
 fs/cifs/cifsfs.h       |   3 +-
 fs/cifs/inode.c        |   5 +-
 fs/ecryptfs/mmap.c     |  16 +++---
 fs/ext2/ext2.h         |   3 +-
 fs/ext2/inode.c        |   6 +--
 fs/ext4/ext4.h         |   6 +--
 fs/ext4/extents.c      |  18 +++----
 fs/ext4/ioctl.c        |   8 +--
 fs/f2fs/data.c         |  21 +++++---
 fs/f2fs/f2fs.h         |   3 +-
 fs/gfs2/inode.c        |   5 +-
 fs/hpfs/file.c         |   4 +-
 fs/inode.c             | 108 +++++++++++++++++++++++++++++++++++-----
 fs/ioctl.c             | 109 ++++++++++++++++++++++++-----------------
 fs/iomap/fiemap.c      |   4 +-
 fs/jbd2/journal.c      |  22 ++++++---
 fs/nilfs2/inode.c      |   5 +-
 fs/nilfs2/nilfs.h      |   3 +-
 fs/ocfs2/extent_map.c  |   6 ++-
 fs/ocfs2/extent_map.h  |   3 +-
 fs/overlayfs/inode.c   |   5 +-
 fs/xfs/xfs_aops.c      |  24 ---------
 fs/xfs/xfs_iops.c      |  14 ++----
 fs/xfs/xfs_trace.h     |   1 -
 include/linux/fs.h     |  38 +++++++++-----
 include/linux/iomap.h  |   2 +-
 mm/page_io.c           |  11 +++--
 31 files changed, 304 insertions(+), 200 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 1/9] fs: Enable bmap() function to properly return errors
  2019-09-11 13:43 [PATCH 0/9 V6] New ->fiemap infrastructure and ->bmap removal Carlos Maiolino
@ 2019-09-11 13:43 ` Carlos Maiolino
  2019-09-11 13:43 ` [PATCH 2/9] cachefiles: drop direct usage of ->bmap method Carlos Maiolino
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 24+ messages in thread
From: Carlos Maiolino @ 2019-09-11 13:43 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: hch, adilger, darrick.wong, linux-xfs

By now, bmap() will either return the physical block number related to
the requested file offset or 0 in case of error or the requested offset
maps into a hole.
This patch makes the needed changes to enable bmap() to proper return
errors, using the return value as an error return, and now, a pointer
must be passed to bmap() to be filled with the mapped physical block.

It will change the behavior of bmap() on return:

- negative value in case of error
- zero on success or map fell into a hole

In case of a hole, the *block will be zero too

Since this is a prep patch, by now, the only error return is -EINVAL if
->bmap doesn't exist.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
Changelog:

	V6:
		- Fix bmap() doc function
			Reported-by: kbuild test robot <lkp@intel.com>
	V5:
		- Rebasing against 5.3 required changes to the f2fs
		  check_swap_activate() function

 drivers/md/md-bitmap.c | 16 ++++++++++------
 fs/f2fs/data.c         | 16 +++++++++++-----
 fs/inode.c             | 30 +++++++++++++++++-------------
 fs/jbd2/journal.c      | 22 +++++++++++++++-------
 include/linux/fs.h     |  2 +-
 mm/page_io.c           | 11 +++++++----
 6 files changed, 61 insertions(+), 36 deletions(-)

diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index b092c7b5282f..bfb7fddd4f1b 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -364,7 +364,7 @@ static int read_page(struct file *file, unsigned long index,
 	int ret = 0;
 	struct inode *inode = file_inode(file);
 	struct buffer_head *bh;
-	sector_t block;
+	sector_t block, blk_cur;
 
 	pr_debug("read bitmap file (%dB @ %llu)\n", (int)PAGE_SIZE,
 		 (unsigned long long)index << PAGE_SHIFT);
@@ -375,17 +375,21 @@ static int read_page(struct file *file, unsigned long index,
 		goto out;
 	}
 	attach_page_buffers(page, bh);
-	block = index << (PAGE_SHIFT - inode->i_blkbits);
+	blk_cur = index << (PAGE_SHIFT - inode->i_blkbits);
 	while (bh) {
+		block = blk_cur;
+
 		if (count == 0)
 			bh->b_blocknr = 0;
 		else {
-			bh->b_blocknr = bmap(inode, block);
-			if (bh->b_blocknr == 0) {
-				/* Cannot use this file! */
+			ret = bmap(inode, &block);
+			if (ret || !block) {
 				ret = -EINVAL;
+				bh->b_blocknr = 0;
 				goto out;
 			}
+
+			bh->b_blocknr = block;
 			bh->b_bdev = inode->i_sb->s_bdev;
 			if (count < (1<<inode->i_blkbits))
 				count = 0;
@@ -399,7 +403,7 @@ static int read_page(struct file *file, unsigned long index,
 			set_buffer_mapped(bh);
 			submit_bh(REQ_OP_READ, 0, bh);
 		}
-		block++;
+		blk_cur++;
 		bh = bh->b_this_page;
 	}
 	page->index = index;
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index ada78a0f49bf..49ff6d2239c3 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -3065,12 +3065,16 @@ static int check_swap_activate(struct file *swap_file, unsigned int max)
 	while ((probe_block + blocks_per_page) <= last_block && page_no < max) {
 		unsigned block_in_page;
 		sector_t first_block;
+		sector_t block = 0;
+		int	 err = 0;
 
 		cond_resched();
 
-		first_block = bmap(inode, probe_block);
-		if (first_block == 0)
+		block = probe_block;
+		err = bmap(inode, &block);
+		if (err || !block)
 			goto bad_bmap;
+		first_block = block;
 
 		/*
 		 * It must be PAGE_SIZE aligned on-disk
@@ -3082,11 +3086,13 @@ static int check_swap_activate(struct file *swap_file, unsigned int max)
 
 		for (block_in_page = 1; block_in_page < blocks_per_page;
 					block_in_page++) {
-			sector_t block;
 
-			block = bmap(inode, probe_block + block_in_page);
-			if (block == 0)
+			block = probe_block + block_in_page;
+			err = bmap(inode, &block);
+
+			if (err || !block)
 				goto bad_bmap;
+
 			if (block != first_block + block_in_page) {
 				/* Discontiguity */
 				probe_block++;
diff --git a/fs/inode.c b/fs/inode.c
index fef457a42882..0a20aaa572f2 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1593,21 +1593,25 @@ EXPORT_SYMBOL(iput);
 
 /**
  *	bmap	- find a block number in a file
- *	@inode: inode of file
- *	@block: block to find
- *
- *	Returns the block number on the device holding the inode that
- *	is the disk block number for the block of the file requested.
- *	That is, asked for block 4 of inode 1 the function will return the
- *	disk block relative to the disk start that holds that block of the
- *	file.
+ *	@inode:  inode owning the block number being requested
+ *	@block: pointer containing the block to find
+ *
+ *	Replaces the value in *block with the block number on the device holding
+ *	corresponding to the requested block number in the file.
+ *	That is, asked for block 4 of inode 1 the function will replace the
+ *	4 in *block, with disk block relative to the disk start that holds that
+ *	block of the file.
+ *
+ *	Returns -EINVAL in case of error, 0 otherwise. If mapping falls into a
+ *	hole, returns 0 and *block is also set to 0.
  */
-sector_t bmap(struct inode *inode, sector_t block)
+int bmap(struct inode *inode, sector_t *block)
 {
-	sector_t res = 0;
-	if (inode->i_mapping->a_ops->bmap)
-		res = inode->i_mapping->a_ops->bmap(inode->i_mapping, block);
-	return res;
+	if (!inode->i_mapping->a_ops->bmap)
+		return -EINVAL;
+
+	*block = inode->i_mapping->a_ops->bmap(inode->i_mapping, *block);
+	return 0;
 }
 EXPORT_SYMBOL(bmap);
 
diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index 1c58859aa592..5664101c73a8 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -795,18 +795,23 @@ int jbd2_journal_bmap(journal_t *journal, unsigned long blocknr,
 {
 	int err = 0;
 	unsigned long long ret;
+	sector_t block = 0;
 
 	if (journal->j_inode) {
-		ret = bmap(journal->j_inode, blocknr);
-		if (ret)
-			*retp = ret;
-		else {
+		block = blocknr;
+		ret = bmap(journal->j_inode, &block);
+
+		if (ret || !block) {
 			printk(KERN_ALERT "%s: journal block not found "
 					"at offset %lu on %s\n",
 			       __func__, blocknr, journal->j_devname);
 			err = -EIO;
 			__journal_abort_soft(journal, err);
+
+		} else {
+			*retp = block;
 		}
+
 	} else {
 		*retp = blocknr; /* +journal->j_blk_offset */
 	}
@@ -1232,11 +1237,14 @@ journal_t *jbd2_journal_init_dev(struct block_device *bdev,
 journal_t *jbd2_journal_init_inode(struct inode *inode)
 {
 	journal_t *journal;
+	sector_t blocknr;
 	char *p;
-	unsigned long long blocknr;
+	int err = 0;
+
+	blocknr = 0;
+	err = bmap(inode, &blocknr);
 
-	blocknr = bmap(inode, 0);
-	if (!blocknr) {
+	if (err || !blocknr) {
 		pr_err("%s: Cannot locate journal superblock\n",
 			__func__);
 		return NULL;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index c6495389bb43..d787ceefc937 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2860,7 +2860,7 @@ static inline ssize_t generic_write_sync(struct kiocb *iocb, ssize_t count)
 extern void emergency_sync(void);
 extern void emergency_remount(void);
 #ifdef CONFIG_BLOCK
-extern sector_t bmap(struct inode *, sector_t);
+extern int bmap(struct inode *, sector_t *);
 #endif
 extern int notify_change(struct dentry *, struct iattr *, struct inode **);
 extern int inode_permission(struct inode *, int);
diff --git a/mm/page_io.c b/mm/page_io.c
index 24ee600f9131..dfc55616ff17 100644
--- a/mm/page_io.c
+++ b/mm/page_io.c
@@ -176,8 +176,9 @@ int generic_swapfile_activate(struct swap_info_struct *sis,
 
 		cond_resched();
 
-		first_block = bmap(inode, probe_block);
-		if (first_block == 0)
+		first_block = probe_block;
+		ret = bmap(inode, &first_block);
+		if (ret || !first_block)
 			goto bad_bmap;
 
 		/*
@@ -192,9 +193,11 @@ int generic_swapfile_activate(struct swap_info_struct *sis,
 					block_in_page++) {
 			sector_t block;
 
-			block = bmap(inode, probe_block + block_in_page);
-			if (block == 0)
+			block = probe_block + block_in_page;
+			ret = bmap(inode, &block);
+			if (ret || !block)
 				goto bad_bmap;
+
 			if (block != first_block + block_in_page) {
 				/* Discontiguity */
 				probe_block++;
-- 
2.20.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 2/9] cachefiles: drop direct usage of ->bmap method.
  2019-09-11 13:43 [PATCH 0/9 V6] New ->fiemap infrastructure and ->bmap removal Carlos Maiolino
  2019-09-11 13:43 ` [PATCH 1/9] fs: Enable bmap() function to properly return errors Carlos Maiolino
@ 2019-09-11 13:43 ` Carlos Maiolino
  2019-09-11 13:43 ` [PATCH 3/9] ecryptfs: drop direct calls to ->bmap Carlos Maiolino
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 24+ messages in thread
From: Carlos Maiolino @ 2019-09-11 13:43 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: hch, adilger, darrick.wong, linux-xfs

Replace the direct usage of ->bmap method by a bmap() call.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---

Changelog:
	V6:
		- ASSERT only if filesystem does not support bmap() to
		  match the original logic


 fs/cachefiles/rdwr.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/fs/cachefiles/rdwr.c b/fs/cachefiles/rdwr.c
index 44a3ce1e4ce4..1dc97f2d6201 100644
--- a/fs/cachefiles/rdwr.c
+++ b/fs/cachefiles/rdwr.c
@@ -396,7 +396,7 @@ int cachefiles_read_or_alloc_page(struct fscache_retrieval *op,
 	struct cachefiles_object *object;
 	struct cachefiles_cache *cache;
 	struct inode *inode;
-	sector_t block0, block;
+	sector_t block;
 	unsigned shift;
 	int ret;
 
@@ -412,7 +412,6 @@ int cachefiles_read_or_alloc_page(struct fscache_retrieval *op,
 
 	inode = d_backing_inode(object->backer);
 	ASSERT(S_ISREG(inode->i_mode));
-	ASSERT(inode->i_mapping->a_ops->bmap);
 	ASSERT(inode->i_mapping->a_ops->readpages);
 
 	/* calculate the shift required to use bmap */
@@ -428,12 +427,14 @@ int cachefiles_read_or_alloc_page(struct fscache_retrieval *op,
 	 *   enough for this as it doesn't indicate errors, but it's all we've
 	 *   got for the moment
 	 */
-	block0 = page->index;
-	block0 <<= shift;
+	block = page->index;
+	block <<= shift;
+
+	ret = bmap(inode, &block);
+	ASSERT(ret < 0);
 
-	block = inode->i_mapping->a_ops->bmap(inode->i_mapping, block0);
 	_debug("%llx -> %llx",
-	       (unsigned long long) block0,
+	       (unsigned long long) (page->index << shift),
 	       (unsigned long long) block);
 
 	if (block) {
@@ -711,7 +712,6 @@ int cachefiles_read_or_alloc_pages(struct fscache_retrieval *op,
 
 	inode = d_backing_inode(object->backer);
 	ASSERT(S_ISREG(inode->i_mode));
-	ASSERT(inode->i_mapping->a_ops->bmap);
 	ASSERT(inode->i_mapping->a_ops->readpages);
 
 	/* calculate the shift required to use bmap */
@@ -728,7 +728,7 @@ int cachefiles_read_or_alloc_pages(struct fscache_retrieval *op,
 
 	ret = space ? -ENODATA : -ENOBUFS;
 	list_for_each_entry_safe(page, _n, pages, lru) {
-		sector_t block0, block;
+		sector_t block;
 
 		/* we assume the absence or presence of the first block is a
 		 * good enough indication for the page as a whole
@@ -736,13 +736,14 @@ int cachefiles_read_or_alloc_pages(struct fscache_retrieval *op,
 		 *   good enough for this as it doesn't indicate errors, but
 		 *   it's all we've got for the moment
 		 */
-		block0 = page->index;
-		block0 <<= shift;
+		block = page->index;
+		block <<= shift;
+
+		ret = bmap(inode, &block);
+		ASSERT(!ret);
 
-		block = inode->i_mapping->a_ops->bmap(inode->i_mapping,
-						      block0);
 		_debug("%llx -> %llx",
-		       (unsigned long long) block0,
+		       (unsigned long long) (page->index << shift),
 		       (unsigned long long) block);
 
 		if (block) {
-- 
2.20.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 3/9] ecryptfs: drop direct calls to ->bmap
  2019-09-11 13:43 [PATCH 0/9 V6] New ->fiemap infrastructure and ->bmap removal Carlos Maiolino
  2019-09-11 13:43 ` [PATCH 1/9] fs: Enable bmap() function to properly return errors Carlos Maiolino
  2019-09-11 13:43 ` [PATCH 2/9] cachefiles: drop direct usage of ->bmap method Carlos Maiolino
@ 2019-09-11 13:43 ` Carlos Maiolino
  2019-09-11 13:43 ` [PATCH 4/9] fibmap: Use bmap instead of ->bmap method in ioctl_fibmap Carlos Maiolino
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 24+ messages in thread
From: Carlos Maiolino @ 2019-09-11 13:43 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: hch, adilger, darrick.wong, linux-xfs

Replace direct ->bmap calls by bmap() method.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/ecryptfs/mmap.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/fs/ecryptfs/mmap.c b/fs/ecryptfs/mmap.c
index cffa0c1ec829..019572c6b39a 100644
--- a/fs/ecryptfs/mmap.c
+++ b/fs/ecryptfs/mmap.c
@@ -524,16 +524,12 @@ static int ecryptfs_write_end(struct file *file,
 
 static sector_t ecryptfs_bmap(struct address_space *mapping, sector_t block)
 {
-	int rc = 0;
-	struct inode *inode;
-	struct inode *lower_inode;
-
-	inode = (struct inode *)mapping->host;
-	lower_inode = ecryptfs_inode_to_lower(inode);
-	if (lower_inode->i_mapping->a_ops->bmap)
-		rc = lower_inode->i_mapping->a_ops->bmap(lower_inode->i_mapping,
-							 block);
-	return rc;
+	struct inode *lower_inode = ecryptfs_inode_to_lower(mapping->host);
+	int ret = bmap(lower_inode, &block);
+
+	if (ret)
+		return 0;
+	return block;
 }
 
 const struct address_space_operations ecryptfs_aops = {
-- 
2.20.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 4/9] fibmap: Use bmap instead of ->bmap method in ioctl_fibmap
  2019-09-11 13:43 [PATCH 0/9 V6] New ->fiemap infrastructure and ->bmap removal Carlos Maiolino
                   ` (2 preceding siblings ...)
  2019-09-11 13:43 ` [PATCH 3/9] ecryptfs: drop direct calls to ->bmap Carlos Maiolino
@ 2019-09-11 13:43 ` Carlos Maiolino
  2019-09-11 13:43 ` [PATCH 5/9] fs: Move start and length fiemap fields into fiemap_extent_info Carlos Maiolino
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 24+ messages in thread
From: Carlos Maiolino @ 2019-09-11 13:43 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: hch, adilger, darrick.wong, linux-xfs

Now we have the possibility of proper error return in bmap, use bmap()
function in ioctl_fibmap() instead of calling ->bmap method directly.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
Changelog:

	V6:
		- Add a dummy bmap() definition so build does not break if
		  CONFIG_BLOCK is not set
			Reported-by: kbuild test robot <lkp@intel.com>
	V4:
		- Ensure ioctl_fibmap() returns 0 in case of error returned from
		  bmap(). Otherwise we'll be changing the user interface (which
		  returns 0 in case of error)
	V3:
		- Rename usr_blk to ur_block
	V2:
		- Use a local sector_t variable to asign the block number
		  instead of using direct casting.

 fs/ioctl.c         | 29 +++++++++++++++++++----------
 include/linux/fs.h |  7 +++++++
 2 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/fs/ioctl.c b/fs/ioctl.c
index fef3a6bf7c78..6b589c873bc2 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -53,19 +53,28 @@ EXPORT_SYMBOL(vfs_ioctl);
 
 static int ioctl_fibmap(struct file *filp, int __user *p)
 {
-	struct address_space *mapping = filp->f_mapping;
-	int res, block;
+	struct inode *inode = file_inode(filp);
+	int error, ur_block;
+	sector_t block;
 
-	/* do we support this mess? */
-	if (!mapping->a_ops->bmap)
-		return -EINVAL;
 	if (!capable(CAP_SYS_RAWIO))
 		return -EPERM;
-	res = get_user(block, p);
-	if (res)
-		return res;
-	res = mapping->a_ops->bmap(mapping, block);
-	return put_user(res, p);
+
+	error = get_user(ur_block, p);
+	if (error)
+		return error;
+
+	block = ur_block;
+	error = bmap(inode, &block);
+
+	if (error)
+		ur_block = 0;
+	else
+		ur_block = block;
+
+	error = put_user(ur_block, p);
+
+	return error;
 }
 
 /**
diff --git a/include/linux/fs.h b/include/linux/fs.h
index d787ceefc937..f1fbd8298ca4 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2859,9 +2859,16 @@ static inline ssize_t generic_write_sync(struct kiocb *iocb, ssize_t count)
 
 extern void emergency_sync(void);
 extern void emergency_remount(void);
+
 #ifdef CONFIG_BLOCK
 extern int bmap(struct inode *, sector_t *);
+#else
+static inline int bmap(struct inode *,  sector_t *)
+{
+	return -EINVAL;
+}
 #endif
+
 extern int notify_change(struct dentry *, struct iattr *, struct inode **);
 extern int inode_permission(struct inode *, int);
 extern int generic_permission(struct inode *, int);
-- 
2.20.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 5/9] fs: Move start and length fiemap fields into fiemap_extent_info
  2019-09-11 13:43 [PATCH 0/9 V6] New ->fiemap infrastructure and ->bmap removal Carlos Maiolino
                   ` (3 preceding siblings ...)
  2019-09-11 13:43 ` [PATCH 4/9] fibmap: Use bmap instead of ->bmap method in ioctl_fibmap Carlos Maiolino
@ 2019-09-11 13:43 ` Carlos Maiolino
  2019-09-16 17:42   ` Darrick J. Wong
  2019-09-11 13:43 ` [PATCH 6/9] iomap: Remove length and start fields from iomap_fiemap Carlos Maiolino
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 24+ messages in thread
From: Carlos Maiolino @ 2019-09-11 13:43 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: hch, adilger, darrick.wong, linux-xfs

As the overall goal to deprecate fibmap, Christoph suggested a rework of
the ->fiemap API, in a way we could pass to it a callback to fill the
fiemap structure (one of these callbacks being fiemap_fill_next_extent).

To avoid the need to add several fields into the ->fiemap method, aggregate
everything into a single data structure, and pass it along.

This patch isn't suppose to add any functional change, only to update
filesystems providing ->fiemap() method.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
Changelog:

	V6:
		- Update cifs_fiemap to comply to the new ->fiemap()
		  interface
			Reported-by: kbuild test robot <lkp@intel.com>
		- Fix ext4 conflict with the new EXT4_IOC_GET_ES_CACHE
		  ioctl
	V5:
		- Add comments to fi_start and fi_len fields of
		  fiemap_extent_info
		- Fix xfs_vn_fiemap indentation

 fs/bad_inode.c        |  3 +--
 fs/btrfs/inode.c      |  5 +++--
 fs/cifs/cifsfs.h      |  3 +--
 fs/cifs/inode.c       |  5 +++--
 fs/ext2/ext2.h        |  3 +--
 fs/ext2/inode.c       |  6 ++----
 fs/ext4/ext4.h        |  6 ++----
 fs/ext4/extents.c     | 18 +++++++-----------
 fs/ext4/ioctl.c       |  4 +++-
 fs/f2fs/data.c        |  5 +++--
 fs/f2fs/f2fs.h        |  3 +--
 fs/gfs2/inode.c       |  5 +++--
 fs/hpfs/file.c        |  4 ++--
 fs/ioctl.c            | 16 ++++++++++------
 fs/nilfs2/inode.c     |  5 +++--
 fs/nilfs2/nilfs.h     |  3 +--
 fs/ocfs2/extent_map.c |  5 +++--
 fs/ocfs2/extent_map.h |  3 +--
 fs/overlayfs/inode.c  |  5 ++---
 fs/xfs/xfs_iops.c     | 10 +++++-----
 include/linux/fs.h    | 23 +++++++++++++----------
 21 files changed, 70 insertions(+), 70 deletions(-)

diff --git a/fs/bad_inode.c b/fs/bad_inode.c
index 8035d2a44561..21dfaf876814 100644
--- a/fs/bad_inode.c
+++ b/fs/bad_inode.c
@@ -120,8 +120,7 @@ static struct posix_acl *bad_inode_get_acl(struct inode *inode, int type)
 }
 
 static int bad_inode_fiemap(struct inode *inode,
-			    struct fiemap_extent_info *fieinfo, u64 start,
-			    u64 len)
+			    struct fiemap_extent_info *fieinfo)
 {
 	return -EIO;
 }
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index ef9310553a73..3d42f07c31b0 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -8762,9 +8762,10 @@ static ssize_t btrfs_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
 
 #define BTRFS_FIEMAP_FLAGS	(FIEMAP_FLAG_SYNC)
 
-static int btrfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-		__u64 start, __u64 len)
+static int btrfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
 {
+	u64	start = fieinfo->fi_start;
+	u64	len = fieinfo->fi_len;
 	int	ret;
 
 	ret = fiemap_check_flags(fieinfo, BTRFS_FIEMAP_FLAGS);
diff --git a/fs/cifs/cifsfs.h b/fs/cifs/cifsfs.h
index 99caf77df4a2..ca4c9443cd4e 100644
--- a/fs/cifs/cifsfs.h
+++ b/fs/cifs/cifsfs.h
@@ -84,8 +84,7 @@ extern int cifs_revalidate_mapping(struct inode *inode);
 extern int cifs_zap_mapping(struct inode *inode);
 extern int cifs_getattr(const struct path *, struct kstat *, u32, unsigned int);
 extern int cifs_setattr(struct dentry *, struct iattr *);
-extern int cifs_fiemap(struct inode *, struct fiemap_extent_info *, u64 start,
-		       u64 len);
+extern int cifs_fiemap(struct inode *, struct fiemap_extent_info *);
 
 extern const struct inode_operations cifs_file_inode_ops;
 extern const struct inode_operations cifs_symlink_inode_ops;
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index 26cdfbf1e164..ad5731c5a7d3 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -2123,14 +2123,15 @@ int cifs_getattr(const struct path *path, struct kstat *stat,
 	return rc;
 }
 
-int cifs_fiemap(struct inode *inode, struct fiemap_extent_info *fei, u64 start,
-		u64 len)
+int cifs_fiemap(struct inode *inode, struct fiemap_extent_info *fei)
 {
 	struct cifsInodeInfo *cifs_i = CIFS_I(inode);
 	struct cifs_sb_info *cifs_sb = CIFS_SB(cifs_i->vfs_inode.i_sb);
 	struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb);
 	struct TCP_Server_Info *server = tcon->ses->server;
 	struct cifsFileInfo *cfile;
+	u64 start = fei->fi_start;
+	u64 len = fei->fi_len;
 	int rc;
 
 	/*
diff --git a/fs/ext2/ext2.h b/fs/ext2/ext2.h
index 10ab238de9a6..284df1af9474 100644
--- a/fs/ext2/ext2.h
+++ b/fs/ext2/ext2.h
@@ -760,8 +760,7 @@ extern int ext2_get_block(struct inode *, sector_t, struct buffer_head *, int);
 extern int ext2_setattr (struct dentry *, struct iattr *);
 extern int ext2_getattr (const struct path *, struct kstat *, u32, unsigned int);
 extern void ext2_set_inode_flags(struct inode *inode);
-extern int ext2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-		       u64 start, u64 len);
+extern int ext2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo);
 
 /* ioctl.c */
 extern long ext2_ioctl(struct file *, unsigned int, unsigned long);
diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
index 7004ce581a32..6956df98dd53 100644
--- a/fs/ext2/inode.c
+++ b/fs/ext2/inode.c
@@ -857,11 +857,9 @@ const struct iomap_ops ext2_iomap_ops = {
 const struct iomap_ops ext2_iomap_ops;
 #endif /* CONFIG_FS_DAX */
 
-int ext2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-		u64 start, u64 len)
+int ext2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
 {
-	return generic_block_fiemap(inode, fieinfo, start, len,
-				    ext2_get_block);
+	return generic_block_fiemap(inode, fieinfo, ext2_get_block);
 }
 
 static int ext2_writepage(struct page *page, struct writeback_control *wbc)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 99a10f3ec440..be0aa671d701 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -3285,11 +3285,9 @@ extern struct ext4_ext_path *ext4_find_extent(struct inode *, ext4_lblk_t,
 extern void ext4_ext_drop_refs(struct ext4_ext_path *);
 extern int ext4_ext_check_inode(struct inode *inode);
 extern ext4_lblk_t ext4_ext_next_allocated_block(struct ext4_ext_path *path);
-extern int ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-			__u64 start, __u64 len);
 extern int ext4_get_es_cache(struct inode *inode,
-			     struct fiemap_extent_info *fieinfo,
-			     __u64 start, __u64 len);
+			     struct fiemap_extent_info *fieinfo);
+extern int ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo);
 extern int ext4_ext_precache(struct inode *inode);
 extern int ext4_collapse_range(struct inode *inode, loff_t offset, loff_t len);
 extern int ext4_insert_range(struct inode *inode, loff_t offset, loff_t len);
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index fb0f99dc8c22..7e399fcb90e3 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -5101,11 +5101,12 @@ static int ext4_xattr_fiemap(struct inode *inode,
 
 static int _ext4_fiemap(struct inode *inode,
 			struct fiemap_extent_info *fieinfo,
-			__u64 start, __u64 len,
 			int (*fill)(struct inode *, ext4_lblk_t,
 				    ext4_lblk_t,
 				    struct fiemap_extent_info *))
 {
+	u64 start = fieinfo->fi_start;
+	u64 len = fieinfo->fi_len;
 	ext4_lblk_t start_blk;
 	u32 ext4_fiemap_flags = FIEMAP_FLAG_SYNC|FIEMAP_FLAG_XATTR;
 
@@ -5131,8 +5132,7 @@ static int _ext4_fiemap(struct inode *inode,
 	/* fallback to generic here if not in extents fmt */
 	if (!(ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)) &&
 	    fill == ext4_fill_fiemap_extents)
-		return generic_block_fiemap(inode, fieinfo, start, len,
-			ext4_get_block);
+		return generic_block_fiemap(inode, fieinfo, ext4_get_block);
 
 	if (fill == ext4_fill_es_cache_info)
 		ext4_fiemap_flags &= FIEMAP_FLAG_XATTR;
@@ -5160,15 +5160,12 @@ static int _ext4_fiemap(struct inode *inode,
 	return error;
 }
 
-int ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-		__u64 start, __u64 len)
+int ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
 {
-	return _ext4_fiemap(inode, fieinfo, start, len,
-			    ext4_fill_fiemap_extents);
+	return _ext4_fiemap(inode, fieinfo, ext4_fill_fiemap_extents);
 }
 
-int ext4_get_es_cache(struct inode *inode, struct fiemap_extent_info *fieinfo,
-		      __u64 start, __u64 len)
+int ext4_get_es_cache(struct inode *inode, struct fiemap_extent_info *fieinfo)
 {
 	if (ext4_has_inline_data(inode)) {
 		int has_inline;
@@ -5180,8 +5177,7 @@ int ext4_get_es_cache(struct inode *inode, struct fiemap_extent_info *fieinfo,
 			return 0;
 	}
 
-	return _ext4_fiemap(inode, fieinfo, start, len,
-			    ext4_fill_es_cache_info);
+	return _ext4_fiemap(inode, fieinfo, ext4_fill_es_cache_info);
 }
 
 
diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index e0a5d8d7c1b4..546a26a2c800 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -795,6 +795,8 @@ static int ext4_ioctl_get_es_cache(struct file *filp, unsigned long arg)
 	fieinfo.fi_flags = fiemap.fm_flags;
 	fieinfo.fi_extents_max = fiemap.fm_extent_count;
 	fieinfo.fi_extents_start = ufiemap->fm_extents;
+	fieinfo.fi_start = fiemap.fm_start;
+	fieinfo.fi_len = len;
 
 	if (fiemap.fm_extent_count != 0 &&
 	    !access_ok(fieinfo.fi_extents_start,
@@ -804,7 +806,7 @@ static int ext4_ioctl_get_es_cache(struct file *filp, unsigned long arg)
 	if (fieinfo.fi_flags & FIEMAP_FLAG_SYNC)
 		filemap_write_and_wait(inode->i_mapping);
 
-	error = ext4_get_es_cache(inode, &fieinfo, fiemap.fm_start, len);
+	error = ext4_get_es_cache(inode, &fieinfo);
 	fiemap.fm_flags = fieinfo.fi_flags;
 	fiemap.fm_mapped_extents = fieinfo.fi_extents_mapped;
 	if (copy_to_user(ufiemap, &fiemap, sizeof(fiemap)))
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 49ff6d2239c3..af2312858069 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1543,9 +1543,10 @@ static int f2fs_xattr_fiemap(struct inode *inode,
 	return (err < 0 ? err : 0);
 }
 
-int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-		u64 start, u64 len)
+int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
 {
+	u64 start = fieinfo->fi_start;
+	u64 len = fieinfo->fi_len;
 	struct buffer_head map_bh;
 	sector_t start_blk, last_blk;
 	pgoff_t next_pgofs;
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 7af36369dd94..42c27e1b5730 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -3217,8 +3217,7 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio);
 void __do_map_lock(struct f2fs_sb_info *sbi, int flag, bool lock);
 int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
 			int create, int flag);
-int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-			u64 start, u64 len);
+int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo);
 bool f2fs_should_update_inplace(struct inode *inode, struct f2fs_io_info *fio);
 bool f2fs_should_update_outplace(struct inode *inode, struct f2fs_io_info *fio);
 void f2fs_invalidate_page(struct page *page, unsigned int offset,
diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index e1e18fb587eb..a02a52cbbec4 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -2026,9 +2026,10 @@ static int gfs2_getattr(const struct path *path, struct kstat *stat,
 	return 0;
 }
 
-static int gfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-		       u64 start, u64 len)
+static int gfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
 {
+	u64 start = fieinfo->fi_start;
+	u64 len = fieinfo->fi_len;
 	struct gfs2_inode *ip = GFS2_I(inode);
 	struct gfs2_holder gh;
 	int ret;
diff --git a/fs/hpfs/file.c b/fs/hpfs/file.c
index 1ecec124e76f..0eece4ae1f11 100644
--- a/fs/hpfs/file.c
+++ b/fs/hpfs/file.c
@@ -190,9 +190,9 @@ static sector_t _hpfs_bmap(struct address_space *mapping, sector_t block)
 	return generic_block_bmap(mapping, block, hpfs_get_block);
 }
 
-static int hpfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo, u64 start, u64 len)
+static int hpfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
 {
-	return generic_block_fiemap(inode, fieinfo, start, len, hpfs_get_block);
+	return generic_block_fiemap(inode, fieinfo, hpfs_get_block);
 }
 
 const struct address_space_operations hpfs_aops = {
diff --git a/fs/ioctl.c b/fs/ioctl.c
index 6b589c873bc2..ad8edcb10dc9 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -210,6 +210,8 @@ static int ioctl_fiemap(struct file *filp, unsigned long arg)
 	fieinfo.fi_flags = fiemap.fm_flags;
 	fieinfo.fi_extents_max = fiemap.fm_extent_count;
 	fieinfo.fi_extents_start = ufiemap->fm_extents;
+	fieinfo.fi_start = fiemap.fm_start;
+	fieinfo.fi_len = len;
 
 	if (fiemap.fm_extent_count != 0 &&
 	    !access_ok(fieinfo.fi_extents_start,
@@ -219,7 +221,7 @@ static int ioctl_fiemap(struct file *filp, unsigned long arg)
 	if (fieinfo.fi_flags & FIEMAP_FLAG_SYNC)
 		filemap_write_and_wait(inode->i_mapping);
 
-	error = inode->i_op->fiemap(inode, &fieinfo, fiemap.fm_start, len);
+	error = inode->i_op->fiemap(inode, &fieinfo);
 	fiemap.fm_flags = fieinfo.fi_flags;
 	fiemap.fm_mapped_extents = fieinfo.fi_extents_mapped;
 	if (copy_to_user(ufiemap, &fiemap, sizeof(fiemap)))
@@ -296,9 +298,11 @@ static inline loff_t blk_to_logical(struct inode *inode, sector_t blk)
  */
 
 int __generic_block_fiemap(struct inode *inode,
-			   struct fiemap_extent_info *fieinfo, loff_t start,
-			   loff_t len, get_block_t *get_block)
+			   struct fiemap_extent_info *fieinfo,
+			   get_block_t *get_block)
 {
+	loff_t start = fieinfo->fi_start;
+	loff_t len = fieinfo->fi_len;
 	struct buffer_head map_bh;
 	sector_t start_blk, last_blk;
 	loff_t isize = i_size_read(inode);
@@ -455,12 +459,12 @@ EXPORT_SYMBOL(__generic_block_fiemap);
  */
 
 int generic_block_fiemap(struct inode *inode,
-			 struct fiemap_extent_info *fieinfo, u64 start,
-			 u64 len, get_block_t *get_block)
+			 struct fiemap_extent_info *fieinfo,
+			 get_block_t *get_block)
 {
 	int ret;
 	inode_lock(inode);
-	ret = __generic_block_fiemap(inode, fieinfo, start, len, get_block);
+	ret = __generic_block_fiemap(inode, fieinfo, get_block);
 	inode_unlock(inode);
 	return ret;
 }
diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
index 671085512e0f..1f37d086371c 100644
--- a/fs/nilfs2/inode.c
+++ b/fs/nilfs2/inode.c
@@ -992,9 +992,10 @@ void nilfs_dirty_inode(struct inode *inode, int flags)
 	nilfs_transaction_commit(inode->i_sb); /* never fails */
 }
 
-int nilfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-		 __u64 start, __u64 len)
+int nilfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
 {
+	u64 start = fieinfo->fi_start;
+	u64 len = fieinfo->fi_len;
 	struct the_nilfs *nilfs = inode->i_sb->s_fs_info;
 	__u64 logical = 0, phys = 0, size = 0;
 	__u32 flags = 0;
diff --git a/fs/nilfs2/nilfs.h b/fs/nilfs2/nilfs.h
index 42395ba52da6..f74b64a76c52 100644
--- a/fs/nilfs2/nilfs.h
+++ b/fs/nilfs2/nilfs.h
@@ -275,8 +275,7 @@ extern int nilfs_inode_dirty(struct inode *);
 int nilfs_set_file_dirty(struct inode *inode, unsigned int nr_dirty);
 extern int __nilfs_mark_inode_dirty(struct inode *, int);
 extern void nilfs_dirty_inode(struct inode *, int flags);
-int nilfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-		 __u64 start, __u64 len);
+int nilfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo);
 static inline int nilfs_mark_inode_dirty(struct inode *inode)
 {
 	return __nilfs_mark_inode_dirty(inode, I_DIRTY);
diff --git a/fs/ocfs2/extent_map.c b/fs/ocfs2/extent_map.c
index e66a249fe07c..1a5b6af62ee0 100644
--- a/fs/ocfs2/extent_map.c
+++ b/fs/ocfs2/extent_map.c
@@ -736,8 +736,7 @@ static int ocfs2_fiemap_inline(struct inode *inode, struct buffer_head *di_bh,
 
 #define OCFS2_FIEMAP_FLAGS	(FIEMAP_FLAG_SYNC)
 
-int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-		 u64 map_start, u64 map_len)
+int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
 {
 	int ret, is_last;
 	u32 mapping_end, cpos;
@@ -746,6 +745,8 @@ int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 	u64 len_bytes, phys_bytes, virt_bytes;
 	struct buffer_head *di_bh = NULL;
 	struct ocfs2_extent_rec rec;
+	u64 map_start = fieinfo->fi_start;
+	u64 map_len = fieinfo->fi_len;
 
 	ret = fiemap_check_flags(fieinfo, OCFS2_FIEMAP_FLAGS);
 	if (ret)
diff --git a/fs/ocfs2/extent_map.h b/fs/ocfs2/extent_map.h
index e5464f6cee8a..be3724b67d7e 100644
--- a/fs/ocfs2/extent_map.h
+++ b/fs/ocfs2/extent_map.h
@@ -37,8 +37,7 @@ int ocfs2_get_clusters(struct inode *inode, u32 v_cluster, u32 *p_cluster,
 int ocfs2_extent_map_get_blocks(struct inode *inode, u64 v_blkno, u64 *p_blkno,
 				u64 *ret_count, unsigned int *extent_flags);
 
-int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-		 u64 map_start, u64 map_len);
+int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo);
 
 int ocfs2_overwrite_io(struct inode *inode, struct buffer_head *di_bh,
 		       u64 map_start, u64 map_len);
diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 7663aeb85fa3..6fd54fb23275 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -453,8 +453,7 @@ int ovl_update_time(struct inode *inode, struct timespec64 *ts, int flags)
 	return 0;
 }
 
-static int ovl_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-		      u64 start, u64 len)
+static int ovl_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
 {
 	int err;
 	struct inode *realinode = ovl_inode_real(inode);
@@ -468,7 +467,7 @@ static int ovl_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 	if (fieinfo->fi_flags & FIEMAP_FLAG_SYNC)
 		filemap_write_and_wait(realinode->i_mapping);
 
-	err = realinode->i_op->fiemap(realinode, fieinfo, start, len);
+	err = realinode->i_op->fiemap(realinode, fieinfo);
 	revert_creds(old_cred);
 
 	return err;
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index fe285d123d69..8d14d733a0e6 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -1100,12 +1100,12 @@ xfs_vn_update_time(
 
 STATIC int
 xfs_vn_fiemap(
-	struct inode		*inode,
-	struct fiemap_extent_info *fieinfo,
-	u64			start,
-	u64			length)
+	struct inode			*inode,
+	struct fiemap_extent_info	*fieinfo)
 {
-	int			error;
+	u64				start = fieinfo->fi_start;
+	u64				length = fieinfo->fi_len;
+	int				error;
 
 	xfs_ilock(XFS_I(inode), XFS_IOLOCK_SHARED);
 	if (fieinfo->fi_flags & FIEMAP_FLAG_XATTR) {
diff --git a/include/linux/fs.h b/include/linux/fs.h
index f1fbd8298ca4..2c0438de4982 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1737,11 +1737,16 @@ extern bool may_open_dev(const struct path *path);
  * VFS FS_IOC_FIEMAP helper definitions.
  */
 struct fiemap_extent_info {
-	unsigned int fi_flags;		/* Flags as passed from user */
-	unsigned int fi_extents_mapped;	/* Number of mapped extents */
-	unsigned int fi_extents_max;	/* Size of fiemap_extent array */
-	struct fiemap_extent __user *fi_extents_start; /* Start of
-							fiemap_extent array */
+	unsigned int	fi_flags;		/* Flags as passed from user */
+	u64		fi_start;		/* Logical offset at which
+						   start mapping */
+	u64		fi_len;			/* Logical length of mapping the
+						   caller cares about */
+	unsigned int	fi_extents_mapped;	/* Number of mapped extents */
+	unsigned int	fi_extents_max;		/* Size of fiemap_extent array */
+	struct		fiemap_extent __user *fi_extents_start;	/* Start of
+								   fiemap_extent
+								   array */
 };
 int fiemap_fill_next_extent(struct fiemap_extent_info *info, u64 logical,
 			    u64 phys, u64 len, u32 flags);
@@ -1873,8 +1878,7 @@ struct inode_operations {
 	int (*setattr) (struct dentry *, struct iattr *);
 	int (*getattr) (const struct path *, struct kstat *, u32, unsigned int);
 	ssize_t (*listxattr) (struct dentry *, char *, size_t);
-	int (*fiemap)(struct inode *, struct fiemap_extent_info *, u64 start,
-		      u64 len);
+	int (*fiemap)(struct inode *, struct fiemap_extent_info *);
 	int (*update_time)(struct inode *, struct timespec64 *, int);
 	int (*atomic_open)(struct inode *, struct dentry *,
 			   struct file *, unsigned open_flag,
@@ -3268,11 +3272,10 @@ extern int vfs_readlink(struct dentry *, char __user *, int);
 
 extern int __generic_block_fiemap(struct inode *inode,
 				  struct fiemap_extent_info *fieinfo,
-				  loff_t start, loff_t len,
 				  get_block_t *get_block);
 extern int generic_block_fiemap(struct inode *inode,
-				struct fiemap_extent_info *fieinfo, u64 start,
-				u64 len, get_block_t *get_block);
+				struct fiemap_extent_info *fieinfo,
+				get_block_t *get_block);
 
 extern struct file_system_type *get_filesystem(struct file_system_type *fs);
 extern void put_filesystem(struct file_system_type *fs);
-- 
2.20.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 6/9] iomap: Remove length and start fields from iomap_fiemap
  2019-09-11 13:43 [PATCH 0/9 V6] New ->fiemap infrastructure and ->bmap removal Carlos Maiolino
                   ` (4 preceding siblings ...)
  2019-09-11 13:43 ` [PATCH 5/9] fs: Move start and length fiemap fields into fiemap_extent_info Carlos Maiolino
@ 2019-09-11 13:43 ` Carlos Maiolino
  2019-09-11 13:43 ` [PATCH 7/9] fiemap: Use a callback to fill fiemap extents Carlos Maiolino
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 24+ messages in thread
From: Carlos Maiolino @ 2019-09-11 13:43 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: hch, adilger, darrick.wong, linux-xfs

fiemap_extent_info now embeds start and length parameters, users of
iomap_fiemap() doesn't need to pass it individually anymore.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---

Changelog:
	V5:
		- Rebased against 5.3
		- Fix small conflict with previous patch
		  due indentation in xfs_vn_fiemap
 fs/gfs2/inode.c       | 4 +---
 fs/iomap/fiemap.c     | 4 +++-
 fs/xfs/xfs_iops.c     | 8 ++------
 include/linux/iomap.h | 2 +-
 4 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index a02a52cbbec4..38e202ed1685 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -2028,8 +2028,6 @@ static int gfs2_getattr(const struct path *path, struct kstat *stat,
 
 static int gfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
 {
-	u64 start = fieinfo->fi_start;
-	u64 len = fieinfo->fi_len;
 	struct gfs2_inode *ip = GFS2_I(inode);
 	struct gfs2_holder gh;
 	int ret;
@@ -2040,7 +2038,7 @@ static int gfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
 	if (ret)
 		goto out;
 
-	ret = iomap_fiemap(inode, fieinfo, start, len, &gfs2_iomap_ops);
+	ret = iomap_fiemap(inode, fieinfo, &gfs2_iomap_ops);
 
 	gfs2_glock_dq_uninit(&gh);
 
diff --git a/fs/iomap/fiemap.c b/fs/iomap/fiemap.c
index f26fdd36e383..03f214f5df94 100644
--- a/fs/iomap/fiemap.c
+++ b/fs/iomap/fiemap.c
@@ -65,9 +65,11 @@ iomap_fiemap_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
 }
 
 int iomap_fiemap(struct inode *inode, struct fiemap_extent_info *fi,
-		loff_t start, loff_t len, const struct iomap_ops *ops)
+		const struct iomap_ops *ops)
 {
 	struct fiemap_ctx ctx;
+	loff_t start = fi->fi_start;
+	loff_t len = fi->fi_len;
 	loff_t ret;
 
 	memset(&ctx, 0, sizeof(ctx));
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index 8d14d733a0e6..6ba59762c17c 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -1103,18 +1103,14 @@ xfs_vn_fiemap(
 	struct inode			*inode,
 	struct fiemap_extent_info	*fieinfo)
 {
-	u64				start = fieinfo->fi_start;
-	u64				length = fieinfo->fi_len;
 	int				error;
 
 	xfs_ilock(XFS_I(inode), XFS_IOLOCK_SHARED);
 	if (fieinfo->fi_flags & FIEMAP_FLAG_XATTR) {
 		fieinfo->fi_flags &= ~FIEMAP_FLAG_XATTR;
-		error = iomap_fiemap(inode, fieinfo, start, length,
-				&xfs_xattr_iomap_ops);
+		error = iomap_fiemap(inode, fieinfo, &xfs_xattr_iomap_ops);
 	} else {
-		error = iomap_fiemap(inode, fieinfo, start, length,
-				&xfs_iomap_ops);
+		error = iomap_fiemap(inode, fieinfo, &xfs_iomap_ops);
 	}
 	xfs_iunlock(XFS_I(inode), XFS_IOLOCK_SHARED);
 
diff --git a/include/linux/iomap.h b/include/linux/iomap.h
index 834d3923e2f2..dfd4b193f1c0 100644
--- a/include/linux/iomap.h
+++ b/include/linux/iomap.h
@@ -177,7 +177,7 @@ int iomap_truncate_page(struct inode *inode, loff_t pos, bool *did_zero,
 vm_fault_t iomap_page_mkwrite(struct vm_fault *vmf,
 			const struct iomap_ops *ops);
 int iomap_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-		loff_t start, loff_t len, const struct iomap_ops *ops);
+		const struct iomap_ops *ops);
 loff_t iomap_seek_hole(struct inode *inode, loff_t offset,
 		const struct iomap_ops *ops);
 loff_t iomap_seek_data(struct inode *inode, loff_t offset,
-- 
2.20.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 7/9] fiemap: Use a callback to fill fiemap extents
  2019-09-11 13:43 [PATCH 0/9 V6] New ->fiemap infrastructure and ->bmap removal Carlos Maiolino
                   ` (5 preceding siblings ...)
  2019-09-11 13:43 ` [PATCH 6/9] iomap: Remove length and start fields from iomap_fiemap Carlos Maiolino
@ 2019-09-11 13:43 ` Carlos Maiolino
  2019-09-11 13:43 ` [PATCH 8/9] Use FIEMAP for FIBMAP calls Carlos Maiolino
  2019-09-11 13:43 ` [PATCH 9/9] xfs: Get rid of ->bmap Carlos Maiolino
  8 siblings, 0 replies; 24+ messages in thread
From: Carlos Maiolino @ 2019-09-11 13:43 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: hch, adilger, darrick.wong, linux-xfs

As a goal to enable fiemap infrastructure to be used by fibmap too, we need a
way to use different helpers to fill extent data, depending on its usage. One
helper to fill extent data stored in user address space (used in fiemap), and
another fo fill extent data stored in kernel address space (will be used in
fibmap).

This patch sets up the usage of a callback to be used to fill in the extents.
It transforms the current fiemap_fill_next_extent, into a simple helper to call
the callback, avoiding unneeded changes on any filesystem, and reutilizes the
original function as the callback used by FIEMAP.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---

Changelog:
	V6:
		- Rebased against next/master
		- Fix conflict on ext4 code
	V5:
		- Rebased against 5.3 with no changes
	V3:
		- Rebase to current linux-next
		- Fix conflict on rebase
		- Merge this patch into patch 07 from V2
		- Rename fi_extents_start to fi_cb_data
	V2:
		- Now based on the rework on fiemap_extent_info (previous was
		  based on fiemap_ctx)

 fs/ext4/ioctl.c    |  4 ++--
 fs/ioctl.c         | 45 ++++++++++++++++++++++++++-------------------
 include/linux/fs.h | 12 +++++++++---
 3 files changed, 37 insertions(+), 24 deletions(-)

diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index 546a26a2c800..1dd267e7d450 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -794,12 +794,12 @@ static int ext4_ioctl_get_es_cache(struct file *filp, unsigned long arg)
 
 	fieinfo.fi_flags = fiemap.fm_flags;
 	fieinfo.fi_extents_max = fiemap.fm_extent_count;
-	fieinfo.fi_extents_start = ufiemap->fm_extents;
+	fieinfo.fi_cb_data = ufiemap->fm_extents;
 	fieinfo.fi_start = fiemap.fm_start;
 	fieinfo.fi_len = len;
 
 	if (fiemap.fm_extent_count != 0 &&
-	    !access_ok(fieinfo.fi_extents_start,
+	    !access_ok(fieinfo.fi_cb_data,
 		       fieinfo.fi_extents_max * sizeof(struct fiemap_extent)))
 		return -EFAULT;
 
diff --git a/fs/ioctl.c b/fs/ioctl.c
index ad8edcb10dc9..d72696c222de 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -77,29 +77,14 @@ static int ioctl_fibmap(struct file *filp, int __user *p)
 	return error;
 }
 
-/**
- * fiemap_fill_next_extent - Fiemap helper function
- * @fieinfo:	Fiemap context passed into ->fiemap
- * @logical:	Extent logical start offset, in bytes
- * @phys:	Extent physical start offset, in bytes
- * @len:	Extent length, in bytes
- * @flags:	FIEMAP_EXTENT flags that describe this extent
- *
- * Called from file system ->fiemap callback. Will populate extent
- * info as passed in via arguments and copy to user memory. On
- * success, extent count on fieinfo is incremented.
- *
- * Returns 0 on success, -errno on error, 1 if this was the last
- * extent that will fit in user array.
- */
 #define SET_UNKNOWN_FLAGS	(FIEMAP_EXTENT_DELALLOC)
 #define SET_NO_UNMOUNTED_IO_FLAGS	(FIEMAP_EXTENT_DATA_ENCRYPTED)
 #define SET_NOT_ALIGNED_FLAGS	(FIEMAP_EXTENT_DATA_TAIL|FIEMAP_EXTENT_DATA_INLINE)
-int fiemap_fill_next_extent(struct fiemap_extent_info *fieinfo, u64 logical,
+int fiemap_fill_user_extent(struct fiemap_extent_info *fieinfo, u64 logical,
 			    u64 phys, u64 len, u32 flags)
 {
 	struct fiemap_extent extent;
-	struct fiemap_extent __user *dest = fieinfo->fi_extents_start;
+	struct fiemap_extent __user *dest = fieinfo->fi_cb_data;
 
 	/* only count the extents */
 	if (fieinfo->fi_extents_max == 0) {
@@ -132,6 +117,27 @@ int fiemap_fill_next_extent(struct fiemap_extent_info *fieinfo, u64 logical,
 		return 1;
 	return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0;
 }
+
+/**
+ * fiemap_fill_next_extent - Fiemap helper function
+ * @fieinfo:	Fiemap context passed into ->fiemap
+ * @logical:	Extent logical start offset, in bytes
+ * @phys:	Extent physical start offset, in bytes
+ * @len:	Extent length, in bytes
+ * @flags:	FIEMAP_EXTENT flags that describe this extent
+ *
+ * Called from file system ->fiemap callback. Will populate extent
+ * info as passed in via arguments and copy to user memory. On
+ * success, extent count on fieinfo is incremented.
+ *
+ * Returns 0 on success, -errno on error, 1 if this was the last
+ * extent that will fit in user array.
+ */
+int fiemap_fill_next_extent(struct fiemap_extent_info *fieinfo, u64 logical,
+			    u64 phys, u64 len, u32 flags)
+{
+	return fieinfo->fi_cb(fieinfo, logical, phys, len, flags);
+}
 EXPORT_SYMBOL(fiemap_fill_next_extent);
 
 /**
@@ -209,12 +215,13 @@ static int ioctl_fiemap(struct file *filp, unsigned long arg)
 
 	fieinfo.fi_flags = fiemap.fm_flags;
 	fieinfo.fi_extents_max = fiemap.fm_extent_count;
-	fieinfo.fi_extents_start = ufiemap->fm_extents;
+	fieinfo.fi_cb_data = ufiemap->fm_extents;
 	fieinfo.fi_start = fiemap.fm_start;
 	fieinfo.fi_len = len;
+	fieinfo.fi_cb = fiemap_fill_user_extent;
 
 	if (fiemap.fm_extent_count != 0 &&
-	    !access_ok(fieinfo.fi_extents_start,
+	    !access_ok(fieinfo.fi_cb_data,
 		       fieinfo.fi_extents_max * sizeof(struct fiemap_extent)))
 		return -EFAULT;
 
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 2c0438de4982..df33997cc209 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -68,6 +68,7 @@ struct fsverity_info;
 struct fsverity_operations;
 struct fs_context;
 struct fs_parameter_description;
+struct fiemap_extent_info;
 
 extern void __init inode_init(void);
 extern void __init inode_init_early(void);
@@ -1736,6 +1737,10 @@ extern bool may_open_dev(const struct path *path);
 /*
  * VFS FS_IOC_FIEMAP helper definitions.
  */
+
+typedef int (*fiemap_fill_cb)(struct fiemap_extent_info *fieinfo, u64 logical,
+			      u64 phys, u64 len, u32 flags);
+
 struct fiemap_extent_info {
 	unsigned int	fi_flags;		/* Flags as passed from user */
 	u64		fi_start;		/* Logical offset at which
@@ -1744,10 +1749,11 @@ struct fiemap_extent_info {
 						   caller cares about */
 	unsigned int	fi_extents_mapped;	/* Number of mapped extents */
 	unsigned int	fi_extents_max;		/* Size of fiemap_extent array */
-	struct		fiemap_extent __user *fi_extents_start;	/* Start of
-								   fiemap_extent
-								   array */
+	void		*fi_cb_data;		/* Start of fiemap_extent
+						   array */
+	fiemap_fill_cb	fi_cb;
 };
+
 int fiemap_fill_next_extent(struct fiemap_extent_info *info, u64 logical,
 			    u64 phys, u64 len, u32 flags);
 int fiemap_check_flags(struct fiemap_extent_info *fieinfo, u32 fs_flags);
-- 
2.20.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 8/9] Use FIEMAP for FIBMAP calls
  2019-09-11 13:43 [PATCH 0/9 V6] New ->fiemap infrastructure and ->bmap removal Carlos Maiolino
                   ` (6 preceding siblings ...)
  2019-09-11 13:43 ` [PATCH 7/9] fiemap: Use a callback to fill fiemap extents Carlos Maiolino
@ 2019-09-11 13:43 ` Carlos Maiolino
  2019-09-16 17:44   ` Darrick J. Wong
  2019-09-11 13:43 ` [PATCH 9/9] xfs: Get rid of ->bmap Carlos Maiolino
  8 siblings, 1 reply; 24+ messages in thread
From: Carlos Maiolino @ 2019-09-11 13:43 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: hch, adilger, darrick.wong, linux-xfs

Enables the usage of FIEMAP ioctl infrastructure to handle FIBMAP calls.
From now on, ->bmap() methods can start to be removed from filesystems
which already provides ->fiemap().

This adds a new helper - bmap_fiemap() - which is used to fill in the
fiemap request, call into the underlying filesystem, check the flags
set in the extent requested.

Add a new fiemap fill extent callback to handle the in-kernel only
fiemap_extent structure used for FIBMAP.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---

With the removal of FIEMAP_KERNEL_FIBMAP flag in this version, we hide
from the filesystem whether the call is coming from FIEMAP, or FIBMAP.

In this way, filesystems that previously did not support FIBMAP (but
supports FIEMAP), may start to support it. It *may* be an issue for
filesystems like btrfs, where we can't say for sure on which device the
requested block resides in. But well, this is also true for FIEMAP
requests.
At a first glance, I didn't see any harm in having FIBMAP enabled in
btrfs, my first worry was about bootloaders trying to use it, but I
don't think it will change much in practice.

Changelog:

	V6:
		- Fix sparse warning on fiemal_fill_kernel_extent:
			Reported-by: kbuild test robot <lkp@intel.com>
		- Fix conflict in ext4
		- Remove the FIEMAP_KERNEL_FIBMAP flag and let the
		  caller decide whether to reply user's FIBMAP call
		  based on the FIEMAP flags returned by the filesystem,
		  suggested by Christoph.

	V5:
		- Properly rebase against 5.3
		- Fix xfs coding style
		- Use xfs_is_cow_inode() check in xfs_vn_fiemap.
			- It needs xfs_reflink.h, but maybe it's better to move
			  static calls into xfs_inode.h
		- Fix small conflict due indentation update in xfs_vn_fiemap
	V4:
		- Fix if conditional in bmap()
		- Add filesystem-specific modifications
	V3:
		- Add FIEMAP_EXTENT_SHARED to the list of invalid extents in
		  bmap_fiemap()
		- Rename fi_extents_start to fi_cb_data
		- Use if conditional instead of ternary operator
		- Make fiemap_fill_* callbacks static (which required the
		  removal of some macros
		- Set FIEMAP_FLAG_SYNC when calling in ->fiemap method from
		  fibmap
		- Add FIEMAP_KERNEL_FIBMAP flag, to identify the usage of fiemap
		  infrastructure for fibmap calls, defined in fs.h so it's not
		  exported to userspace.
		- Update fiemap_check_flags() to understand FIEMAP_KERNEL_FIBMAP
		- Update filesystems supporting both FIBMAP and FIEMAP, which
		  need extra checks on FIBMAP calls

	V2:
		- Now based on the updated fiemap_extent_info,
		- move the fiemap call itself to a new helper

 fs/inode.c            | 84 +++++++++++++++++++++++++++++++++++++++++--
 fs/ioctl.c            | 21 +++++------
 fs/ocfs2/extent_map.c |  1 +
 3 files changed, 94 insertions(+), 12 deletions(-)

diff --git a/fs/inode.c b/fs/inode.c
index 0a20aaa572f2..ab33d16bc29d 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1591,6 +1591,81 @@ void iput(struct inode *inode)
 }
 EXPORT_SYMBOL(iput);
 
+static int fiemap_fill_kernel_extent(struct fiemap_extent_info *fieinfo,
+			u64 logical, u64 phys, u64 len, u32 flags)
+{
+	struct fiemap_extent *extent = fieinfo->fi_cb_data;
+
+	/* only count the extents */
+	if (!fieinfo->fi_cb_data) {
+		fieinfo->fi_extents_mapped++;
+		goto out;
+	}
+
+	if (fieinfo->fi_extents_mapped >= fieinfo->fi_extents_max)
+		return 1;
+
+	if (flags & FIEMAP_EXTENT_DELALLOC)
+		flags |= FIEMAP_EXTENT_UNKNOWN;
+	if (flags & FIEMAP_EXTENT_DATA_ENCRYPTED)
+		flags |= FIEMAP_EXTENT_ENCODED;
+	if (flags & (FIEMAP_EXTENT_DATA_TAIL | FIEMAP_EXTENT_DATA_INLINE))
+		flags |= FIEMAP_EXTENT_NOT_ALIGNED;
+
+	extent->fe_logical = logical;
+	extent->fe_physical = phys;
+	extent->fe_length = len;
+	extent->fe_flags = flags;
+
+	fieinfo->fi_extents_mapped++;
+
+	if (fieinfo->fi_extents_mapped == fieinfo->fi_extents_max)
+		return 1;
+
+out:
+	if (flags & FIEMAP_EXTENT_LAST)
+		return 1;
+	return 0;
+}
+
+#define FIBMAP_INCOMPAT_FLAGS \
+	(FIEMAP_EXTENT_UNKNOWN | FIEMAP_EXTENT_DELALLOC | \
+	 FIEMAP_EXTENT_ENCODED | FIEMAP_EXTENT_DATA_ENCRYPTED | \
+	 FIEMAP_EXTENT_NOT_ALIGNED | FIEMAP_EXTENT_DATA_INLINE | \
+	 FIEMAP_EXTENT_DATA_TAIL | FIEMAP_EXTENT_UNWRITTEN | \
+	 FIEMAP_EXTENT_SHARED)
+
+static int bmap_fiemap(struct inode *inode, sector_t *block)
+{
+	struct fiemap_extent_info fieinfo = { 0, };
+	struct fiemap_extent fextent;
+	u64 start = *block << inode->i_blkbits;
+	int error = -EINVAL;
+
+	fextent.fe_logical = 0;
+	fextent.fe_physical = 0;
+	fieinfo.fi_extents_max = 1;
+	fieinfo.fi_extents_mapped = 0;
+	fieinfo.fi_cb_data = &fextent;
+	fieinfo.fi_start = start;
+	fieinfo.fi_len = 1 << inode->i_blkbits;
+	fieinfo.fi_cb = fiemap_fill_kernel_extent;
+	fieinfo.fi_flags = FIEMAP_FLAG_SYNC;
+
+	error = inode->i_op->fiemap(inode, &fieinfo);
+
+	if (error)
+		return error;
+
+	if (fextent.fe_flags & FIBMAP_INCOMPAT_FLAGS)
+		return -EINVAL;
+
+	*block = (fextent.fe_physical +
+		  (start - fextent.fe_logical)) >> inode->i_blkbits;
+
+	return error;
+}
+
 /**
  *	bmap	- find a block number in a file
  *	@inode:  inode owning the block number being requested
@@ -1607,10 +1682,15 @@ EXPORT_SYMBOL(iput);
  */
 int bmap(struct inode *inode, sector_t *block)
 {
-	if (!inode->i_mapping->a_ops->bmap)
+	if (inode->i_op->fiemap)
+		return bmap_fiemap(inode, block);
+
+	if (inode->i_mapping->a_ops->bmap)
+		*block = inode->i_mapping->a_ops->bmap(inode->i_mapping,
+						       *block);
+	else
 		return -EINVAL;
 
-	*block = inode->i_mapping->a_ops->bmap(inode->i_mapping, *block);
 	return 0;
 }
 EXPORT_SYMBOL(bmap);
diff --git a/fs/ioctl.c b/fs/ioctl.c
index d72696c222de..fbfd631ba3cb 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -77,11 +77,8 @@ static int ioctl_fibmap(struct file *filp, int __user *p)
 	return error;
 }
 
-#define SET_UNKNOWN_FLAGS	(FIEMAP_EXTENT_DELALLOC)
-#define SET_NO_UNMOUNTED_IO_FLAGS	(FIEMAP_EXTENT_DATA_ENCRYPTED)
-#define SET_NOT_ALIGNED_FLAGS	(FIEMAP_EXTENT_DATA_TAIL|FIEMAP_EXTENT_DATA_INLINE)
-int fiemap_fill_user_extent(struct fiemap_extent_info *fieinfo, u64 logical,
-			    u64 phys, u64 len, u32 flags)
+static int fiemap_fill_user_extent(struct fiemap_extent_info *fieinfo,
+			u64 logical, u64 phys, u64 len, u32 flags)
 {
 	struct fiemap_extent extent;
 	struct fiemap_extent __user *dest = fieinfo->fi_cb_data;
@@ -89,17 +86,17 @@ int fiemap_fill_user_extent(struct fiemap_extent_info *fieinfo, u64 logical,
 	/* only count the extents */
 	if (fieinfo->fi_extents_max == 0) {
 		fieinfo->fi_extents_mapped++;
-		return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0;
+		goto out;
 	}
 
 	if (fieinfo->fi_extents_mapped >= fieinfo->fi_extents_max)
 		return 1;
 
-	if (flags & SET_UNKNOWN_FLAGS)
+	if (flags & FIEMAP_EXTENT_DELALLOC)
 		flags |= FIEMAP_EXTENT_UNKNOWN;
-	if (flags & SET_NO_UNMOUNTED_IO_FLAGS)
+	if (flags & FIEMAP_EXTENT_DATA_ENCRYPTED)
 		flags |= FIEMAP_EXTENT_ENCODED;
-	if (flags & SET_NOT_ALIGNED_FLAGS)
+	if (flags & (FIEMAP_EXTENT_DATA_TAIL | FIEMAP_EXTENT_DATA_INLINE))
 		flags |= FIEMAP_EXTENT_NOT_ALIGNED;
 
 	memset(&extent, 0, sizeof(extent));
@@ -115,7 +112,11 @@ int fiemap_fill_user_extent(struct fiemap_extent_info *fieinfo, u64 logical,
 	fieinfo->fi_extents_mapped++;
 	if (fieinfo->fi_extents_mapped == fieinfo->fi_extents_max)
 		return 1;
-	return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0;
+
+out:
+	if (flags & FIEMAP_EXTENT_LAST)
+		return 1;
+	return 0;
 }
 
 /**
diff --git a/fs/ocfs2/extent_map.c b/fs/ocfs2/extent_map.c
index 1a5b6af62ee0..0dd838615d14 100644
--- a/fs/ocfs2/extent_map.c
+++ b/fs/ocfs2/extent_map.c
@@ -743,6 +743,7 @@ int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
 	unsigned int hole_size;
 	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
 	u64 len_bytes, phys_bytes, virt_bytes;
+
 	struct buffer_head *di_bh = NULL;
 	struct ocfs2_extent_rec rec;
 	u64 map_start = fieinfo->fi_start;
-- 
2.20.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 9/9] xfs: Get rid of ->bmap
  2019-09-11 13:43 [PATCH 0/9 V6] New ->fiemap infrastructure and ->bmap removal Carlos Maiolino
                   ` (7 preceding siblings ...)
  2019-09-11 13:43 ` [PATCH 8/9] Use FIEMAP for FIBMAP calls Carlos Maiolino
@ 2019-09-11 13:43 ` Carlos Maiolino
  2019-09-16 17:50   ` Darrick J. Wong
  8 siblings, 1 reply; 24+ messages in thread
From: Carlos Maiolino @ 2019-09-11 13:43 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: hch, adilger, darrick.wong, linux-xfs

We don't need ->bmap anymore, only usage for it was FIBMAP, which is now
gone.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---

Changelog:
	V5:
		- Properly rebase against 5.3
		- iomap_{bmap(),bmap_actor()} are now used also by GFS2, so
		  don't remove them anymore
	V2:
		- Kill iomap_bmap() and iomap_bmap_actor()

 fs/xfs/xfs_aops.c  | 24 ------------------------
 fs/xfs/xfs_trace.h |  1 -
 2 files changed, 25 deletions(-)

diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
index 4e4a4d7df5ac..a2884537d2c2 100644
--- a/fs/xfs/xfs_aops.c
+++ b/fs/xfs/xfs_aops.c
@@ -1138,29 +1138,6 @@ xfs_vm_releasepage(
 	return iomap_releasepage(page, gfp_mask);
 }
 
-STATIC sector_t
-xfs_vm_bmap(
-	struct address_space	*mapping,
-	sector_t		block)
-{
-	struct xfs_inode	*ip = XFS_I(mapping->host);
-
-	trace_xfs_vm_bmap(ip);
-
-	/*
-	 * The swap code (ab-)uses ->bmap to get a block mapping and then
-	 * bypasses the file system for actual I/O.  We really can't allow
-	 * that on reflinks inodes, so we have to skip out here.  And yes,
-	 * 0 is the magic code for a bmap error.
-	 *
-	 * Since we don't pass back blockdev info, we can't return bmap
-	 * information for rt files either.
-	 */
-	if (xfs_is_cow_inode(ip) || XFS_IS_REALTIME_INODE(ip))
-		return 0;
-	return iomap_bmap(mapping, block, &xfs_iomap_ops);
-}
-
 STATIC int
 xfs_vm_readpage(
 	struct file		*unused,
@@ -1199,7 +1176,6 @@ const struct address_space_operations xfs_address_space_operations = {
 	.set_page_dirty		= iomap_set_page_dirty,
 	.releasepage		= xfs_vm_releasepage,
 	.invalidatepage		= xfs_vm_invalidatepage,
-	.bmap			= xfs_vm_bmap,
 	.direct_IO		= noop_direct_IO,
 	.migratepage		= iomap_migrate_page,
 	.is_partially_uptodate  = iomap_is_partially_uptodate,
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index eaae275ed430..c226b562f5da 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -626,7 +626,6 @@ DEFINE_INODE_EVENT(xfs_readdir);
 #ifdef CONFIG_XFS_POSIX_ACL
 DEFINE_INODE_EVENT(xfs_get_acl);
 #endif
-DEFINE_INODE_EVENT(xfs_vm_bmap);
 DEFINE_INODE_EVENT(xfs_file_ioctl);
 DEFINE_INODE_EVENT(xfs_file_compat_ioctl);
 DEFINE_INODE_EVENT(xfs_ioctl_setattr);
-- 
2.20.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 5/9] fs: Move start and length fiemap fields into fiemap_extent_info
  2019-09-11 13:43 ` [PATCH 5/9] fs: Move start and length fiemap fields into fiemap_extent_info Carlos Maiolino
@ 2019-09-16 17:42   ` Darrick J. Wong
  0 siblings, 0 replies; 24+ messages in thread
From: Darrick J. Wong @ 2019-09-16 17:42 UTC (permalink / raw)
  To: Carlos Maiolino; +Cc: linux-fsdevel, hch, adilger, linux-xfs

On Wed, Sep 11, 2019 at 03:43:11PM +0200, Carlos Maiolino wrote:
> As the overall goal to deprecate fibmap, Christoph suggested a rework of
> the ->fiemap API, in a way we could pass to it a callback to fill the
> fiemap structure (one of these callbacks being fiemap_fill_next_extent).
> 
> To avoid the need to add several fields into the ->fiemap method, aggregate
> everything into a single data structure, and pass it along.
> 
> This patch isn't suppose to add any functional change, only to update
> filesystems providing ->fiemap() method.
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---
> Changelog:
> 
> 	V6:
> 		- Update cifs_fiemap to comply to the new ->fiemap()
> 		  interface
> 			Reported-by: kbuild test robot <lkp@intel.com>
> 		- Fix ext4 conflict with the new EXT4_IOC_GET_ES_CACHE
> 		  ioctl
> 	V5:
> 		- Add comments to fi_start and fi_len fields of
> 		  fiemap_extent_info
> 		- Fix xfs_vn_fiemap indentation
> 
>  fs/bad_inode.c        |  3 +--
>  fs/btrfs/inode.c      |  5 +++--
>  fs/cifs/cifsfs.h      |  3 +--
>  fs/cifs/inode.c       |  5 +++--
>  fs/ext2/ext2.h        |  3 +--
>  fs/ext2/inode.c       |  6 ++----
>  fs/ext4/ext4.h        |  6 ++----
>  fs/ext4/extents.c     | 18 +++++++-----------
>  fs/ext4/ioctl.c       |  4 +++-
>  fs/f2fs/data.c        |  5 +++--
>  fs/f2fs/f2fs.h        |  3 +--
>  fs/gfs2/inode.c       |  5 +++--
>  fs/hpfs/file.c        |  4 ++--
>  fs/ioctl.c            | 16 ++++++++++------
>  fs/nilfs2/inode.c     |  5 +++--
>  fs/nilfs2/nilfs.h     |  3 +--
>  fs/ocfs2/extent_map.c |  5 +++--
>  fs/ocfs2/extent_map.h |  3 +--
>  fs/overlayfs/inode.c  |  5 ++---
>  fs/xfs/xfs_iops.c     | 10 +++++-----
>  include/linux/fs.h    | 23 +++++++++++++----------
>  21 files changed, 70 insertions(+), 70 deletions(-)
> 
> diff --git a/fs/bad_inode.c b/fs/bad_inode.c
> index 8035d2a44561..21dfaf876814 100644
> --- a/fs/bad_inode.c
> +++ b/fs/bad_inode.c
> @@ -120,8 +120,7 @@ static struct posix_acl *bad_inode_get_acl(struct inode *inode, int type)
>  }
>  
>  static int bad_inode_fiemap(struct inode *inode,
> -			    struct fiemap_extent_info *fieinfo, u64 start,
> -			    u64 len)
> +			    struct fiemap_extent_info *fieinfo)
>  {
>  	return -EIO;
>  }
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index ef9310553a73..3d42f07c31b0 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -8762,9 +8762,10 @@ static ssize_t btrfs_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
>  
>  #define BTRFS_FIEMAP_FLAGS	(FIEMAP_FLAG_SYNC)
>  
> -static int btrfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> -		__u64 start, __u64 len)
> +static int btrfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
>  {
> +	u64	start = fieinfo->fi_start;
> +	u64	len = fieinfo->fi_len;
>  	int	ret;
>  
>  	ret = fiemap_check_flags(fieinfo, BTRFS_FIEMAP_FLAGS);
> diff --git a/fs/cifs/cifsfs.h b/fs/cifs/cifsfs.h
> index 99caf77df4a2..ca4c9443cd4e 100644
> --- a/fs/cifs/cifsfs.h
> +++ b/fs/cifs/cifsfs.h
> @@ -84,8 +84,7 @@ extern int cifs_revalidate_mapping(struct inode *inode);
>  extern int cifs_zap_mapping(struct inode *inode);
>  extern int cifs_getattr(const struct path *, struct kstat *, u32, unsigned int);
>  extern int cifs_setattr(struct dentry *, struct iattr *);
> -extern int cifs_fiemap(struct inode *, struct fiemap_extent_info *, u64 start,
> -		       u64 len);
> +extern int cifs_fiemap(struct inode *, struct fiemap_extent_info *);
>  
>  extern const struct inode_operations cifs_file_inode_ops;
>  extern const struct inode_operations cifs_symlink_inode_ops;
> diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
> index 26cdfbf1e164..ad5731c5a7d3 100644
> --- a/fs/cifs/inode.c
> +++ b/fs/cifs/inode.c
> @@ -2123,14 +2123,15 @@ int cifs_getattr(const struct path *path, struct kstat *stat,
>  	return rc;
>  }
>  
> -int cifs_fiemap(struct inode *inode, struct fiemap_extent_info *fei, u64 start,
> -		u64 len)
> +int cifs_fiemap(struct inode *inode, struct fiemap_extent_info *fei)
>  {
>  	struct cifsInodeInfo *cifs_i = CIFS_I(inode);
>  	struct cifs_sb_info *cifs_sb = CIFS_SB(cifs_i->vfs_inode.i_sb);
>  	struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb);
>  	struct TCP_Server_Info *server = tcon->ses->server;
>  	struct cifsFileInfo *cfile;
> +	u64 start = fei->fi_start;
> +	u64 len = fei->fi_len;
>  	int rc;
>  
>  	/*
> diff --git a/fs/ext2/ext2.h b/fs/ext2/ext2.h
> index 10ab238de9a6..284df1af9474 100644
> --- a/fs/ext2/ext2.h
> +++ b/fs/ext2/ext2.h
> @@ -760,8 +760,7 @@ extern int ext2_get_block(struct inode *, sector_t, struct buffer_head *, int);
>  extern int ext2_setattr (struct dentry *, struct iattr *);
>  extern int ext2_getattr (const struct path *, struct kstat *, u32, unsigned int);
>  extern void ext2_set_inode_flags(struct inode *inode);
> -extern int ext2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> -		       u64 start, u64 len);
> +extern int ext2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo);
>  
>  /* ioctl.c */
>  extern long ext2_ioctl(struct file *, unsigned int, unsigned long);
> diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
> index 7004ce581a32..6956df98dd53 100644
> --- a/fs/ext2/inode.c
> +++ b/fs/ext2/inode.c
> @@ -857,11 +857,9 @@ const struct iomap_ops ext2_iomap_ops = {
>  const struct iomap_ops ext2_iomap_ops;
>  #endif /* CONFIG_FS_DAX */
>  
> -int ext2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> -		u64 start, u64 len)
> +int ext2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
>  {
> -	return generic_block_fiemap(inode, fieinfo, start, len,
> -				    ext2_get_block);
> +	return generic_block_fiemap(inode, fieinfo, ext2_get_block);
>  }
>  
>  static int ext2_writepage(struct page *page, struct writeback_control *wbc)
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 99a10f3ec440..be0aa671d701 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -3285,11 +3285,9 @@ extern struct ext4_ext_path *ext4_find_extent(struct inode *, ext4_lblk_t,
>  extern void ext4_ext_drop_refs(struct ext4_ext_path *);
>  extern int ext4_ext_check_inode(struct inode *inode);
>  extern ext4_lblk_t ext4_ext_next_allocated_block(struct ext4_ext_path *path);
> -extern int ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> -			__u64 start, __u64 len);
>  extern int ext4_get_es_cache(struct inode *inode,
> -			     struct fiemap_extent_info *fieinfo,
> -			     __u64 start, __u64 len);
> +			     struct fiemap_extent_info *fieinfo);
> +extern int ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo);
>  extern int ext4_ext_precache(struct inode *inode);
>  extern int ext4_collapse_range(struct inode *inode, loff_t offset, loff_t len);
>  extern int ext4_insert_range(struct inode *inode, loff_t offset, loff_t len);
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index fb0f99dc8c22..7e399fcb90e3 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -5101,11 +5101,12 @@ static int ext4_xattr_fiemap(struct inode *inode,
>  
>  static int _ext4_fiemap(struct inode *inode,
>  			struct fiemap_extent_info *fieinfo,
> -			__u64 start, __u64 len,
>  			int (*fill)(struct inode *, ext4_lblk_t,
>  				    ext4_lblk_t,
>  				    struct fiemap_extent_info *))
>  {
> +	u64 start = fieinfo->fi_start;
> +	u64 len = fieinfo->fi_len;
>  	ext4_lblk_t start_blk;
>  	u32 ext4_fiemap_flags = FIEMAP_FLAG_SYNC|FIEMAP_FLAG_XATTR;
>  
> @@ -5131,8 +5132,7 @@ static int _ext4_fiemap(struct inode *inode,
>  	/* fallback to generic here if not in extents fmt */
>  	if (!(ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)) &&
>  	    fill == ext4_fill_fiemap_extents)
> -		return generic_block_fiemap(inode, fieinfo, start, len,
> -			ext4_get_block);
> +		return generic_block_fiemap(inode, fieinfo, ext4_get_block);
>  
>  	if (fill == ext4_fill_es_cache_info)
>  		ext4_fiemap_flags &= FIEMAP_FLAG_XATTR;
> @@ -5160,15 +5160,12 @@ static int _ext4_fiemap(struct inode *inode,
>  	return error;
>  }
>  
> -int ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> -		__u64 start, __u64 len)
> +int ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
>  {
> -	return _ext4_fiemap(inode, fieinfo, start, len,
> -			    ext4_fill_fiemap_extents);
> +	return _ext4_fiemap(inode, fieinfo, ext4_fill_fiemap_extents);
>  }
>  
> -int ext4_get_es_cache(struct inode *inode, struct fiemap_extent_info *fieinfo,
> -		      __u64 start, __u64 len)
> +int ext4_get_es_cache(struct inode *inode, struct fiemap_extent_info *fieinfo)
>  {
>  	if (ext4_has_inline_data(inode)) {
>  		int has_inline;
> @@ -5180,8 +5177,7 @@ int ext4_get_es_cache(struct inode *inode, struct fiemap_extent_info *fieinfo,
>  			return 0;
>  	}
>  
> -	return _ext4_fiemap(inode, fieinfo, start, len,
> -			    ext4_fill_es_cache_info);
> +	return _ext4_fiemap(inode, fieinfo, ext4_fill_es_cache_info);
>  }
>  
>  
> diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
> index e0a5d8d7c1b4..546a26a2c800 100644
> --- a/fs/ext4/ioctl.c
> +++ b/fs/ext4/ioctl.c
> @@ -795,6 +795,8 @@ static int ext4_ioctl_get_es_cache(struct file *filp, unsigned long arg)
>  	fieinfo.fi_flags = fiemap.fm_flags;
>  	fieinfo.fi_extents_max = fiemap.fm_extent_count;
>  	fieinfo.fi_extents_start = ufiemap->fm_extents;
> +	fieinfo.fi_start = fiemap.fm_start;
> +	fieinfo.fi_len = len;
>  
>  	if (fiemap.fm_extent_count != 0 &&
>  	    !access_ok(fieinfo.fi_extents_start,
> @@ -804,7 +806,7 @@ static int ext4_ioctl_get_es_cache(struct file *filp, unsigned long arg)
>  	if (fieinfo.fi_flags & FIEMAP_FLAG_SYNC)
>  		filemap_write_and_wait(inode->i_mapping);
>  
> -	error = ext4_get_es_cache(inode, &fieinfo, fiemap.fm_start, len);
> +	error = ext4_get_es_cache(inode, &fieinfo);
>  	fiemap.fm_flags = fieinfo.fi_flags;
>  	fiemap.fm_mapped_extents = fieinfo.fi_extents_mapped;
>  	if (copy_to_user(ufiemap, &fiemap, sizeof(fiemap)))
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 49ff6d2239c3..af2312858069 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1543,9 +1543,10 @@ static int f2fs_xattr_fiemap(struct inode *inode,
>  	return (err < 0 ? err : 0);
>  }
>  
> -int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> -		u64 start, u64 len)
> +int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
>  {
> +	u64 start = fieinfo->fi_start;
> +	u64 len = fieinfo->fi_len;
>  	struct buffer_head map_bh;
>  	sector_t start_blk, last_blk;
>  	pgoff_t next_pgofs;
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 7af36369dd94..42c27e1b5730 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -3217,8 +3217,7 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio);
>  void __do_map_lock(struct f2fs_sb_info *sbi, int flag, bool lock);
>  int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
>  			int create, int flag);
> -int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> -			u64 start, u64 len);
> +int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo);
>  bool f2fs_should_update_inplace(struct inode *inode, struct f2fs_io_info *fio);
>  bool f2fs_should_update_outplace(struct inode *inode, struct f2fs_io_info *fio);
>  void f2fs_invalidate_page(struct page *page, unsigned int offset,
> diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
> index e1e18fb587eb..a02a52cbbec4 100644
> --- a/fs/gfs2/inode.c
> +++ b/fs/gfs2/inode.c
> @@ -2026,9 +2026,10 @@ static int gfs2_getattr(const struct path *path, struct kstat *stat,
>  	return 0;
>  }
>  
> -static int gfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> -		       u64 start, u64 len)
> +static int gfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
>  {
> +	u64 start = fieinfo->fi_start;
> +	u64 len = fieinfo->fi_len;
>  	struct gfs2_inode *ip = GFS2_I(inode);
>  	struct gfs2_holder gh;
>  	int ret;
> diff --git a/fs/hpfs/file.c b/fs/hpfs/file.c
> index 1ecec124e76f..0eece4ae1f11 100644
> --- a/fs/hpfs/file.c
> +++ b/fs/hpfs/file.c
> @@ -190,9 +190,9 @@ static sector_t _hpfs_bmap(struct address_space *mapping, sector_t block)
>  	return generic_block_bmap(mapping, block, hpfs_get_block);
>  }
>  
> -static int hpfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo, u64 start, u64 len)
> +static int hpfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
>  {
> -	return generic_block_fiemap(inode, fieinfo, start, len, hpfs_get_block);
> +	return generic_block_fiemap(inode, fieinfo, hpfs_get_block);
>  }
>  
>  const struct address_space_operations hpfs_aops = {
> diff --git a/fs/ioctl.c b/fs/ioctl.c
> index 6b589c873bc2..ad8edcb10dc9 100644
> --- a/fs/ioctl.c
> +++ b/fs/ioctl.c
> @@ -210,6 +210,8 @@ static int ioctl_fiemap(struct file *filp, unsigned long arg)
>  	fieinfo.fi_flags = fiemap.fm_flags;
>  	fieinfo.fi_extents_max = fiemap.fm_extent_count;
>  	fieinfo.fi_extents_start = ufiemap->fm_extents;
> +	fieinfo.fi_start = fiemap.fm_start;
> +	fieinfo.fi_len = len;
>  
>  	if (fiemap.fm_extent_count != 0 &&
>  	    !access_ok(fieinfo.fi_extents_start,
> @@ -219,7 +221,7 @@ static int ioctl_fiemap(struct file *filp, unsigned long arg)
>  	if (fieinfo.fi_flags & FIEMAP_FLAG_SYNC)
>  		filemap_write_and_wait(inode->i_mapping);
>  
> -	error = inode->i_op->fiemap(inode, &fieinfo, fiemap.fm_start, len);
> +	error = inode->i_op->fiemap(inode, &fieinfo);
>  	fiemap.fm_flags = fieinfo.fi_flags;
>  	fiemap.fm_mapped_extents = fieinfo.fi_extents_mapped;
>  	if (copy_to_user(ufiemap, &fiemap, sizeof(fiemap)))
> @@ -296,9 +298,11 @@ static inline loff_t blk_to_logical(struct inode *inode, sector_t blk)
>   */
>  
>  int __generic_block_fiemap(struct inode *inode,
> -			   struct fiemap_extent_info *fieinfo, loff_t start,
> -			   loff_t len, get_block_t *get_block)
> +			   struct fiemap_extent_info *fieinfo,
> +			   get_block_t *get_block)
>  {
> +	loff_t start = fieinfo->fi_start;
> +	loff_t len = fieinfo->fi_len;
>  	struct buffer_head map_bh;
>  	sector_t start_blk, last_blk;
>  	loff_t isize = i_size_read(inode);
> @@ -455,12 +459,12 @@ EXPORT_SYMBOL(__generic_block_fiemap);
>   */
>  
>  int generic_block_fiemap(struct inode *inode,
> -			 struct fiemap_extent_info *fieinfo, u64 start,
> -			 u64 len, get_block_t *get_block)
> +			 struct fiemap_extent_info *fieinfo,
> +			 get_block_t *get_block)
>  {
>  	int ret;
>  	inode_lock(inode);
> -	ret = __generic_block_fiemap(inode, fieinfo, start, len, get_block);
> +	ret = __generic_block_fiemap(inode, fieinfo, get_block);
>  	inode_unlock(inode);
>  	return ret;
>  }
> diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
> index 671085512e0f..1f37d086371c 100644
> --- a/fs/nilfs2/inode.c
> +++ b/fs/nilfs2/inode.c
> @@ -992,9 +992,10 @@ void nilfs_dirty_inode(struct inode *inode, int flags)
>  	nilfs_transaction_commit(inode->i_sb); /* never fails */
>  }
>  
> -int nilfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> -		 __u64 start, __u64 len)
> +int nilfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
>  {
> +	u64 start = fieinfo->fi_start;
> +	u64 len = fieinfo->fi_len;
>  	struct the_nilfs *nilfs = inode->i_sb->s_fs_info;
>  	__u64 logical = 0, phys = 0, size = 0;
>  	__u32 flags = 0;
> diff --git a/fs/nilfs2/nilfs.h b/fs/nilfs2/nilfs.h
> index 42395ba52da6..f74b64a76c52 100644
> --- a/fs/nilfs2/nilfs.h
> +++ b/fs/nilfs2/nilfs.h
> @@ -275,8 +275,7 @@ extern int nilfs_inode_dirty(struct inode *);
>  int nilfs_set_file_dirty(struct inode *inode, unsigned int nr_dirty);
>  extern int __nilfs_mark_inode_dirty(struct inode *, int);
>  extern void nilfs_dirty_inode(struct inode *, int flags);
> -int nilfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> -		 __u64 start, __u64 len);
> +int nilfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo);
>  static inline int nilfs_mark_inode_dirty(struct inode *inode)
>  {
>  	return __nilfs_mark_inode_dirty(inode, I_DIRTY);
> diff --git a/fs/ocfs2/extent_map.c b/fs/ocfs2/extent_map.c
> index e66a249fe07c..1a5b6af62ee0 100644
> --- a/fs/ocfs2/extent_map.c
> +++ b/fs/ocfs2/extent_map.c
> @@ -736,8 +736,7 @@ static int ocfs2_fiemap_inline(struct inode *inode, struct buffer_head *di_bh,
>  
>  #define OCFS2_FIEMAP_FLAGS	(FIEMAP_FLAG_SYNC)
>  
> -int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> -		 u64 map_start, u64 map_len)
> +int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
>  {
>  	int ret, is_last;
>  	u32 mapping_end, cpos;
> @@ -746,6 +745,8 @@ int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>  	u64 len_bytes, phys_bytes, virt_bytes;
>  	struct buffer_head *di_bh = NULL;
>  	struct ocfs2_extent_rec rec;
> +	u64 map_start = fieinfo->fi_start;
> +	u64 map_len = fieinfo->fi_len;
>  
>  	ret = fiemap_check_flags(fieinfo, OCFS2_FIEMAP_FLAGS);
>  	if (ret)
> diff --git a/fs/ocfs2/extent_map.h b/fs/ocfs2/extent_map.h
> index e5464f6cee8a..be3724b67d7e 100644
> --- a/fs/ocfs2/extent_map.h
> +++ b/fs/ocfs2/extent_map.h
> @@ -37,8 +37,7 @@ int ocfs2_get_clusters(struct inode *inode, u32 v_cluster, u32 *p_cluster,
>  int ocfs2_extent_map_get_blocks(struct inode *inode, u64 v_blkno, u64 *p_blkno,
>  				u64 *ret_count, unsigned int *extent_flags);
>  
> -int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> -		 u64 map_start, u64 map_len);
> +int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo);
>  
>  int ocfs2_overwrite_io(struct inode *inode, struct buffer_head *di_bh,
>  		       u64 map_start, u64 map_len);
> diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
> index 7663aeb85fa3..6fd54fb23275 100644
> --- a/fs/overlayfs/inode.c
> +++ b/fs/overlayfs/inode.c
> @@ -453,8 +453,7 @@ int ovl_update_time(struct inode *inode, struct timespec64 *ts, int flags)
>  	return 0;
>  }
>  
> -static int ovl_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> -		      u64 start, u64 len)
> +static int ovl_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
>  {
>  	int err;
>  	struct inode *realinode = ovl_inode_real(inode);
> @@ -468,7 +467,7 @@ static int ovl_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>  	if (fieinfo->fi_flags & FIEMAP_FLAG_SYNC)
>  		filemap_write_and_wait(realinode->i_mapping);
>  
> -	err = realinode->i_op->fiemap(realinode, fieinfo, start, len);
> +	err = realinode->i_op->fiemap(realinode, fieinfo);
>  	revert_creds(old_cred);
>  
>  	return err;
> diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> index fe285d123d69..8d14d733a0e6 100644
> --- a/fs/xfs/xfs_iops.c
> +++ b/fs/xfs/xfs_iops.c
> @@ -1100,12 +1100,12 @@ xfs_vn_update_time(
>  
>  STATIC int
>  xfs_vn_fiemap(
> -	struct inode		*inode,
> -	struct fiemap_extent_info *fieinfo,
> -	u64			start,
> -	u64			length)
> +	struct inode			*inode,
> +	struct fiemap_extent_info	*fieinfo)
>  {
> -	int			error;
> +	u64				start = fieinfo->fi_start;
> +	u64				length = fieinfo->fi_len;
> +	int				error;
>  
>  	xfs_ilock(XFS_I(inode), XFS_IOLOCK_SHARED);
>  	if (fieinfo->fi_flags & FIEMAP_FLAG_XATTR) {
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index f1fbd8298ca4..2c0438de4982 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1737,11 +1737,16 @@ extern bool may_open_dev(const struct path *path);
>   * VFS FS_IOC_FIEMAP helper definitions.
>   */
>  struct fiemap_extent_info {
> -	unsigned int fi_flags;		/* Flags as passed from user */
> -	unsigned int fi_extents_mapped;	/* Number of mapped extents */
> -	unsigned int fi_extents_max;	/* Size of fiemap_extent array */
> -	struct fiemap_extent __user *fi_extents_start; /* Start of
> -							fiemap_extent array */
> +	unsigned int	fi_flags;		/* Flags as passed from user */
> +	u64		fi_start;		/* Logical offset at which
> +						   start mapping */
> +	u64		fi_len;			/* Logical length of mapping the
> +						   caller cares about */
> +	unsigned int	fi_extents_mapped;	/* Number of mapped extents */
> +	unsigned int	fi_extents_max;		/* Size of fiemap_extent array */
> +	struct		fiemap_extent __user *fi_extents_start;	/* Start of
> +								   fiemap_extent
> +								   array */

I might've just put the comments above rather than squash them into the
right side of the tty, but eh.  If you end up resending this series then
I'd want that fixed up, but for now:

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

:)

--D

>  };
>  int fiemap_fill_next_extent(struct fiemap_extent_info *info, u64 logical,
>  			    u64 phys, u64 len, u32 flags);
> @@ -1873,8 +1878,7 @@ struct inode_operations {
>  	int (*setattr) (struct dentry *, struct iattr *);
>  	int (*getattr) (const struct path *, struct kstat *, u32, unsigned int);
>  	ssize_t (*listxattr) (struct dentry *, char *, size_t);
> -	int (*fiemap)(struct inode *, struct fiemap_extent_info *, u64 start,
> -		      u64 len);
> +	int (*fiemap)(struct inode *, struct fiemap_extent_info *);
>  	int (*update_time)(struct inode *, struct timespec64 *, int);
>  	int (*atomic_open)(struct inode *, struct dentry *,
>  			   struct file *, unsigned open_flag,
> @@ -3268,11 +3272,10 @@ extern int vfs_readlink(struct dentry *, char __user *, int);
>  
>  extern int __generic_block_fiemap(struct inode *inode,
>  				  struct fiemap_extent_info *fieinfo,
> -				  loff_t start, loff_t len,
>  				  get_block_t *get_block);
>  extern int generic_block_fiemap(struct inode *inode,
> -				struct fiemap_extent_info *fieinfo, u64 start,
> -				u64 len, get_block_t *get_block);
> +				struct fiemap_extent_info *fieinfo,
> +				get_block_t *get_block);
>  
>  extern struct file_system_type *get_filesystem(struct file_system_type *fs);
>  extern void put_filesystem(struct file_system_type *fs);
> -- 
> 2.20.1
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 8/9] Use FIEMAP for FIBMAP calls
  2019-09-11 13:43 ` [PATCH 8/9] Use FIEMAP for FIBMAP calls Carlos Maiolino
@ 2019-09-16 17:44   ` Darrick J. Wong
  0 siblings, 0 replies; 24+ messages in thread
From: Darrick J. Wong @ 2019-09-16 17:44 UTC (permalink / raw)
  To: Carlos Maiolino; +Cc: linux-fsdevel, hch, adilger, linux-xfs

On Wed, Sep 11, 2019 at 03:43:14PM +0200, Carlos Maiolino wrote:
> Enables the usage of FIEMAP ioctl infrastructure to handle FIBMAP calls.
> From now on, ->bmap() methods can start to be removed from filesystems
> which already provides ->fiemap().
> 
> This adds a new helper - bmap_fiemap() - which is used to fill in the
> fiemap request, call into the underlying filesystem, check the flags
> set in the extent requested.
> 
> Add a new fiemap fill extent callback to handle the in-kernel only
> fiemap_extent structure used for FIBMAP.
> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---
> 
> With the removal of FIEMAP_KERNEL_FIBMAP flag in this version, we hide
> from the filesystem whether the call is coming from FIEMAP, or FIBMAP.
> 
> In this way, filesystems that previously did not support FIBMAP (but
> supports FIEMAP), may start to support it. It *may* be an issue for
> filesystems like btrfs, where we can't say for sure on which device the
> requested block resides in. But well, this is also true for FIEMAP
> requests.
> At a first glance, I didn't see any harm in having FIBMAP enabled in
> btrfs, my first worry was about bootloaders trying to use it, but I
> don't think it will change much in practice.
> 
> Changelog:
> 
> 	V6:
> 		- Fix sparse warning on fiemal_fill_kernel_extent:
> 			Reported-by: kbuild test robot <lkp@intel.com>
> 		- Fix conflict in ext4
> 		- Remove the FIEMAP_KERNEL_FIBMAP flag and let the
> 		  caller decide whether to reply user's FIBMAP call
> 		  based on the FIEMAP flags returned by the filesystem,
> 		  suggested by Christoph.
> 
> 	V5:
> 		- Properly rebase against 5.3
> 		- Fix xfs coding style
> 		- Use xfs_is_cow_inode() check in xfs_vn_fiemap.
> 			- It needs xfs_reflink.h, but maybe it's better to move
> 			  static calls into xfs_inode.h
> 		- Fix small conflict due indentation update in xfs_vn_fiemap
> 	V4:
> 		- Fix if conditional in bmap()
> 		- Add filesystem-specific modifications
> 	V3:
> 		- Add FIEMAP_EXTENT_SHARED to the list of invalid extents in
> 		  bmap_fiemap()
> 		- Rename fi_extents_start to fi_cb_data
> 		- Use if conditional instead of ternary operator
> 		- Make fiemap_fill_* callbacks static (which required the
> 		  removal of some macros
> 		- Set FIEMAP_FLAG_SYNC when calling in ->fiemap method from
> 		  fibmap
> 		- Add FIEMAP_KERNEL_FIBMAP flag, to identify the usage of fiemap
> 		  infrastructure for fibmap calls, defined in fs.h so it's not
> 		  exported to userspace.
> 		- Update fiemap_check_flags() to understand FIEMAP_KERNEL_FIBMAP
> 		- Update filesystems supporting both FIBMAP and FIEMAP, which
> 		  need extra checks on FIBMAP calls
> 
> 	V2:
> 		- Now based on the updated fiemap_extent_info,
> 		- move the fiemap call itself to a new helper
> 
>  fs/inode.c            | 84 +++++++++++++++++++++++++++++++++++++++++--
>  fs/ioctl.c            | 21 +++++------
>  fs/ocfs2/extent_map.c |  1 +
>  3 files changed, 94 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/inode.c b/fs/inode.c
> index 0a20aaa572f2..ab33d16bc29d 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -1591,6 +1591,81 @@ void iput(struct inode *inode)
>  }
>  EXPORT_SYMBOL(iput);
>  
> +static int fiemap_fill_kernel_extent(struct fiemap_extent_info *fieinfo,
> +			u64 logical, u64 phys, u64 len, u32 flags)
> +{
> +	struct fiemap_extent *extent = fieinfo->fi_cb_data;
> +
> +	/* only count the extents */
> +	if (!fieinfo->fi_cb_data) {
> +		fieinfo->fi_extents_mapped++;
> +		goto out;
> +	}
> +
> +	if (fieinfo->fi_extents_mapped >= fieinfo->fi_extents_max)
> +		return 1;
> +
> +	if (flags & FIEMAP_EXTENT_DELALLOC)
> +		flags |= FIEMAP_EXTENT_UNKNOWN;
> +	if (flags & FIEMAP_EXTENT_DATA_ENCRYPTED)
> +		flags |= FIEMAP_EXTENT_ENCODED;
> +	if (flags & (FIEMAP_EXTENT_DATA_TAIL | FIEMAP_EXTENT_DATA_INLINE))
> +		flags |= FIEMAP_EXTENT_NOT_ALIGNED;
> +
> +	extent->fe_logical = logical;
> +	extent->fe_physical = phys;
> +	extent->fe_length = len;
> +	extent->fe_flags = flags;
> +
> +	fieinfo->fi_extents_mapped++;
> +
> +	if (fieinfo->fi_extents_mapped == fieinfo->fi_extents_max)
> +		return 1;
> +
> +out:
> +	if (flags & FIEMAP_EXTENT_LAST)
> +		return 1;
> +	return 0;
> +}
> +
> +#define FIBMAP_INCOMPAT_FLAGS \
> +	(FIEMAP_EXTENT_UNKNOWN | FIEMAP_EXTENT_DELALLOC | \
> +	 FIEMAP_EXTENT_ENCODED | FIEMAP_EXTENT_DATA_ENCRYPTED | \
> +	 FIEMAP_EXTENT_NOT_ALIGNED | FIEMAP_EXTENT_DATA_INLINE | \
> +	 FIEMAP_EXTENT_DATA_TAIL | FIEMAP_EXTENT_UNWRITTEN | \
> +	 FIEMAP_EXTENT_SHARED)
> +
> +static int bmap_fiemap(struct inode *inode, sector_t *block)
> +{
> +	struct fiemap_extent_info fieinfo = { 0, };
> +	struct fiemap_extent fextent;
> +	u64 start = *block << inode->i_blkbits;
> +	int error = -EINVAL;
> +
> +	fextent.fe_logical = 0;
> +	fextent.fe_physical = 0;
> +	fieinfo.fi_extents_max = 1;
> +	fieinfo.fi_extents_mapped = 0;
> +	fieinfo.fi_cb_data = &fextent;
> +	fieinfo.fi_start = start;
> +	fieinfo.fi_len = 1 << inode->i_blkbits;
> +	fieinfo.fi_cb = fiemap_fill_kernel_extent;
> +	fieinfo.fi_flags = FIEMAP_FLAG_SYNC;
> +
> +	error = inode->i_op->fiemap(inode, &fieinfo);
> +

Unnecessary blank line?

> +	if (error)
> +		return error;
> +
> +	if (fextent.fe_flags & FIBMAP_INCOMPAT_FLAGS)
> +		return -EINVAL;
> +
> +	*block = (fextent.fe_physical +
> +		  (start - fextent.fe_logical)) >> inode->i_blkbits;
> +
> +	return error;
> +}
> +
>  /**
>   *	bmap	- find a block number in a file
>   *	@inode:  inode owning the block number being requested
> @@ -1607,10 +1682,15 @@ EXPORT_SYMBOL(iput);
>   */
>  int bmap(struct inode *inode, sector_t *block)
>  {
> -	if (!inode->i_mapping->a_ops->bmap)
> +	if (inode->i_op->fiemap)
> +		return bmap_fiemap(inode, block);
> +
> +	if (inode->i_mapping->a_ops->bmap)
> +		*block = inode->i_mapping->a_ops->bmap(inode->i_mapping,
> +						       *block);
> +	else
>  		return -EINVAL;
>  
> -	*block = inode->i_mapping->a_ops->bmap(inode->i_mapping, *block);
>  	return 0;
>  }
>  EXPORT_SYMBOL(bmap);
> diff --git a/fs/ioctl.c b/fs/ioctl.c
> index d72696c222de..fbfd631ba3cb 100644
> --- a/fs/ioctl.c
> +++ b/fs/ioctl.c
> @@ -77,11 +77,8 @@ static int ioctl_fibmap(struct file *filp, int __user *p)
>  	return error;
>  }
>  
> -#define SET_UNKNOWN_FLAGS	(FIEMAP_EXTENT_DELALLOC)
> -#define SET_NO_UNMOUNTED_IO_FLAGS	(FIEMAP_EXTENT_DATA_ENCRYPTED)
> -#define SET_NOT_ALIGNED_FLAGS	(FIEMAP_EXTENT_DATA_TAIL|FIEMAP_EXTENT_DATA_INLINE)
> -int fiemap_fill_user_extent(struct fiemap_extent_info *fieinfo, u64 logical,
> -			    u64 phys, u64 len, u32 flags)
> +static int fiemap_fill_user_extent(struct fiemap_extent_info *fieinfo,
> +			u64 logical, u64 phys, u64 len, u32 flags)
>  {
>  	struct fiemap_extent extent;
>  	struct fiemap_extent __user *dest = fieinfo->fi_cb_data;
> @@ -89,17 +86,17 @@ int fiemap_fill_user_extent(struct fiemap_extent_info *fieinfo, u64 logical,
>  	/* only count the extents */
>  	if (fieinfo->fi_extents_max == 0) {
>  		fieinfo->fi_extents_mapped++;
> -		return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0;
> +		goto out;
>  	}
>  
>  	if (fieinfo->fi_extents_mapped >= fieinfo->fi_extents_max)
>  		return 1;
>  
> -	if (flags & SET_UNKNOWN_FLAGS)
> +	if (flags & FIEMAP_EXTENT_DELALLOC)
>  		flags |= FIEMAP_EXTENT_UNKNOWN;
> -	if (flags & SET_NO_UNMOUNTED_IO_FLAGS)
> +	if (flags & FIEMAP_EXTENT_DATA_ENCRYPTED)
>  		flags |= FIEMAP_EXTENT_ENCODED;
> -	if (flags & SET_NOT_ALIGNED_FLAGS)
> +	if (flags & (FIEMAP_EXTENT_DATA_TAIL | FIEMAP_EXTENT_DATA_INLINE))
>  		flags |= FIEMAP_EXTENT_NOT_ALIGNED;
>  
>  	memset(&extent, 0, sizeof(extent));
> @@ -115,7 +112,11 @@ int fiemap_fill_user_extent(struct fiemap_extent_info *fieinfo, u64 logical,
>  	fieinfo->fi_extents_mapped++;
>  	if (fieinfo->fi_extents_mapped == fieinfo->fi_extents_max)
>  		return 1;
> -	return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0;
> +
> +out:
> +	if (flags & FIEMAP_EXTENT_LAST)
> +		return 1;
> +	return 0;
>  }
>  
>  /**
> diff --git a/fs/ocfs2/extent_map.c b/fs/ocfs2/extent_map.c
> index 1a5b6af62ee0..0dd838615d14 100644
> --- a/fs/ocfs2/extent_map.c
> +++ b/fs/ocfs2/extent_map.c
> @@ -743,6 +743,7 @@ int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
>  	unsigned int hole_size;
>  	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
>  	u64 len_bytes, phys_bytes, virt_bytes;
> +

Unnecessary code change...

With that fixed up, I'd say:
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

>  	struct buffer_head *di_bh = NULL;
>  	struct ocfs2_extent_rec rec;
>  	u64 map_start = fieinfo->fi_start;
> -- 
> 2.20.1
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 9/9] xfs: Get rid of ->bmap
  2019-09-11 13:43 ` [PATCH 9/9] xfs: Get rid of ->bmap Carlos Maiolino
@ 2019-09-16 17:50   ` Darrick J. Wong
  2019-09-18  8:13     ` Carlos Maiolino
  0 siblings, 1 reply; 24+ messages in thread
From: Darrick J. Wong @ 2019-09-16 17:50 UTC (permalink / raw)
  To: Carlos Maiolino; +Cc: linux-fsdevel, hch, adilger, linux-xfs

On Wed, Sep 11, 2019 at 03:43:15PM +0200, Carlos Maiolino wrote:
> We don't need ->bmap anymore, only usage for it was FIBMAP, which is now
> gone.
> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---
> 
> Changelog:
> 	V5:
> 		- Properly rebase against 5.3
> 		- iomap_{bmap(),bmap_actor()} are now used also by GFS2, so
> 		  don't remove them anymore
> 	V2:
> 		- Kill iomap_bmap() and iomap_bmap_actor()
> 
>  fs/xfs/xfs_aops.c  | 24 ------------------------
>  fs/xfs/xfs_trace.h |  1 -
>  2 files changed, 25 deletions(-)
> 
> diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
> index 4e4a4d7df5ac..a2884537d2c2 100644
> --- a/fs/xfs/xfs_aops.c
> +++ b/fs/xfs/xfs_aops.c
> @@ -1138,29 +1138,6 @@ xfs_vm_releasepage(
>  	return iomap_releasepage(page, gfp_mask);
>  }
>  
> -STATIC sector_t
> -xfs_vm_bmap(
> -	struct address_space	*mapping,
> -	sector_t		block)
> -{
> -	struct xfs_inode	*ip = XFS_I(mapping->host);
> -
> -	trace_xfs_vm_bmap(ip);
> -
> -	/*
> -	 * The swap code (ab-)uses ->bmap to get a block mapping and then
> -	 * bypasses the file system for actual I/O.  We really can't allow
> -	 * that on reflinks inodes, so we have to skip out here.  And yes,
> -	 * 0 is the magic code for a bmap error.
> -	 *
> -	 * Since we don't pass back blockdev info, we can't return bmap
> -	 * information for rt files either.
> -	 */
> -	if (xfs_is_cow_inode(ip) || XFS_IS_REALTIME_INODE(ip))

Uhhhh where does this check happen now?

--D

> -		return 0;
> -	return iomap_bmap(mapping, block, &xfs_iomap_ops);
> -}
> -
>  STATIC int
>  xfs_vm_readpage(
>  	struct file		*unused,
> @@ -1199,7 +1176,6 @@ const struct address_space_operations xfs_address_space_operations = {
>  	.set_page_dirty		= iomap_set_page_dirty,
>  	.releasepage		= xfs_vm_releasepage,
>  	.invalidatepage		= xfs_vm_invalidatepage,
> -	.bmap			= xfs_vm_bmap,
>  	.direct_IO		= noop_direct_IO,
>  	.migratepage		= iomap_migrate_page,
>  	.is_partially_uptodate  = iomap_is_partially_uptodate,
> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> index eaae275ed430..c226b562f5da 100644
> --- a/fs/xfs/xfs_trace.h
> +++ b/fs/xfs/xfs_trace.h
> @@ -626,7 +626,6 @@ DEFINE_INODE_EVENT(xfs_readdir);
>  #ifdef CONFIG_XFS_POSIX_ACL
>  DEFINE_INODE_EVENT(xfs_get_acl);
>  #endif
> -DEFINE_INODE_EVENT(xfs_vm_bmap);
>  DEFINE_INODE_EVENT(xfs_file_ioctl);
>  DEFINE_INODE_EVENT(xfs_file_compat_ioctl);
>  DEFINE_INODE_EVENT(xfs_ioctl_setattr);
> -- 
> 2.20.1
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 9/9] xfs: Get rid of ->bmap
  2019-09-16 17:50   ` Darrick J. Wong
@ 2019-09-18  8:13     ` Carlos Maiolino
  2019-09-18 13:24       ` Christoph Hellwig
  0 siblings, 1 reply; 24+ messages in thread
From: Carlos Maiolino @ 2019-09-18  8:13 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-fsdevel, hch, adilger, linux-xfs

On Mon, Sep 16, 2019 at 10:50:49AM -0700, Darrick J. Wong wrote:
> On Wed, Sep 11, 2019 at 03:43:15PM +0200, Carlos Maiolino wrote:
> > We don't need ->bmap anymore, only usage for it was FIBMAP, which is now
> > gone.
> > 
> > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> > ---
> > 
> > Changelog:
> > 	V5:
> > 		- Properly rebase against 5.3
> > 		- iomap_{bmap(),bmap_actor()} are now used also by GFS2, so
> > 		  don't remove them anymore
> > 	V2:
> > 		- Kill iomap_bmap() and iomap_bmap_actor()
> > 
> >  fs/xfs/xfs_aops.c  | 24 ------------------------
> >  fs/xfs/xfs_trace.h |  1 -
> >  2 files changed, 25 deletions(-)
> > 
> > diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
> > index 4e4a4d7df5ac..a2884537d2c2 100644
> > --- a/fs/xfs/xfs_aops.c
> > +++ b/fs/xfs/xfs_aops.c
> > @@ -1138,29 +1138,6 @@ xfs_vm_releasepage(
> >  	return iomap_releasepage(page, gfp_mask);
> >  }
> >  
> > -STATIC sector_t
> > -xfs_vm_bmap(
> > -	struct address_space	*mapping,
> > -	sector_t		block)
> > -{
> > -	struct xfs_inode	*ip = XFS_I(mapping->host);
> > -
> > -	trace_xfs_vm_bmap(ip);
> > -
> > -	/*
> > -	 * The swap code (ab-)uses ->bmap to get a block mapping and then
> > -	 * bypasses the file system for actual I/O.  We really can't allow
> > -	 * that on reflinks inodes, so we have to skip out here.  And yes,
> > -	 * 0 is the magic code for a bmap error.
> > -	 *
> > -	 * Since we don't pass back blockdev info, we can't return bmap
> > -	 * information for rt files either.
> > -	 */
> > -	if (xfs_is_cow_inode(ip) || XFS_IS_REALTIME_INODE(ip))
> 
> Uhhhh where does this check happen now?

All checks are now made in the caller, bmap_fiemap() based on the filesystem's
returned flags in the fiemap structure. So, it will decide to pass the result
back, or just return -EINVAL.

Well, there is no way for iomap (or bmap_fiemap now) detect the block is in a
realtime device, since we have no flags for that.

Following Christoph's line of thought here, maybe we can add a new IOMAP_F_* so
the filesystem can notify iomap the extent is in a different device? I don't
know, just a thought.

This would still keep the consistency of leaving bmap_fiemap() with the decision
of passing or not.

Cheers.

> 
> --D
> 
> > -		return 0;
> > -	return iomap_bmap(mapping, block, &xfs_iomap_ops);
> > -}
> > -
> >  STATIC int
> >  xfs_vm_readpage(
> >  	struct file		*unused,
> > @@ -1199,7 +1176,6 @@ const struct address_space_operations xfs_address_space_operations = {
> >  	.set_page_dirty		= iomap_set_page_dirty,
> >  	.releasepage		= xfs_vm_releasepage,
> >  	.invalidatepage		= xfs_vm_invalidatepage,
> > -	.bmap			= xfs_vm_bmap,
> >  	.direct_IO		= noop_direct_IO,
> >  	.migratepage		= iomap_migrate_page,
> >  	.is_partially_uptodate  = iomap_is_partially_uptodate,
> > diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> > index eaae275ed430..c226b562f5da 100644
> > --- a/fs/xfs/xfs_trace.h
> > +++ b/fs/xfs/xfs_trace.h
> > @@ -626,7 +626,6 @@ DEFINE_INODE_EVENT(xfs_readdir);
> >  #ifdef CONFIG_XFS_POSIX_ACL
> >  DEFINE_INODE_EVENT(xfs_get_acl);
> >  #endif
> > -DEFINE_INODE_EVENT(xfs_vm_bmap);
> >  DEFINE_INODE_EVENT(xfs_file_ioctl);
> >  DEFINE_INODE_EVENT(xfs_file_compat_ioctl);
> >  DEFINE_INODE_EVENT(xfs_ioctl_setattr);
> > -- 
> > 2.20.1
> > 

-- 
Carlos


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 9/9] xfs: Get rid of ->bmap
  2019-09-18  8:13     ` Carlos Maiolino
@ 2019-09-18 13:24       ` Christoph Hellwig
  2019-09-18 16:12         ` Darrick J. Wong
  0 siblings, 1 reply; 24+ messages in thread
From: Christoph Hellwig @ 2019-09-18 13:24 UTC (permalink / raw)
  To: Darrick J. Wong, linux-fsdevel, hch, adilger, linux-xfs

On Wed, Sep 18, 2019 at 10:13:04AM +0200, Carlos Maiolino wrote:
> All checks are now made in the caller, bmap_fiemap() based on the filesystem's
> returned flags in the fiemap structure. So, it will decide to pass the result
> back, or just return -EINVAL.
> 
> Well, there is no way for iomap (or bmap_fiemap now) detect the block is in a
> realtime device, since we have no flags for that.
> 
> Following Christoph's line of thought here, maybe we can add a new IOMAP_F_* so
> the filesystem can notify iomap the extent is in a different device? I don't
> know, just a thought.
> 
> This would still keep the consistency of leaving bmap_fiemap() with the decision
> of passing or not.

I think this actually is a problem with FIEMAP as well, as it
doesn't report that things are on a different device.  So I guess for
now we should fail FIEMAP on the RT device as well.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 9/9] xfs: Get rid of ->bmap
  2019-09-18 13:24       ` Christoph Hellwig
@ 2019-09-18 16:12         ` Darrick J. Wong
  0 siblings, 0 replies; 24+ messages in thread
From: Darrick J. Wong @ 2019-09-18 16:12 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-fsdevel, adilger, linux-xfs

On Wed, Sep 18, 2019 at 03:24:36PM +0200, Christoph Hellwig wrote:
> On Wed, Sep 18, 2019 at 10:13:04AM +0200, Carlos Maiolino wrote:
> > All checks are now made in the caller, bmap_fiemap() based on the filesystem's
> > returned flags in the fiemap structure. So, it will decide to pass the result
> > back, or just return -EINVAL.
> > 
> > Well, there is no way for iomap (or bmap_fiemap now) detect the block is in a
> > realtime device, since we have no flags for that.
> > 
> > Following Christoph's line of thought here, maybe we can add a new IOMAP_F_* so
> > the filesystem can notify iomap the extent is in a different device? I don't
> > know, just a thought.
> > 
> > This would still keep the consistency of leaving bmap_fiemap() with the decision
> > of passing or not.
> 
> I think this actually is a problem with FIEMAP as well, as it
> doesn't report that things are on a different device.  So I guess for
> now we should fail FIEMAP on the RT device as well.

Or enhance FIEMAP to report some kind of device id like I suggested a
while back...

--D

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 5/9] fs: Move start and length fiemap fields into fiemap_extent_info
  2019-08-08  8:27 ` [PATCH 5/9] fs: Move start and length fiemap fields into fiemap_extent_info Carlos Maiolino
@ 2019-08-08 20:21   ` kbuild test robot
  0 siblings, 0 replies; 24+ messages in thread
From: kbuild test robot @ 2019-08-08 20:21 UTC (permalink / raw)
  To: Carlos Maiolino
  Cc: kbuild-all, linux-fsdevel, hch, adilger, jaegeuk, darrick.wong,
	miklos, rpeterso, linux-xfs

[-- Attachment #1: Type: text/plain, Size: 2222 bytes --]

Hi Carlos,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[cannot apply to v5.3-rc3 next-20190808]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Carlos-Maiolino/New-fiemap-infrastructure-and-bmap-removal/20190808-221354
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gcc (GCC) 7.4.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.4.0 make.cross ARCH=sh 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> fs//cifs/cifsfs.c:1006:12: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
     .fiemap = cifs_fiemap,
               ^~~~~~~~~~~
   fs//cifs/cifsfs.c:1006:12: note: (near initialization for 'cifs_file_inode_ops.fiemap')
   cc1: some warnings being treated as errors

vim +1006 fs//cifs/cifsfs.c

^1da177e4c3f41 Linus Torvalds   2005-04-16  1000  
754661f143e70d Arjan van de Ven 2007-02-12  1001  const struct inode_operations cifs_file_inode_ops = {
^1da177e4c3f41 Linus Torvalds   2005-04-16  1002  	.setattr = cifs_setattr,
48a77aa7e20557 Steve French     2016-05-18  1003  	.getattr = cifs_getattr,
^1da177e4c3f41 Linus Torvalds   2005-04-16  1004  	.permission = cifs_permission,
^1da177e4c3f41 Linus Torvalds   2005-04-16  1005  	.listxattr = cifs_listxattr,
2f3ebaba13cebd Ronnie Sahlberg  2019-04-25 @1006  	.fiemap = cifs_fiemap,
^1da177e4c3f41 Linus Torvalds   2005-04-16  1007  };
^1da177e4c3f41 Linus Torvalds   2005-04-16  1008  

:::::: The code at line 1006 was first introduced by commit
:::::: 2f3ebaba13cebd8badfb9aed31c0cf3cc82eb4f4 cifs: add fiemap support

:::::: TO: Ronnie Sahlberg <lsahlber@redhat.com>
:::::: CC: Steve French <stfrench@microsoft.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 51784 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 5/9] fs: Move start and length fiemap fields into fiemap_extent_info
  2019-08-08  8:27 [PATCH 0/9 V5] New ->fiemap infrastructure and ->bmap removal Carlos Maiolino
@ 2019-08-08  8:27 ` Carlos Maiolino
  2019-08-08 20:21   ` kbuild test robot
  0 siblings, 1 reply; 24+ messages in thread
From: Carlos Maiolino @ 2019-08-08  8:27 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: hch, adilger, jaegeuk, darrick.wong, miklos, rpeterso, linux-xfs

As the overall goal to deprecate fibmap, Christoph suggested a rework of
the ->fiemap API, in a way we could pass to it a callback to fill the
fiemap structure (one of these callbacks being fiemap_fill_next_extent).

To avoid the need to add several fields into the ->fiemap method, aggregate
everything into a single data structure, and pass it along.

This patch isn't suppose to add any functional change, only to update
filesystems providing ->fiemap() method.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
Changelog:

	V5:
		- Add comments to fi_start and fi_len fields of
		  fiemap_extent_info
		- Fix xfs_vn_fiemap indentation

 fs/bad_inode.c        |  3 +--
 fs/btrfs/inode.c      |  5 +++--
 fs/ext2/ext2.h        |  3 +--
 fs/ext2/inode.c       |  6 ++----
 fs/ext4/ext4.h        |  3 +--
 fs/ext4/extents.c     |  8 ++++----
 fs/f2fs/data.c        |  5 +++--
 fs/f2fs/f2fs.h        |  3 +--
 fs/gfs2/inode.c       |  5 +++--
 fs/hpfs/file.c        |  4 ++--
 fs/ioctl.c            | 16 ++++++++++------
 fs/nilfs2/inode.c     |  5 +++--
 fs/nilfs2/nilfs.h     |  3 +--
 fs/ocfs2/extent_map.c |  5 +++--
 fs/ocfs2/extent_map.h |  3 +--
 fs/overlayfs/inode.c  |  5 ++---
 fs/xfs/xfs_iops.c     | 10 +++++-----
 include/linux/fs.h    | 23 +++++++++++++----------
 18 files changed, 59 insertions(+), 56 deletions(-)

diff --git a/fs/bad_inode.c b/fs/bad_inode.c
index 8035d2a44561..21dfaf876814 100644
--- a/fs/bad_inode.c
+++ b/fs/bad_inode.c
@@ -120,8 +120,7 @@ static struct posix_acl *bad_inode_get_acl(struct inode *inode, int type)
 }
 
 static int bad_inode_fiemap(struct inode *inode,
-			    struct fiemap_extent_info *fieinfo, u64 start,
-			    u64 len)
+			    struct fiemap_extent_info *fieinfo)
 {
 	return -EIO;
 }
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index ee582a36653d..f89767f112df 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -8695,9 +8695,10 @@ static ssize_t btrfs_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
 
 #define BTRFS_FIEMAP_FLAGS	(FIEMAP_FLAG_SYNC)
 
-static int btrfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-		__u64 start, __u64 len)
+static int btrfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
 {
+	u64	start = fieinfo->fi_start;
+	u64	len = fieinfo->fi_len;
 	int	ret;
 
 	ret = fiemap_check_flags(fieinfo, BTRFS_FIEMAP_FLAGS);
diff --git a/fs/ext2/ext2.h b/fs/ext2/ext2.h
index 10ab238de9a6..284df1af9474 100644
--- a/fs/ext2/ext2.h
+++ b/fs/ext2/ext2.h
@@ -760,8 +760,7 @@ extern int ext2_get_block(struct inode *, sector_t, struct buffer_head *, int);
 extern int ext2_setattr (struct dentry *, struct iattr *);
 extern int ext2_getattr (const struct path *, struct kstat *, u32, unsigned int);
 extern void ext2_set_inode_flags(struct inode *inode);
-extern int ext2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-		       u64 start, u64 len);
+extern int ext2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo);
 
 /* ioctl.c */
 extern long ext2_ioctl(struct file *, unsigned int, unsigned long);
diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
index 7004ce581a32..6956df98dd53 100644
--- a/fs/ext2/inode.c
+++ b/fs/ext2/inode.c
@@ -857,11 +857,9 @@ const struct iomap_ops ext2_iomap_ops = {
 const struct iomap_ops ext2_iomap_ops;
 #endif /* CONFIG_FS_DAX */
 
-int ext2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-		u64 start, u64 len)
+int ext2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
 {
-	return generic_block_fiemap(inode, fieinfo, start, len,
-				    ext2_get_block);
+	return generic_block_fiemap(inode, fieinfo, ext2_get_block);
 }
 
 static int ext2_writepage(struct page *page, struct writeback_control *wbc)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index bf660aa7a9e0..2c12b011d1d6 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -3243,8 +3243,7 @@ extern struct ext4_ext_path *ext4_find_extent(struct inode *, ext4_lblk_t,
 extern void ext4_ext_drop_refs(struct ext4_ext_path *);
 extern int ext4_ext_check_inode(struct inode *inode);
 extern ext4_lblk_t ext4_ext_next_allocated_block(struct ext4_ext_path *path);
-extern int ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-			__u64 start, __u64 len);
+extern int ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo);
 extern int ext4_ext_precache(struct inode *inode);
 extern int ext4_collapse_range(struct inode *inode, loff_t offset, loff_t len);
 extern int ext4_insert_range(struct inode *inode, loff_t offset, loff_t len);
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 92266a2da7d6..300339d1cd62 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -5055,9 +5055,10 @@ static int ext4_xattr_fiemap(struct inode *inode,
 	return (error < 0 ? error : 0);
 }
 
-int ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-		__u64 start, __u64 len)
+int ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
 {
+	u64 start = fieinfo->fi_start;
+	u64 len = fieinfo->fi_len;
 	ext4_lblk_t start_blk;
 	int error = 0;
 
@@ -5079,8 +5080,7 @@ int ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 
 	/* fallback to generic here if not in extents fmt */
 	if (!(ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)))
-		return generic_block_fiemap(inode, fieinfo, start, len,
-			ext4_get_block);
+		return generic_block_fiemap(inode, fieinfo, ext4_get_block);
 
 	if (fiemap_check_flags(fieinfo, EXT4_FIEMAP_FLAGS))
 		return -EBADR;
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 4f5afff39f2a..83e8c2d4a7a9 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1476,9 +1476,10 @@ static int f2fs_xattr_fiemap(struct inode *inode,
 	return (err < 0 ? err : 0);
 }
 
-int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-		u64 start, u64 len)
+int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
 {
+	u64 start = fieinfo->fi_start;
+	u64 len = fieinfo->fi_len;
 	struct buffer_head map_bh;
 	sector_t start_blk, last_blk;
 	pgoff_t next_pgofs;
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 17382da7f0bd..4c4b2f27f94d 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -3187,8 +3187,7 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio);
 void __do_map_lock(struct f2fs_sb_info *sbi, int flag, bool lock);
 int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
 			int create, int flag);
-int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-			u64 start, u64 len);
+int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo);
 bool f2fs_should_update_inplace(struct inode *inode, struct f2fs_io_info *fio);
 bool f2fs_should_update_outplace(struct inode *inode, struct f2fs_io_info *fio);
 void f2fs_invalidate_page(struct page *page, unsigned int offset,
diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index 2e2a8a2fb51d..c8310b823f1d 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -2001,9 +2001,10 @@ static int gfs2_getattr(const struct path *path, struct kstat *stat,
 	return 0;
 }
 
-static int gfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-		       u64 start, u64 len)
+static int gfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
 {
+	u64 start = fieinfo->fi_start;
+	u64 len = fieinfo->fi_len;
 	struct gfs2_inode *ip = GFS2_I(inode);
 	struct gfs2_holder gh;
 	int ret;
diff --git a/fs/hpfs/file.c b/fs/hpfs/file.c
index 1ecec124e76f..0eece4ae1f11 100644
--- a/fs/hpfs/file.c
+++ b/fs/hpfs/file.c
@@ -190,9 +190,9 @@ static sector_t _hpfs_bmap(struct address_space *mapping, sector_t block)
 	return generic_block_bmap(mapping, block, hpfs_get_block);
 }
 
-static int hpfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo, u64 start, u64 len)
+static int hpfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
 {
-	return generic_block_fiemap(inode, fieinfo, start, len, hpfs_get_block);
+	return generic_block_fiemap(inode, fieinfo, hpfs_get_block);
 }
 
 const struct address_space_operations hpfs_aops = {
diff --git a/fs/ioctl.c b/fs/ioctl.c
index 6b589c873bc2..ad8edcb10dc9 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -210,6 +210,8 @@ static int ioctl_fiemap(struct file *filp, unsigned long arg)
 	fieinfo.fi_flags = fiemap.fm_flags;
 	fieinfo.fi_extents_max = fiemap.fm_extent_count;
 	fieinfo.fi_extents_start = ufiemap->fm_extents;
+	fieinfo.fi_start = fiemap.fm_start;
+	fieinfo.fi_len = len;
 
 	if (fiemap.fm_extent_count != 0 &&
 	    !access_ok(fieinfo.fi_extents_start,
@@ -219,7 +221,7 @@ static int ioctl_fiemap(struct file *filp, unsigned long arg)
 	if (fieinfo.fi_flags & FIEMAP_FLAG_SYNC)
 		filemap_write_and_wait(inode->i_mapping);
 
-	error = inode->i_op->fiemap(inode, &fieinfo, fiemap.fm_start, len);
+	error = inode->i_op->fiemap(inode, &fieinfo);
 	fiemap.fm_flags = fieinfo.fi_flags;
 	fiemap.fm_mapped_extents = fieinfo.fi_extents_mapped;
 	if (copy_to_user(ufiemap, &fiemap, sizeof(fiemap)))
@@ -296,9 +298,11 @@ static inline loff_t blk_to_logical(struct inode *inode, sector_t blk)
  */
 
 int __generic_block_fiemap(struct inode *inode,
-			   struct fiemap_extent_info *fieinfo, loff_t start,
-			   loff_t len, get_block_t *get_block)
+			   struct fiemap_extent_info *fieinfo,
+			   get_block_t *get_block)
 {
+	loff_t start = fieinfo->fi_start;
+	loff_t len = fieinfo->fi_len;
 	struct buffer_head map_bh;
 	sector_t start_blk, last_blk;
 	loff_t isize = i_size_read(inode);
@@ -455,12 +459,12 @@ EXPORT_SYMBOL(__generic_block_fiemap);
  */
 
 int generic_block_fiemap(struct inode *inode,
-			 struct fiemap_extent_info *fieinfo, u64 start,
-			 u64 len, get_block_t *get_block)
+			 struct fiemap_extent_info *fieinfo,
+			 get_block_t *get_block)
 {
 	int ret;
 	inode_lock(inode);
-	ret = __generic_block_fiemap(inode, fieinfo, start, len, get_block);
+	ret = __generic_block_fiemap(inode, fieinfo, get_block);
 	inode_unlock(inode);
 	return ret;
 }
diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
index 671085512e0f..1f37d086371c 100644
--- a/fs/nilfs2/inode.c
+++ b/fs/nilfs2/inode.c
@@ -992,9 +992,10 @@ void nilfs_dirty_inode(struct inode *inode, int flags)
 	nilfs_transaction_commit(inode->i_sb); /* never fails */
 }
 
-int nilfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-		 __u64 start, __u64 len)
+int nilfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
 {
+	u64 start = fieinfo->fi_start;
+	u64 len = fieinfo->fi_len;
 	struct the_nilfs *nilfs = inode->i_sb->s_fs_info;
 	__u64 logical = 0, phys = 0, size = 0;
 	__u32 flags = 0;
diff --git a/fs/nilfs2/nilfs.h b/fs/nilfs2/nilfs.h
index 42395ba52da6..f74b64a76c52 100644
--- a/fs/nilfs2/nilfs.h
+++ b/fs/nilfs2/nilfs.h
@@ -275,8 +275,7 @@ extern int nilfs_inode_dirty(struct inode *);
 int nilfs_set_file_dirty(struct inode *inode, unsigned int nr_dirty);
 extern int __nilfs_mark_inode_dirty(struct inode *, int);
 extern void nilfs_dirty_inode(struct inode *, int flags);
-int nilfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-		 __u64 start, __u64 len);
+int nilfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo);
 static inline int nilfs_mark_inode_dirty(struct inode *inode)
 {
 	return __nilfs_mark_inode_dirty(inode, I_DIRTY);
diff --git a/fs/ocfs2/extent_map.c b/fs/ocfs2/extent_map.c
index e66a249fe07c..1a5b6af62ee0 100644
--- a/fs/ocfs2/extent_map.c
+++ b/fs/ocfs2/extent_map.c
@@ -736,8 +736,7 @@ static int ocfs2_fiemap_inline(struct inode *inode, struct buffer_head *di_bh,
 
 #define OCFS2_FIEMAP_FLAGS	(FIEMAP_FLAG_SYNC)
 
-int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-		 u64 map_start, u64 map_len)
+int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
 {
 	int ret, is_last;
 	u32 mapping_end, cpos;
@@ -746,6 +745,8 @@ int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 	u64 len_bytes, phys_bytes, virt_bytes;
 	struct buffer_head *di_bh = NULL;
 	struct ocfs2_extent_rec rec;
+	u64 map_start = fieinfo->fi_start;
+	u64 map_len = fieinfo->fi_len;
 
 	ret = fiemap_check_flags(fieinfo, OCFS2_FIEMAP_FLAGS);
 	if (ret)
diff --git a/fs/ocfs2/extent_map.h b/fs/ocfs2/extent_map.h
index e5464f6cee8a..be3724b67d7e 100644
--- a/fs/ocfs2/extent_map.h
+++ b/fs/ocfs2/extent_map.h
@@ -37,8 +37,7 @@ int ocfs2_get_clusters(struct inode *inode, u32 v_cluster, u32 *p_cluster,
 int ocfs2_extent_map_get_blocks(struct inode *inode, u64 v_blkno, u64 *p_blkno,
 				u64 *ret_count, unsigned int *extent_flags);
 
-int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-		 u64 map_start, u64 map_len);
+int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo);
 
 int ocfs2_overwrite_io(struct inode *inode, struct buffer_head *di_bh,
 		       u64 map_start, u64 map_len);
diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 7663aeb85fa3..6fd54fb23275 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -453,8 +453,7 @@ int ovl_update_time(struct inode *inode, struct timespec64 *ts, int flags)
 	return 0;
 }
 
-static int ovl_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-		      u64 start, u64 len)
+static int ovl_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
 {
 	int err;
 	struct inode *realinode = ovl_inode_real(inode);
@@ -468,7 +467,7 @@ static int ovl_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 	if (fieinfo->fi_flags & FIEMAP_FLAG_SYNC)
 		filemap_write_and_wait(realinode->i_mapping);
 
-	err = realinode->i_op->fiemap(realinode, fieinfo, start, len);
+	err = realinode->i_op->fiemap(realinode, fieinfo);
 	revert_creds(old_cred);
 
 	return err;
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index ff3c1fae5357..89982271ffce 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -1099,12 +1099,12 @@ xfs_vn_update_time(
 
 STATIC int
 xfs_vn_fiemap(
-	struct inode		*inode,
-	struct fiemap_extent_info *fieinfo,
-	u64			start,
-	u64			length)
+	struct inode			*inode,
+	struct fiemap_extent_info	*fieinfo)
 {
-	int			error;
+	u64				start = fieinfo->fi_start;
+	u64				length = fieinfo->fi_len;
+	int				error;
 
 	xfs_ilock(XFS_I(inode), XFS_IOLOCK_SHARED);
 	if (fieinfo->fi_flags & FIEMAP_FLAG_XATTR) {
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 934d7c57eec6..070ce5a64ba7 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1712,11 +1712,16 @@ extern bool may_open_dev(const struct path *path);
  * VFS FS_IOC_FIEMAP helper definitions.
  */
 struct fiemap_extent_info {
-	unsigned int fi_flags;		/* Flags as passed from user */
-	unsigned int fi_extents_mapped;	/* Number of mapped extents */
-	unsigned int fi_extents_max;	/* Size of fiemap_extent array */
-	struct fiemap_extent __user *fi_extents_start; /* Start of
-							fiemap_extent array */
+	unsigned int	fi_flags;		/* Flags as passed from user */
+	u64		fi_start;		/* Logical offset at which
+						   start mapping */
+	u64		fi_len;			/* Logical length of mapping the
+						   caller cares about */
+	unsigned int	fi_extents_mapped;	/* Number of mapped extents */
+	unsigned int	fi_extents_max;		/* Size of fiemap_extent array */
+	struct		fiemap_extent __user *fi_extents_start;	/* Start of
+								   fiemap_extent
+								   array */
 };
 int fiemap_fill_next_extent(struct fiemap_extent_info *info, u64 logical,
 			    u64 phys, u64 len, u32 flags);
@@ -1848,8 +1853,7 @@ struct inode_operations {
 	int (*setattr) (struct dentry *, struct iattr *);
 	int (*getattr) (const struct path *, struct kstat *, u32, unsigned int);
 	ssize_t (*listxattr) (struct dentry *, char *, size_t);
-	int (*fiemap)(struct inode *, struct fiemap_extent_info *, u64 start,
-		      u64 len);
+	int (*fiemap)(struct inode *, struct fiemap_extent_info *);
 	int (*update_time)(struct inode *, struct timespec64 *, int);
 	int (*atomic_open)(struct inode *, struct dentry *,
 			   struct file *, unsigned open_flag,
@@ -3204,11 +3208,10 @@ extern int vfs_readlink(struct dentry *, char __user *, int);
 
 extern int __generic_block_fiemap(struct inode *inode,
 				  struct fiemap_extent_info *fieinfo,
-				  loff_t start, loff_t len,
 				  get_block_t *get_block);
 extern int generic_block_fiemap(struct inode *inode,
-				struct fiemap_extent_info *fieinfo, u64 start,
-				u64 len, get_block_t *get_block);
+				struct fiemap_extent_info *fieinfo,
+				get_block_t *get_block);
 
 extern struct file_system_type *get_filesystem(struct file_system_type *fs);
 extern void put_filesystem(struct file_system_type *fs);
-- 
2.20.1

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 5/9] fs: Move start and length fiemap fields into fiemap_extent_info
  2019-08-02  9:51     ` Carlos Maiolino
  2019-08-02 15:15       ` Darrick J. Wong
@ 2019-08-06  5:39       ` Christoph Hellwig
  1 sibling, 0 replies; 24+ messages in thread
From: Christoph Hellwig @ 2019-08-06  5:39 UTC (permalink / raw)
  To: Darrick J. Wong, linux-fsdevel, hch, adilger, jaegeuk, miklos,
	rpeterso, linux-xfs

On Fri, Aug 02, 2019 at 11:51:16AM +0200, Carlos Maiolino wrote:
> Christoph, may I keep your reviewed tag by updating the comments as above?
> Otherwise I'll just remove your tag

Feel free to keep them for any trivial changes.  Indentation changes and
adding comments always qualify as trivial.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 5/9] fs: Move start and length fiemap fields into fiemap_extent_info
  2019-08-02 15:15       ` Darrick J. Wong
@ 2019-08-05  9:40         ` Carlos Maiolino
  0 siblings, 0 replies; 24+ messages in thread
From: Carlos Maiolino @ 2019-08-05  9:40 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: linux-fsdevel, hch, adilger, jaegeuk, miklos, rpeterso, linux-xfs

On Fri, Aug 02, 2019 at 08:15:19AM -0700, Darrick J. Wong wrote:
> On Fri, Aug 02, 2019 at 11:51:16AM +0200, Carlos Maiolino wrote:
> > > >  
> > > >  STATIC int
> > > >  xfs_vn_fiemap(
> > > > -	struct inode		*inode,
> > > > -	struct fiemap_extent_info *fieinfo,
> > > > -	u64			start,
> > > > -	u64			length)
> > > > +	struct inode		  *inode,
> > > > +	struct fiemap_extent_info *fieinfo)
> > > >  {
> > > > -	int			error;
> > > > +	u64	start = fieinfo->fi_start;
> > > > +	u64	length = fieinfo->fi_len;
> > > > +	int	error;
> > > 
> > > Would be nice if the variable name indentation was consistent here, but
> > > otherwise the xfs part looks ok.
> > 
> > These fields are removed on the next patch, updating it is really required?
> 
> Yes, please.

NP then

> > > > +	u64		fi_start;
> > > > +	u64		fi_len;
> > > 
> > > Comments for these two new fields?
> > 
> > Sure, how about this:
> > 
> >        u64           fi_start;            /* Logical offset at which
> >                                              start mapping */
> >        u64           fi_len;              /* Logical length of mapping
> >                                              the caller cares about */
> > 
> > 
> > btw, Above indentation won't match final result
> 
> Looks good to me.

Will update the fields, thanks for the review

> 
> > 
> > Christoph, may I keep your reviewed tag by updating the comments as above?
> > Otherwise I'll just remove your tag
> > 
> > > 
> > > --D
> > > 
> > > > +	unsigned int	fi_extents_mapped;	/* Number of mapped extents */
> > > > +	unsigned int	fi_extents_max;		/* Size of fiemap_extent array */
> > > > +	struct		fiemap_extent __user *fi_extents_start;	/* Start of
> > > > +								   fiemap_extent
> > > > +								   array */
> > > >  };
> > > >  int fiemap_fill_next_extent(struct fiemap_extent_info *info, u64 logical,
> > > >  			    u64 phys, u64 len, u32 flags);
> > > > @@ -1841,8 +1844,7 @@ struct inode_operations {
> > > >  	int (*setattr) (struct dentry *, struct iattr *);
> > > >  	int (*getattr) (const struct path *, struct kstat *, u32, unsigned int);
> > > >  	ssize_t (*listxattr) (struct dentry *, char *, size_t);
> > > > -	int (*fiemap)(struct inode *, struct fiemap_extent_info *, u64 start,
> > > > -		      u64 len);
> > > > +	int (*fiemap)(struct inode *, struct fiemap_extent_info *);
> > > >  	int (*update_time)(struct inode *, struct timespec64 *, int);
> > > >  	int (*atomic_open)(struct inode *, struct dentry *,
> > > >  			   struct file *, unsigned open_flag,
> > > > @@ -3199,11 +3201,10 @@ extern int vfs_readlink(struct dentry *, char __user *, int);
> > > >  
> > > >  extern int __generic_block_fiemap(struct inode *inode,
> > > >  				  struct fiemap_extent_info *fieinfo,
> > > > -				  loff_t start, loff_t len,
> > > >  				  get_block_t *get_block);
> > > >  extern int generic_block_fiemap(struct inode *inode,
> > > > -				struct fiemap_extent_info *fieinfo, u64 start,
> > > > -				u64 len, get_block_t *get_block);
> > > > +				struct fiemap_extent_info *fieinfo,
> > > > +				get_block_t *get_block);
> > > >  
> > > >  extern struct file_system_type *get_filesystem(struct file_system_type *fs);
> > > >  extern void put_filesystem(struct file_system_type *fs);
> > > > -- 
> > > > 2.20.1
> > > > 
> > 
> > -- 
> > Carlos

-- 
Carlos

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 5/9] fs: Move start and length fiemap fields into fiemap_extent_info
  2019-08-02  9:51     ` Carlos Maiolino
@ 2019-08-02 15:15       ` Darrick J. Wong
  2019-08-05  9:40         ` Carlos Maiolino
  2019-08-06  5:39       ` Christoph Hellwig
  1 sibling, 1 reply; 24+ messages in thread
From: Darrick J. Wong @ 2019-08-02 15:15 UTC (permalink / raw)
  To: linux-fsdevel, hch, adilger, jaegeuk, miklos, rpeterso, linux-xfs

On Fri, Aug 02, 2019 at 11:51:16AM +0200, Carlos Maiolino wrote:
> > >  
> > >  STATIC int
> > >  xfs_vn_fiemap(
> > > -	struct inode		*inode,
> > > -	struct fiemap_extent_info *fieinfo,
> > > -	u64			start,
> > > -	u64			length)
> > > +	struct inode		  *inode,
> > > +	struct fiemap_extent_info *fieinfo)
> > >  {
> > > -	int			error;
> > > +	u64	start = fieinfo->fi_start;
> > > +	u64	length = fieinfo->fi_len;
> > > +	int	error;
> > 
> > Would be nice if the variable name indentation was consistent here, but
> > otherwise the xfs part looks ok.
> 
> These fields are removed on the next patch, updating it is really required?

Yes, please.

> > 
> > >  
> > >  	xfs_ilock(XFS_I(inode), XFS_IOLOCK_SHARED);
> > >  	if (fieinfo->fi_flags & FIEMAP_FLAG_XATTR) {
> > > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > > index d5e7c744aea6..7b744b7de24e 100644
> > > --- a/include/linux/fs.h
> > > +++ b/include/linux/fs.h
> > > @@ -1705,11 +1705,14 @@ extern bool may_open_dev(const struct path *path);
> > >   * VFS FS_IOC_FIEMAP helper definitions.
> > >   */
> > >  struct fiemap_extent_info {
> > > -	unsigned int fi_flags;		/* Flags as passed from user */
> > > -	unsigned int fi_extents_mapped;	/* Number of mapped extents */
> > > -	unsigned int fi_extents_max;	/* Size of fiemap_extent array */
> > > -	struct fiemap_extent __user *fi_extents_start; /* Start of
> > > -							fiemap_extent array */
> > > +	unsigned int	fi_flags;		/* Flags as passed from user */
> > > +	u64		fi_start;
> > > +	u64		fi_len;
> > 
> > Comments for these two new fields?
> 
> Sure, how about this:
> 
>        u64           fi_start;            /* Logical offset at which
>                                              start mapping */
>        u64           fi_len;              /* Logical length of mapping
>                                              the caller cares about */
> 
> 
> btw, Above indentation won't match final result

Looks good to me.

--D

> 
> Christoph, may I keep your reviewed tag by updating the comments as above?
> Otherwise I'll just remove your tag
> 
> > 
> > --D
> > 
> > > +	unsigned int	fi_extents_mapped;	/* Number of mapped extents */
> > > +	unsigned int	fi_extents_max;		/* Size of fiemap_extent array */
> > > +	struct		fiemap_extent __user *fi_extents_start;	/* Start of
> > > +								   fiemap_extent
> > > +								   array */
> > >  };
> > >  int fiemap_fill_next_extent(struct fiemap_extent_info *info, u64 logical,
> > >  			    u64 phys, u64 len, u32 flags);
> > > @@ -1841,8 +1844,7 @@ struct inode_operations {
> > >  	int (*setattr) (struct dentry *, struct iattr *);
> > >  	int (*getattr) (const struct path *, struct kstat *, u32, unsigned int);
> > >  	ssize_t (*listxattr) (struct dentry *, char *, size_t);
> > > -	int (*fiemap)(struct inode *, struct fiemap_extent_info *, u64 start,
> > > -		      u64 len);
> > > +	int (*fiemap)(struct inode *, struct fiemap_extent_info *);
> > >  	int (*update_time)(struct inode *, struct timespec64 *, int);
> > >  	int (*atomic_open)(struct inode *, struct dentry *,
> > >  			   struct file *, unsigned open_flag,
> > > @@ -3199,11 +3201,10 @@ extern int vfs_readlink(struct dentry *, char __user *, int);
> > >  
> > >  extern int __generic_block_fiemap(struct inode *inode,
> > >  				  struct fiemap_extent_info *fieinfo,
> > > -				  loff_t start, loff_t len,
> > >  				  get_block_t *get_block);
> > >  extern int generic_block_fiemap(struct inode *inode,
> > > -				struct fiemap_extent_info *fieinfo, u64 start,
> > > -				u64 len, get_block_t *get_block);
> > > +				struct fiemap_extent_info *fieinfo,
> > > +				get_block_t *get_block);
> > >  
> > >  extern struct file_system_type *get_filesystem(struct file_system_type *fs);
> > >  extern void put_filesystem(struct file_system_type *fs);
> > > -- 
> > > 2.20.1
> > > 
> 
> -- 
> Carlos

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 5/9] fs: Move start and length fiemap fields into fiemap_extent_info
  2019-07-31 23:28   ` Darrick J. Wong
@ 2019-08-02  9:51     ` Carlos Maiolino
  2019-08-02 15:15       ` Darrick J. Wong
  2019-08-06  5:39       ` Christoph Hellwig
  0 siblings, 2 replies; 24+ messages in thread
From: Carlos Maiolino @ 2019-08-02  9:51 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: linux-fsdevel, hch, adilger, jaegeuk, miklos, rpeterso, linux-xfs

> >  
> >  STATIC int
> >  xfs_vn_fiemap(
> > -	struct inode		*inode,
> > -	struct fiemap_extent_info *fieinfo,
> > -	u64			start,
> > -	u64			length)
> > +	struct inode		  *inode,
> > +	struct fiemap_extent_info *fieinfo)
> >  {
> > -	int			error;
> > +	u64	start = fieinfo->fi_start;
> > +	u64	length = fieinfo->fi_len;
> > +	int	error;
> 
> Would be nice if the variable name indentation was consistent here, but
> otherwise the xfs part looks ok.

These fields are removed on the next patch, updating it is really required?
> 
> >  
> >  	xfs_ilock(XFS_I(inode), XFS_IOLOCK_SHARED);
> >  	if (fieinfo->fi_flags & FIEMAP_FLAG_XATTR) {
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index d5e7c744aea6..7b744b7de24e 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -1705,11 +1705,14 @@ extern bool may_open_dev(const struct path *path);
> >   * VFS FS_IOC_FIEMAP helper definitions.
> >   */
> >  struct fiemap_extent_info {
> > -	unsigned int fi_flags;		/* Flags as passed from user */
> > -	unsigned int fi_extents_mapped;	/* Number of mapped extents */
> > -	unsigned int fi_extents_max;	/* Size of fiemap_extent array */
> > -	struct fiemap_extent __user *fi_extents_start; /* Start of
> > -							fiemap_extent array */
> > +	unsigned int	fi_flags;		/* Flags as passed from user */
> > +	u64		fi_start;
> > +	u64		fi_len;
> 
> Comments for these two new fields?

Sure, how about this:

       u64           fi_start;            /* Logical offset at which
                                             start mapping */
       u64           fi_len;              /* Logical length of mapping
                                             the caller cares about */


btw, Above indentation won't match final result


Christoph, may I keep your reviewed tag by updating the comments as above?
Otherwise I'll just remove your tag

> 
> --D
> 
> > +	unsigned int	fi_extents_mapped;	/* Number of mapped extents */
> > +	unsigned int	fi_extents_max;		/* Size of fiemap_extent array */
> > +	struct		fiemap_extent __user *fi_extents_start;	/* Start of
> > +								   fiemap_extent
> > +								   array */
> >  };
> >  int fiemap_fill_next_extent(struct fiemap_extent_info *info, u64 logical,
> >  			    u64 phys, u64 len, u32 flags);
> > @@ -1841,8 +1844,7 @@ struct inode_operations {
> >  	int (*setattr) (struct dentry *, struct iattr *);
> >  	int (*getattr) (const struct path *, struct kstat *, u32, unsigned int);
> >  	ssize_t (*listxattr) (struct dentry *, char *, size_t);
> > -	int (*fiemap)(struct inode *, struct fiemap_extent_info *, u64 start,
> > -		      u64 len);
> > +	int (*fiemap)(struct inode *, struct fiemap_extent_info *);
> >  	int (*update_time)(struct inode *, struct timespec64 *, int);
> >  	int (*atomic_open)(struct inode *, struct dentry *,
> >  			   struct file *, unsigned open_flag,
> > @@ -3199,11 +3201,10 @@ extern int vfs_readlink(struct dentry *, char __user *, int);
> >  
> >  extern int __generic_block_fiemap(struct inode *inode,
> >  				  struct fiemap_extent_info *fieinfo,
> > -				  loff_t start, loff_t len,
> >  				  get_block_t *get_block);
> >  extern int generic_block_fiemap(struct inode *inode,
> > -				struct fiemap_extent_info *fieinfo, u64 start,
> > -				u64 len, get_block_t *get_block);
> > +				struct fiemap_extent_info *fieinfo,
> > +				get_block_t *get_block);
> >  
> >  extern struct file_system_type *get_filesystem(struct file_system_type *fs);
> >  extern void put_filesystem(struct file_system_type *fs);
> > -- 
> > 2.20.1
> > 

-- 
Carlos

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 5/9] fs: Move start and length fiemap fields into fiemap_extent_info
  2019-07-31 14:12 ` [PATCH 5/9] fs: Move start and length fiemap fields into fiemap_extent_info Carlos Maiolino
@ 2019-07-31 23:28   ` Darrick J. Wong
  2019-08-02  9:51     ` Carlos Maiolino
  0 siblings, 1 reply; 24+ messages in thread
From: Darrick J. Wong @ 2019-07-31 23:28 UTC (permalink / raw)
  To: Carlos Maiolino
  Cc: linux-fsdevel, hch, adilger, jaegeuk, miklos, rpeterso, linux-xfs

On Wed, Jul 31, 2019 at 04:12:41PM +0200, Carlos Maiolino wrote:
> As the overall goal to deprecate fibmap, Christoph suggested a rework of
> the ->fiemap API, in a way we could pass to it a callback to fill the
> fiemap structure (one of these callbacks being fiemap_fill_next_extent).
> 
> To avoid the need to add several fields into the ->fiemap method, aggregate
> everything into a single data structure, and pass it along.
> 
> This patch isn't suppose to add any functional change, only to update
> filesystems providing ->fiemap() method.
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---
>  fs/bad_inode.c        |  3 +--
>  fs/btrfs/inode.c      |  5 +++--
>  fs/ext2/ext2.h        |  3 +--
>  fs/ext2/inode.c       |  6 ++----
>  fs/ext4/ext4.h        |  3 +--
>  fs/ext4/extents.c     |  8 ++++----
>  fs/f2fs/data.c        |  5 +++--
>  fs/f2fs/f2fs.h        |  3 +--
>  fs/gfs2/inode.c       |  5 +++--
>  fs/hpfs/file.c        |  4 ++--
>  fs/ioctl.c            | 16 ++++++++++------
>  fs/nilfs2/inode.c     |  5 +++--
>  fs/nilfs2/nilfs.h     |  3 +--
>  fs/ocfs2/extent_map.c |  5 +++--
>  fs/ocfs2/extent_map.h |  3 +--
>  fs/overlayfs/inode.c  |  5 ++---
>  fs/xfs/xfs_iops.c     | 10 +++++-----
>  include/linux/fs.h    | 21 +++++++++++----------
>  18 files changed, 57 insertions(+), 56 deletions(-)
> 
> diff --git a/fs/bad_inode.c b/fs/bad_inode.c
> index 8035d2a44561..21dfaf876814 100644
> --- a/fs/bad_inode.c
> +++ b/fs/bad_inode.c
> @@ -120,8 +120,7 @@ static struct posix_acl *bad_inode_get_acl(struct inode *inode, int type)
>  }
>  
>  static int bad_inode_fiemap(struct inode *inode,
> -			    struct fiemap_extent_info *fieinfo, u64 start,
> -			    u64 len)
> +			    struct fiemap_extent_info *fieinfo)
>  {
>  	return -EIO;
>  }
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 82fdda8ff5ab..caa06a8ac767 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -8600,9 +8600,10 @@ static ssize_t btrfs_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
>  
>  #define BTRFS_FIEMAP_FLAGS	(FIEMAP_FLAG_SYNC)
>  
> -static int btrfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> -		__u64 start, __u64 len)
> +static int btrfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
>  {
> +	u64	start = fieinfo->fi_start;
> +	u64	len = fieinfo->fi_len;
>  	int	ret;
>  
>  	ret = fiemap_check_flags(fieinfo, BTRFS_FIEMAP_FLAGS);
> diff --git a/fs/ext2/ext2.h b/fs/ext2/ext2.h
> index 10ab238de9a6..284df1af9474 100644
> --- a/fs/ext2/ext2.h
> +++ b/fs/ext2/ext2.h
> @@ -760,8 +760,7 @@ extern int ext2_get_block(struct inode *, sector_t, struct buffer_head *, int);
>  extern int ext2_setattr (struct dentry *, struct iattr *);
>  extern int ext2_getattr (const struct path *, struct kstat *, u32, unsigned int);
>  extern void ext2_set_inode_flags(struct inode *inode);
> -extern int ext2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> -		       u64 start, u64 len);
> +extern int ext2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo);
>  
>  /* ioctl.c */
>  extern long ext2_ioctl(struct file *, unsigned int, unsigned long);
> diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
> index c27c27300d95..267392335f38 100644
> --- a/fs/ext2/inode.c
> +++ b/fs/ext2/inode.c
> @@ -855,11 +855,9 @@ const struct iomap_ops ext2_iomap_ops = {
>  const struct iomap_ops ext2_iomap_ops;
>  #endif /* CONFIG_FS_DAX */
>  
> -int ext2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> -		u64 start, u64 len)
> +int ext2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
>  {
> -	return generic_block_fiemap(inode, fieinfo, start, len,
> -				    ext2_get_block);
> +	return generic_block_fiemap(inode, fieinfo, ext2_get_block);
>  }
>  
>  static int ext2_writepage(struct page *page, struct writeback_control *wbc)
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 82ffdacdc7fa..e4cb40b5893b 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -3154,8 +3154,7 @@ extern struct ext4_ext_path *ext4_find_extent(struct inode *, ext4_lblk_t,
>  extern void ext4_ext_drop_refs(struct ext4_ext_path *);
>  extern int ext4_ext_check_inode(struct inode *inode);
>  extern ext4_lblk_t ext4_ext_next_allocated_block(struct ext4_ext_path *path);
> -extern int ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> -			__u64 start, __u64 len);
> +extern int ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo);
>  extern int ext4_ext_precache(struct inode *inode);
>  extern int ext4_collapse_range(struct inode *inode, loff_t offset, loff_t len);
>  extern int ext4_insert_range(struct inode *inode, loff_t offset, loff_t len);
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 0f89f5190cd7..436e564ebdd6 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -5038,9 +5038,10 @@ static int ext4_xattr_fiemap(struct inode *inode,
>  	return (error < 0 ? error : 0);
>  }
>  
> -int ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> -		__u64 start, __u64 len)
> +int ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
>  {
> +	u64 start = fieinfo->fi_start;
> +	u64 len = fieinfo->fi_len;
>  	ext4_lblk_t start_blk;
>  	int error = 0;
>  
> @@ -5062,8 +5063,7 @@ int ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>  
>  	/* fallback to generic here if not in extents fmt */
>  	if (!(ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)))
> -		return generic_block_fiemap(inode, fieinfo, start, len,
> -			ext4_get_block);
> +		return generic_block_fiemap(inode, fieinfo, ext4_get_block);
>  
>  	if (fiemap_check_flags(fieinfo, EXT4_FIEMAP_FLAGS))
>  		return -EBADR;
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 9727944139f2..2979ca40d192 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1409,9 +1409,10 @@ static int f2fs_xattr_fiemap(struct inode *inode,
>  	return (err < 0 ? err : 0);
>  }
>  
> -int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> -		u64 start, u64 len)
> +int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
>  {
> +	u64 start = fieinfo->fi_start;
> +	u64 len = fieinfo->fi_len;
>  	struct buffer_head map_bh;
>  	sector_t start_blk, last_blk;
>  	pgoff_t next_pgofs;
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 87f75ebd2fd6..fb33809c2552 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -3155,8 +3155,7 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio);
>  void __do_map_lock(struct f2fs_sb_info *sbi, int flag, bool lock);
>  int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
>  			int create, int flag);
> -int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> -			u64 start, u64 len);
> +int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo);
>  bool f2fs_should_update_inplace(struct inode *inode, struct f2fs_io_info *fio);
>  bool f2fs_should_update_outplace(struct inode *inode, struct f2fs_io_info *fio);
>  void f2fs_invalidate_page(struct page *page, unsigned int offset,
> diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
> index 998051c4aea7..5e84d5963506 100644
> --- a/fs/gfs2/inode.c
> +++ b/fs/gfs2/inode.c
> @@ -2004,9 +2004,10 @@ static int gfs2_getattr(const struct path *path, struct kstat *stat,
>  	return 0;
>  }
>  
> -static int gfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> -		       u64 start, u64 len)
> +static int gfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
>  {
> +	u64 start = fieinfo->fi_start;
> +	u64 len = fieinfo->fi_len;
>  	struct gfs2_inode *ip = GFS2_I(inode);
>  	struct gfs2_holder gh;
>  	int ret;
> diff --git a/fs/hpfs/file.c b/fs/hpfs/file.c
> index 1ecec124e76f..0eece4ae1f11 100644
> --- a/fs/hpfs/file.c
> +++ b/fs/hpfs/file.c
> @@ -190,9 +190,9 @@ static sector_t _hpfs_bmap(struct address_space *mapping, sector_t block)
>  	return generic_block_bmap(mapping, block, hpfs_get_block);
>  }
>  
> -static int hpfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo, u64 start, u64 len)
> +static int hpfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
>  {
> -	return generic_block_fiemap(inode, fieinfo, start, len, hpfs_get_block);
> +	return generic_block_fiemap(inode, fieinfo, hpfs_get_block);
>  }
>  
>  const struct address_space_operations hpfs_aops = {
> diff --git a/fs/ioctl.c b/fs/ioctl.c
> index 6b589c873bc2..ad8edcb10dc9 100644
> --- a/fs/ioctl.c
> +++ b/fs/ioctl.c
> @@ -210,6 +210,8 @@ static int ioctl_fiemap(struct file *filp, unsigned long arg)
>  	fieinfo.fi_flags = fiemap.fm_flags;
>  	fieinfo.fi_extents_max = fiemap.fm_extent_count;
>  	fieinfo.fi_extents_start = ufiemap->fm_extents;
> +	fieinfo.fi_start = fiemap.fm_start;
> +	fieinfo.fi_len = len;
>  
>  	if (fiemap.fm_extent_count != 0 &&
>  	    !access_ok(fieinfo.fi_extents_start,
> @@ -219,7 +221,7 @@ static int ioctl_fiemap(struct file *filp, unsigned long arg)
>  	if (fieinfo.fi_flags & FIEMAP_FLAG_SYNC)
>  		filemap_write_and_wait(inode->i_mapping);
>  
> -	error = inode->i_op->fiemap(inode, &fieinfo, fiemap.fm_start, len);
> +	error = inode->i_op->fiemap(inode, &fieinfo);
>  	fiemap.fm_flags = fieinfo.fi_flags;
>  	fiemap.fm_mapped_extents = fieinfo.fi_extents_mapped;
>  	if (copy_to_user(ufiemap, &fiemap, sizeof(fiemap)))
> @@ -296,9 +298,11 @@ static inline loff_t blk_to_logical(struct inode *inode, sector_t blk)
>   */
>  
>  int __generic_block_fiemap(struct inode *inode,
> -			   struct fiemap_extent_info *fieinfo, loff_t start,
> -			   loff_t len, get_block_t *get_block)
> +			   struct fiemap_extent_info *fieinfo,
> +			   get_block_t *get_block)
>  {
> +	loff_t start = fieinfo->fi_start;
> +	loff_t len = fieinfo->fi_len;
>  	struct buffer_head map_bh;
>  	sector_t start_blk, last_blk;
>  	loff_t isize = i_size_read(inode);
> @@ -455,12 +459,12 @@ EXPORT_SYMBOL(__generic_block_fiemap);
>   */
>  
>  int generic_block_fiemap(struct inode *inode,
> -			 struct fiemap_extent_info *fieinfo, u64 start,
> -			 u64 len, get_block_t *get_block)
> +			 struct fiemap_extent_info *fieinfo,
> +			 get_block_t *get_block)
>  {
>  	int ret;
>  	inode_lock(inode);
> -	ret = __generic_block_fiemap(inode, fieinfo, start, len, get_block);
> +	ret = __generic_block_fiemap(inode, fieinfo, get_block);
>  	inode_unlock(inode);
>  	return ret;
>  }
> diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
> index 671085512e0f..1f37d086371c 100644
> --- a/fs/nilfs2/inode.c
> +++ b/fs/nilfs2/inode.c
> @@ -992,9 +992,10 @@ void nilfs_dirty_inode(struct inode *inode, int flags)
>  	nilfs_transaction_commit(inode->i_sb); /* never fails */
>  }
>  
> -int nilfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> -		 __u64 start, __u64 len)
> +int nilfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
>  {
> +	u64 start = fieinfo->fi_start;
> +	u64 len = fieinfo->fi_len;
>  	struct the_nilfs *nilfs = inode->i_sb->s_fs_info;
>  	__u64 logical = 0, phys = 0, size = 0;
>  	__u32 flags = 0;
> diff --git a/fs/nilfs2/nilfs.h b/fs/nilfs2/nilfs.h
> index a2f247b6a209..55d1307ed710 100644
> --- a/fs/nilfs2/nilfs.h
> +++ b/fs/nilfs2/nilfs.h
> @@ -276,8 +276,7 @@ extern int nilfs_inode_dirty(struct inode *);
>  int nilfs_set_file_dirty(struct inode *inode, unsigned int nr_dirty);
>  extern int __nilfs_mark_inode_dirty(struct inode *, int);
>  extern void nilfs_dirty_inode(struct inode *, int flags);
> -int nilfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> -		 __u64 start, __u64 len);
> +int nilfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo);
>  static inline int nilfs_mark_inode_dirty(struct inode *inode)
>  {
>  	return __nilfs_mark_inode_dirty(inode, I_DIRTY);
> diff --git a/fs/ocfs2/extent_map.c b/fs/ocfs2/extent_map.c
> index 06cb96462bf9..e01fd38ea935 100644
> --- a/fs/ocfs2/extent_map.c
> +++ b/fs/ocfs2/extent_map.c
> @@ -749,8 +749,7 @@ static int ocfs2_fiemap_inline(struct inode *inode, struct buffer_head *di_bh,
>  
>  #define OCFS2_FIEMAP_FLAGS	(FIEMAP_FLAG_SYNC)
>  
> -int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> -		 u64 map_start, u64 map_len)
> +int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
>  {
>  	int ret, is_last;
>  	u32 mapping_end, cpos;
> @@ -759,6 +758,8 @@ int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>  	u64 len_bytes, phys_bytes, virt_bytes;
>  	struct buffer_head *di_bh = NULL;
>  	struct ocfs2_extent_rec rec;
> +	u64 map_start = fieinfo->fi_start;
> +	u64 map_len = fieinfo->fi_len;
>  
>  	ret = fiemap_check_flags(fieinfo, OCFS2_FIEMAP_FLAGS);
>  	if (ret)
> diff --git a/fs/ocfs2/extent_map.h b/fs/ocfs2/extent_map.h
> index 1057586ec19f..793be96099c0 100644
> --- a/fs/ocfs2/extent_map.h
> +++ b/fs/ocfs2/extent_map.h
> @@ -50,8 +50,7 @@ int ocfs2_get_clusters(struct inode *inode, u32 v_cluster, u32 *p_cluster,
>  int ocfs2_extent_map_get_blocks(struct inode *inode, u64 v_blkno, u64 *p_blkno,
>  				u64 *ret_count, unsigned int *extent_flags);
>  
> -int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> -		 u64 map_start, u64 map_len);
> +int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo);
>  
>  int ocfs2_overwrite_io(struct inode *inode, struct buffer_head *di_bh,
>  		       u64 map_start, u64 map_len);
> diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
> index 3b7ed5d2279c..6f00b0ef6b43 100644
> --- a/fs/overlayfs/inode.c
> +++ b/fs/overlayfs/inode.c
> @@ -456,8 +456,7 @@ int ovl_update_time(struct inode *inode, struct timespec64 *ts, int flags)
>  	return 0;
>  }
>  
> -static int ovl_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> -		      u64 start, u64 len)
> +static int ovl_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
>  {
>  	int err;
>  	struct inode *realinode = ovl_inode_real(inode);
> @@ -471,7 +470,7 @@ static int ovl_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>  	if (fieinfo->fi_flags & FIEMAP_FLAG_SYNC)
>  		filemap_write_and_wait(realinode->i_mapping);
>  
> -	err = realinode->i_op->fiemap(realinode, fieinfo, start, len);
> +	err = realinode->i_op->fiemap(realinode, fieinfo);
>  	revert_creds(old_cred);
>  
>  	return err;
> diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> index 74047bd0c1ae..1f4354fa989b 100644
> --- a/fs/xfs/xfs_iops.c
> +++ b/fs/xfs/xfs_iops.c
> @@ -1109,12 +1109,12 @@ xfs_vn_update_time(
>  
>  STATIC int
>  xfs_vn_fiemap(
> -	struct inode		*inode,
> -	struct fiemap_extent_info *fieinfo,
> -	u64			start,
> -	u64			length)
> +	struct inode		  *inode,
> +	struct fiemap_extent_info *fieinfo)
>  {
> -	int			error;
> +	u64	start = fieinfo->fi_start;
> +	u64	length = fieinfo->fi_len;
> +	int	error;

Would be nice if the variable name indentation was consistent here, but
otherwise the xfs part looks ok.

>  
>  	xfs_ilock(XFS_I(inode), XFS_IOLOCK_SHARED);
>  	if (fieinfo->fi_flags & FIEMAP_FLAG_XATTR) {
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index d5e7c744aea6..7b744b7de24e 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1705,11 +1705,14 @@ extern bool may_open_dev(const struct path *path);
>   * VFS FS_IOC_FIEMAP helper definitions.
>   */
>  struct fiemap_extent_info {
> -	unsigned int fi_flags;		/* Flags as passed from user */
> -	unsigned int fi_extents_mapped;	/* Number of mapped extents */
> -	unsigned int fi_extents_max;	/* Size of fiemap_extent array */
> -	struct fiemap_extent __user *fi_extents_start; /* Start of
> -							fiemap_extent array */
> +	unsigned int	fi_flags;		/* Flags as passed from user */
> +	u64		fi_start;
> +	u64		fi_len;

Comments for these two new fields?

--D

> +	unsigned int	fi_extents_mapped;	/* Number of mapped extents */
> +	unsigned int	fi_extents_max;		/* Size of fiemap_extent array */
> +	struct		fiemap_extent __user *fi_extents_start;	/* Start of
> +								   fiemap_extent
> +								   array */
>  };
>  int fiemap_fill_next_extent(struct fiemap_extent_info *info, u64 logical,
>  			    u64 phys, u64 len, u32 flags);
> @@ -1841,8 +1844,7 @@ struct inode_operations {
>  	int (*setattr) (struct dentry *, struct iattr *);
>  	int (*getattr) (const struct path *, struct kstat *, u32, unsigned int);
>  	ssize_t (*listxattr) (struct dentry *, char *, size_t);
> -	int (*fiemap)(struct inode *, struct fiemap_extent_info *, u64 start,
> -		      u64 len);
> +	int (*fiemap)(struct inode *, struct fiemap_extent_info *);
>  	int (*update_time)(struct inode *, struct timespec64 *, int);
>  	int (*atomic_open)(struct inode *, struct dentry *,
>  			   struct file *, unsigned open_flag,
> @@ -3199,11 +3201,10 @@ extern int vfs_readlink(struct dentry *, char __user *, int);
>  
>  extern int __generic_block_fiemap(struct inode *inode,
>  				  struct fiemap_extent_info *fieinfo,
> -				  loff_t start, loff_t len,
>  				  get_block_t *get_block);
>  extern int generic_block_fiemap(struct inode *inode,
> -				struct fiemap_extent_info *fieinfo, u64 start,
> -				u64 len, get_block_t *get_block);
> +				struct fiemap_extent_info *fieinfo,
> +				get_block_t *get_block);
>  
>  extern struct file_system_type *get_filesystem(struct file_system_type *fs);
>  extern void put_filesystem(struct file_system_type *fs);
> -- 
> 2.20.1
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 5/9] fs: Move start and length fiemap fields into fiemap_extent_info
  2019-07-31 14:12 [PATCH 0/9 V4] New ->fiemap infrastructure and ->bmap removal Carlos Maiolino
@ 2019-07-31 14:12 ` Carlos Maiolino
  2019-07-31 23:28   ` Darrick J. Wong
  0 siblings, 1 reply; 24+ messages in thread
From: Carlos Maiolino @ 2019-07-31 14:12 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: hch, adilger, jaegeuk, darrick.wong, miklos, rpeterso, linux-xfs

As the overall goal to deprecate fibmap, Christoph suggested a rework of
the ->fiemap API, in a way we could pass to it a callback to fill the
fiemap structure (one of these callbacks being fiemap_fill_next_extent).

To avoid the need to add several fields into the ->fiemap method, aggregate
everything into a single data structure, and pass it along.

This patch isn't suppose to add any functional change, only to update
filesystems providing ->fiemap() method.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/bad_inode.c        |  3 +--
 fs/btrfs/inode.c      |  5 +++--
 fs/ext2/ext2.h        |  3 +--
 fs/ext2/inode.c       |  6 ++----
 fs/ext4/ext4.h        |  3 +--
 fs/ext4/extents.c     |  8 ++++----
 fs/f2fs/data.c        |  5 +++--
 fs/f2fs/f2fs.h        |  3 +--
 fs/gfs2/inode.c       |  5 +++--
 fs/hpfs/file.c        |  4 ++--
 fs/ioctl.c            | 16 ++++++++++------
 fs/nilfs2/inode.c     |  5 +++--
 fs/nilfs2/nilfs.h     |  3 +--
 fs/ocfs2/extent_map.c |  5 +++--
 fs/ocfs2/extent_map.h |  3 +--
 fs/overlayfs/inode.c  |  5 ++---
 fs/xfs/xfs_iops.c     | 10 +++++-----
 include/linux/fs.h    | 21 +++++++++++----------
 18 files changed, 57 insertions(+), 56 deletions(-)

diff --git a/fs/bad_inode.c b/fs/bad_inode.c
index 8035d2a44561..21dfaf876814 100644
--- a/fs/bad_inode.c
+++ b/fs/bad_inode.c
@@ -120,8 +120,7 @@ static struct posix_acl *bad_inode_get_acl(struct inode *inode, int type)
 }
 
 static int bad_inode_fiemap(struct inode *inode,
-			    struct fiemap_extent_info *fieinfo, u64 start,
-			    u64 len)
+			    struct fiemap_extent_info *fieinfo)
 {
 	return -EIO;
 }
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 82fdda8ff5ab..caa06a8ac767 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -8600,9 +8600,10 @@ static ssize_t btrfs_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
 
 #define BTRFS_FIEMAP_FLAGS	(FIEMAP_FLAG_SYNC)
 
-static int btrfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-		__u64 start, __u64 len)
+static int btrfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
 {
+	u64	start = fieinfo->fi_start;
+	u64	len = fieinfo->fi_len;
 	int	ret;
 
 	ret = fiemap_check_flags(fieinfo, BTRFS_FIEMAP_FLAGS);
diff --git a/fs/ext2/ext2.h b/fs/ext2/ext2.h
index 10ab238de9a6..284df1af9474 100644
--- a/fs/ext2/ext2.h
+++ b/fs/ext2/ext2.h
@@ -760,8 +760,7 @@ extern int ext2_get_block(struct inode *, sector_t, struct buffer_head *, int);
 extern int ext2_setattr (struct dentry *, struct iattr *);
 extern int ext2_getattr (const struct path *, struct kstat *, u32, unsigned int);
 extern void ext2_set_inode_flags(struct inode *inode);
-extern int ext2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-		       u64 start, u64 len);
+extern int ext2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo);
 
 /* ioctl.c */
 extern long ext2_ioctl(struct file *, unsigned int, unsigned long);
diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
index c27c27300d95..267392335f38 100644
--- a/fs/ext2/inode.c
+++ b/fs/ext2/inode.c
@@ -855,11 +855,9 @@ const struct iomap_ops ext2_iomap_ops = {
 const struct iomap_ops ext2_iomap_ops;
 #endif /* CONFIG_FS_DAX */
 
-int ext2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-		u64 start, u64 len)
+int ext2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
 {
-	return generic_block_fiemap(inode, fieinfo, start, len,
-				    ext2_get_block);
+	return generic_block_fiemap(inode, fieinfo, ext2_get_block);
 }
 
 static int ext2_writepage(struct page *page, struct writeback_control *wbc)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 82ffdacdc7fa..e4cb40b5893b 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -3154,8 +3154,7 @@ extern struct ext4_ext_path *ext4_find_extent(struct inode *, ext4_lblk_t,
 extern void ext4_ext_drop_refs(struct ext4_ext_path *);
 extern int ext4_ext_check_inode(struct inode *inode);
 extern ext4_lblk_t ext4_ext_next_allocated_block(struct ext4_ext_path *path);
-extern int ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-			__u64 start, __u64 len);
+extern int ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo);
 extern int ext4_ext_precache(struct inode *inode);
 extern int ext4_collapse_range(struct inode *inode, loff_t offset, loff_t len);
 extern int ext4_insert_range(struct inode *inode, loff_t offset, loff_t len);
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 0f89f5190cd7..436e564ebdd6 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -5038,9 +5038,10 @@ static int ext4_xattr_fiemap(struct inode *inode,
 	return (error < 0 ? error : 0);
 }
 
-int ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-		__u64 start, __u64 len)
+int ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
 {
+	u64 start = fieinfo->fi_start;
+	u64 len = fieinfo->fi_len;
 	ext4_lblk_t start_blk;
 	int error = 0;
 
@@ -5062,8 +5063,7 @@ int ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 
 	/* fallback to generic here if not in extents fmt */
 	if (!(ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)))
-		return generic_block_fiemap(inode, fieinfo, start, len,
-			ext4_get_block);
+		return generic_block_fiemap(inode, fieinfo, ext4_get_block);
 
 	if (fiemap_check_flags(fieinfo, EXT4_FIEMAP_FLAGS))
 		return -EBADR;
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 9727944139f2..2979ca40d192 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1409,9 +1409,10 @@ static int f2fs_xattr_fiemap(struct inode *inode,
 	return (err < 0 ? err : 0);
 }
 
-int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-		u64 start, u64 len)
+int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
 {
+	u64 start = fieinfo->fi_start;
+	u64 len = fieinfo->fi_len;
 	struct buffer_head map_bh;
 	sector_t start_blk, last_blk;
 	pgoff_t next_pgofs;
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 87f75ebd2fd6..fb33809c2552 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -3155,8 +3155,7 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio);
 void __do_map_lock(struct f2fs_sb_info *sbi, int flag, bool lock);
 int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
 			int create, int flag);
-int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-			u64 start, u64 len);
+int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo);
 bool f2fs_should_update_inplace(struct inode *inode, struct f2fs_io_info *fio);
 bool f2fs_should_update_outplace(struct inode *inode, struct f2fs_io_info *fio);
 void f2fs_invalidate_page(struct page *page, unsigned int offset,
diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index 998051c4aea7..5e84d5963506 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -2004,9 +2004,10 @@ static int gfs2_getattr(const struct path *path, struct kstat *stat,
 	return 0;
 }
 
-static int gfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-		       u64 start, u64 len)
+static int gfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
 {
+	u64 start = fieinfo->fi_start;
+	u64 len = fieinfo->fi_len;
 	struct gfs2_inode *ip = GFS2_I(inode);
 	struct gfs2_holder gh;
 	int ret;
diff --git a/fs/hpfs/file.c b/fs/hpfs/file.c
index 1ecec124e76f..0eece4ae1f11 100644
--- a/fs/hpfs/file.c
+++ b/fs/hpfs/file.c
@@ -190,9 +190,9 @@ static sector_t _hpfs_bmap(struct address_space *mapping, sector_t block)
 	return generic_block_bmap(mapping, block, hpfs_get_block);
 }
 
-static int hpfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo, u64 start, u64 len)
+static int hpfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
 {
-	return generic_block_fiemap(inode, fieinfo, start, len, hpfs_get_block);
+	return generic_block_fiemap(inode, fieinfo, hpfs_get_block);
 }
 
 const struct address_space_operations hpfs_aops = {
diff --git a/fs/ioctl.c b/fs/ioctl.c
index 6b589c873bc2..ad8edcb10dc9 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -210,6 +210,8 @@ static int ioctl_fiemap(struct file *filp, unsigned long arg)
 	fieinfo.fi_flags = fiemap.fm_flags;
 	fieinfo.fi_extents_max = fiemap.fm_extent_count;
 	fieinfo.fi_extents_start = ufiemap->fm_extents;
+	fieinfo.fi_start = fiemap.fm_start;
+	fieinfo.fi_len = len;
 
 	if (fiemap.fm_extent_count != 0 &&
 	    !access_ok(fieinfo.fi_extents_start,
@@ -219,7 +221,7 @@ static int ioctl_fiemap(struct file *filp, unsigned long arg)
 	if (fieinfo.fi_flags & FIEMAP_FLAG_SYNC)
 		filemap_write_and_wait(inode->i_mapping);
 
-	error = inode->i_op->fiemap(inode, &fieinfo, fiemap.fm_start, len);
+	error = inode->i_op->fiemap(inode, &fieinfo);
 	fiemap.fm_flags = fieinfo.fi_flags;
 	fiemap.fm_mapped_extents = fieinfo.fi_extents_mapped;
 	if (copy_to_user(ufiemap, &fiemap, sizeof(fiemap)))
@@ -296,9 +298,11 @@ static inline loff_t blk_to_logical(struct inode *inode, sector_t blk)
  */
 
 int __generic_block_fiemap(struct inode *inode,
-			   struct fiemap_extent_info *fieinfo, loff_t start,
-			   loff_t len, get_block_t *get_block)
+			   struct fiemap_extent_info *fieinfo,
+			   get_block_t *get_block)
 {
+	loff_t start = fieinfo->fi_start;
+	loff_t len = fieinfo->fi_len;
 	struct buffer_head map_bh;
 	sector_t start_blk, last_blk;
 	loff_t isize = i_size_read(inode);
@@ -455,12 +459,12 @@ EXPORT_SYMBOL(__generic_block_fiemap);
  */
 
 int generic_block_fiemap(struct inode *inode,
-			 struct fiemap_extent_info *fieinfo, u64 start,
-			 u64 len, get_block_t *get_block)
+			 struct fiemap_extent_info *fieinfo,
+			 get_block_t *get_block)
 {
 	int ret;
 	inode_lock(inode);
-	ret = __generic_block_fiemap(inode, fieinfo, start, len, get_block);
+	ret = __generic_block_fiemap(inode, fieinfo, get_block);
 	inode_unlock(inode);
 	return ret;
 }
diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
index 671085512e0f..1f37d086371c 100644
--- a/fs/nilfs2/inode.c
+++ b/fs/nilfs2/inode.c
@@ -992,9 +992,10 @@ void nilfs_dirty_inode(struct inode *inode, int flags)
 	nilfs_transaction_commit(inode->i_sb); /* never fails */
 }
 
-int nilfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-		 __u64 start, __u64 len)
+int nilfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
 {
+	u64 start = fieinfo->fi_start;
+	u64 len = fieinfo->fi_len;
 	struct the_nilfs *nilfs = inode->i_sb->s_fs_info;
 	__u64 logical = 0, phys = 0, size = 0;
 	__u32 flags = 0;
diff --git a/fs/nilfs2/nilfs.h b/fs/nilfs2/nilfs.h
index a2f247b6a209..55d1307ed710 100644
--- a/fs/nilfs2/nilfs.h
+++ b/fs/nilfs2/nilfs.h
@@ -276,8 +276,7 @@ extern int nilfs_inode_dirty(struct inode *);
 int nilfs_set_file_dirty(struct inode *inode, unsigned int nr_dirty);
 extern int __nilfs_mark_inode_dirty(struct inode *, int);
 extern void nilfs_dirty_inode(struct inode *, int flags);
-int nilfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-		 __u64 start, __u64 len);
+int nilfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo);
 static inline int nilfs_mark_inode_dirty(struct inode *inode)
 {
 	return __nilfs_mark_inode_dirty(inode, I_DIRTY);
diff --git a/fs/ocfs2/extent_map.c b/fs/ocfs2/extent_map.c
index 06cb96462bf9..e01fd38ea935 100644
--- a/fs/ocfs2/extent_map.c
+++ b/fs/ocfs2/extent_map.c
@@ -749,8 +749,7 @@ static int ocfs2_fiemap_inline(struct inode *inode, struct buffer_head *di_bh,
 
 #define OCFS2_FIEMAP_FLAGS	(FIEMAP_FLAG_SYNC)
 
-int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-		 u64 map_start, u64 map_len)
+int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
 {
 	int ret, is_last;
 	u32 mapping_end, cpos;
@@ -759,6 +758,8 @@ int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 	u64 len_bytes, phys_bytes, virt_bytes;
 	struct buffer_head *di_bh = NULL;
 	struct ocfs2_extent_rec rec;
+	u64 map_start = fieinfo->fi_start;
+	u64 map_len = fieinfo->fi_len;
 
 	ret = fiemap_check_flags(fieinfo, OCFS2_FIEMAP_FLAGS);
 	if (ret)
diff --git a/fs/ocfs2/extent_map.h b/fs/ocfs2/extent_map.h
index 1057586ec19f..793be96099c0 100644
--- a/fs/ocfs2/extent_map.h
+++ b/fs/ocfs2/extent_map.h
@@ -50,8 +50,7 @@ int ocfs2_get_clusters(struct inode *inode, u32 v_cluster, u32 *p_cluster,
 int ocfs2_extent_map_get_blocks(struct inode *inode, u64 v_blkno, u64 *p_blkno,
 				u64 *ret_count, unsigned int *extent_flags);
 
-int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-		 u64 map_start, u64 map_len);
+int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo);
 
 int ocfs2_overwrite_io(struct inode *inode, struct buffer_head *di_bh,
 		       u64 map_start, u64 map_len);
diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 3b7ed5d2279c..6f00b0ef6b43 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -456,8 +456,7 @@ int ovl_update_time(struct inode *inode, struct timespec64 *ts, int flags)
 	return 0;
 }
 
-static int ovl_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-		      u64 start, u64 len)
+static int ovl_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
 {
 	int err;
 	struct inode *realinode = ovl_inode_real(inode);
@@ -471,7 +470,7 @@ static int ovl_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 	if (fieinfo->fi_flags & FIEMAP_FLAG_SYNC)
 		filemap_write_and_wait(realinode->i_mapping);
 
-	err = realinode->i_op->fiemap(realinode, fieinfo, start, len);
+	err = realinode->i_op->fiemap(realinode, fieinfo);
 	revert_creds(old_cred);
 
 	return err;
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index 74047bd0c1ae..1f4354fa989b 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -1109,12 +1109,12 @@ xfs_vn_update_time(
 
 STATIC int
 xfs_vn_fiemap(
-	struct inode		*inode,
-	struct fiemap_extent_info *fieinfo,
-	u64			start,
-	u64			length)
+	struct inode		  *inode,
+	struct fiemap_extent_info *fieinfo)
 {
-	int			error;
+	u64	start = fieinfo->fi_start;
+	u64	length = fieinfo->fi_len;
+	int	error;
 
 	xfs_ilock(XFS_I(inode), XFS_IOLOCK_SHARED);
 	if (fieinfo->fi_flags & FIEMAP_FLAG_XATTR) {
diff --git a/include/linux/fs.h b/include/linux/fs.h
index d5e7c744aea6..7b744b7de24e 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1705,11 +1705,14 @@ extern bool may_open_dev(const struct path *path);
  * VFS FS_IOC_FIEMAP helper definitions.
  */
 struct fiemap_extent_info {
-	unsigned int fi_flags;		/* Flags as passed from user */
-	unsigned int fi_extents_mapped;	/* Number of mapped extents */
-	unsigned int fi_extents_max;	/* Size of fiemap_extent array */
-	struct fiemap_extent __user *fi_extents_start; /* Start of
-							fiemap_extent array */
+	unsigned int	fi_flags;		/* Flags as passed from user */
+	u64		fi_start;
+	u64		fi_len;
+	unsigned int	fi_extents_mapped;	/* Number of mapped extents */
+	unsigned int	fi_extents_max;		/* Size of fiemap_extent array */
+	struct		fiemap_extent __user *fi_extents_start;	/* Start of
+								   fiemap_extent
+								   array */
 };
 int fiemap_fill_next_extent(struct fiemap_extent_info *info, u64 logical,
 			    u64 phys, u64 len, u32 flags);
@@ -1841,8 +1844,7 @@ struct inode_operations {
 	int (*setattr) (struct dentry *, struct iattr *);
 	int (*getattr) (const struct path *, struct kstat *, u32, unsigned int);
 	ssize_t (*listxattr) (struct dentry *, char *, size_t);
-	int (*fiemap)(struct inode *, struct fiemap_extent_info *, u64 start,
-		      u64 len);
+	int (*fiemap)(struct inode *, struct fiemap_extent_info *);
 	int (*update_time)(struct inode *, struct timespec64 *, int);
 	int (*atomic_open)(struct inode *, struct dentry *,
 			   struct file *, unsigned open_flag,
@@ -3199,11 +3201,10 @@ extern int vfs_readlink(struct dentry *, char __user *, int);
 
 extern int __generic_block_fiemap(struct inode *inode,
 				  struct fiemap_extent_info *fieinfo,
-				  loff_t start, loff_t len,
 				  get_block_t *get_block);
 extern int generic_block_fiemap(struct inode *inode,
-				struct fiemap_extent_info *fieinfo, u64 start,
-				u64 len, get_block_t *get_block);
+				struct fiemap_extent_info *fieinfo,
+				get_block_t *get_block);
 
 extern struct file_system_type *get_filesystem(struct file_system_type *fs);
 extern void put_filesystem(struct file_system_type *fs);
-- 
2.20.1

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, back to index

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-11 13:43 [PATCH 0/9 V6] New ->fiemap infrastructure and ->bmap removal Carlos Maiolino
2019-09-11 13:43 ` [PATCH 1/9] fs: Enable bmap() function to properly return errors Carlos Maiolino
2019-09-11 13:43 ` [PATCH 2/9] cachefiles: drop direct usage of ->bmap method Carlos Maiolino
2019-09-11 13:43 ` [PATCH 3/9] ecryptfs: drop direct calls to ->bmap Carlos Maiolino
2019-09-11 13:43 ` [PATCH 4/9] fibmap: Use bmap instead of ->bmap method in ioctl_fibmap Carlos Maiolino
2019-09-11 13:43 ` [PATCH 5/9] fs: Move start and length fiemap fields into fiemap_extent_info Carlos Maiolino
2019-09-16 17:42   ` Darrick J. Wong
2019-09-11 13:43 ` [PATCH 6/9] iomap: Remove length and start fields from iomap_fiemap Carlos Maiolino
2019-09-11 13:43 ` [PATCH 7/9] fiemap: Use a callback to fill fiemap extents Carlos Maiolino
2019-09-11 13:43 ` [PATCH 8/9] Use FIEMAP for FIBMAP calls Carlos Maiolino
2019-09-16 17:44   ` Darrick J. Wong
2019-09-11 13:43 ` [PATCH 9/9] xfs: Get rid of ->bmap Carlos Maiolino
2019-09-16 17:50   ` Darrick J. Wong
2019-09-18  8:13     ` Carlos Maiolino
2019-09-18 13:24       ` Christoph Hellwig
2019-09-18 16:12         ` Darrick J. Wong
  -- strict thread matches above, loose matches on Subject: below --
2019-08-08  8:27 [PATCH 0/9 V5] New ->fiemap infrastructure and ->bmap removal Carlos Maiolino
2019-08-08  8:27 ` [PATCH 5/9] fs: Move start and length fiemap fields into fiemap_extent_info Carlos Maiolino
2019-08-08 20:21   ` kbuild test robot
2019-07-31 14:12 [PATCH 0/9 V4] New ->fiemap infrastructure and ->bmap removal Carlos Maiolino
2019-07-31 14:12 ` [PATCH 5/9] fs: Move start and length fiemap fields into fiemap_extent_info Carlos Maiolino
2019-07-31 23:28   ` Darrick J. Wong
2019-08-02  9:51     ` Carlos Maiolino
2019-08-02 15:15       ` Darrick J. Wong
2019-08-05  9:40         ` Carlos Maiolino
2019-08-06  5:39       ` Christoph Hellwig

Linux-XFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-xfs/0 linux-xfs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-xfs linux-xfs/ https://lore.kernel.org/linux-xfs \
		linux-xfs@vger.kernel.org linux-xfs@archiver.kernel.org
	public-inbox-index linux-xfs


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-xfs


AGPL code for this site: git clone https://public-inbox.org/ public-inbox