[PATCH 00/20] New ->fiemap infrastructure and ->bmap removal

[PATCH 00/20] New ->fiemap infrastructure and ->bmap removal

[Carlos Maiolino]

Hi.

This the first version of the complete series with the goal to remove ->bmap
interface completely, in lieu of FIEMAP.

The first 3 patches of this series has been previously posted, and the following
patches is a rework based on a previous RFC, and some suggestions from
Christoph, regarding a new interface for ->fiemap.

I tried to split the patches in a way to avoid a build breakage between any
patch, and also, tried to split individual filesystem changes on individual
patches as possible.

I'd like to raise attention specifically to Btrfs case, which is the only
filesystem who access fiemap_extent_info structure directly, so, it couldn't be
completely reworked (maybe if I had used a different organization of the
patches), As is pointed out in patch description of patch:
"Use FIEMAP for FIBMAP calls", there is room for several code structure
optimizations there, but the functionality is already there, and believe it is
way better to post it now, and discuss the design, than keep working on several
micro improvements when the design isn't decided.

I'd love to hear what you guys have to say, suggestions, comments, etc.

I'd like to also mention that, there are some secondary goals regarding this
patchset, like the ability to detect overflow in FIBMAP for example, which I
didn't forget, but I don't think it adds anything interesting to the overall
design, of the new structure of ->fiemap and removal of ->bmap, but I'll work on
that, once the design is decided and implemented.

I've tested these patches exhaustively on Ext4 and XFS (which are the
filesystems I'm mostly familiar with), and did some testing on Btrfs due its
peculiar usage of fiemap_extent_info directly, and everything is properly
working, including survival of several xfstests runs.

Thanks in advance for any comments.

Carlos Maiolino (20):
  fs: Enable bmap() function to properly return errors
  cachefiles: drop direct usage of ->bmap method.
  ecryptfs: drop direct calls to ->bmap
  iomap: Rename fiemap_ctx to fiemap_iomap_ctx
  fs: Introduce fiemap_ctx data structure
  iomap: Update iomap_fiemap to use new fiemap_ctx structure
  fiemap: Move fiemap flags to fiemap_ctx
  ext4: Remove direct usage of fiemap_extent_info
  f2fs: Remove direct usage of fiemap_extent_info
  Btrfs: Remove direct usage of fiemap_extent_info
  nilfs2: Remove direct usage of fiemap_extent_info
  ocfs2: Remove direct usage of fiemap_extent_info
  iomap: Remove direct usage of fiemap_extent_info
  fiemap: Use fiemap_ctx as fiemap_fill_next_extent argument
  fiemap: Start using new callback from fiemap_ctx
  fibmap: Use bmap instead of ->bmap method in ioctl_fibmap
  fiemap: Get rid of fiemap_extent_info
  Use FIEMAP for FIBMAP calls
  xfs: Get rid of ->bmap
  ext4: Get rid of ->bmap interface

 drivers/md/md-bitmap.c |  16 ++--
 fs/bad_inode.c         |   4 +-
 fs/btrfs/extent_io.c   |  27 +++----
 fs/btrfs/extent_io.h   |   3 +-
 fs/btrfs/inode.c       |   7 +-
 fs/cachefiles/rdwr.c   |  27 +++----
 fs/ecryptfs/mmap.c     |  16 ++--
 fs/ext2/ext2.h         |   3 +-
 fs/ext2/inode.c        |   6 +-
 fs/ext4/ext4.h         |   5 +-
 fs/ext4/extents.c      |  28 +++----
 fs/ext4/inline.c       |   4 +-
 fs/ext4/inode.c        |  73 ------------------
 fs/f2fs/data.c         |  27 +++----
 fs/f2fs/f2fs.h         |   7 +-
 fs/f2fs/inline.c       |   8 +-
 fs/gfs2/inode.c        |   7 +-
 fs/hpfs/file.c         |   4 +-
 fs/inode.c             |  57 ++++++++++----
 fs/ioctl.c             | 164 ++++++++++++++++++++++++++---------------
 fs/iomap.c             |  25 ++++---
 fs/jbd2/journal.c      |  22 ++++--
 fs/nilfs2/inode.c      |  21 +++---
 fs/nilfs2/nilfs.h      |   3 +-
 fs/ocfs2/extent_map.c  |  17 ++---
 fs/ocfs2/extent_map.h  |   3 +-
 fs/overlayfs/inode.c   |   7 +-
 fs/xfs/xfs_aops.c      |  24 ------
 fs/xfs/xfs_iops.c      |  16 ++--
 fs/xfs/xfs_trace.h     |   1 -
 include/linux/fs.h     |  41 ++++++-----
 include/linux/iomap.h  |   5 +-
 mm/page_io.c           |  11 ++-
 33 files changed, 338 insertions(+), 351 deletions(-)

-- 
2.17.1

[PATCH 01/20] fs: Enable bmap() function to properly return errors

[Carlos Maiolino]

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.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 drivers/md/md-bitmap.c | 16 ++++++++++------
 fs/inode.c             | 30 +++++++++++++++++-------------
 fs/jbd2/journal.c      | 22 +++++++++++++++-------
 include/linux/fs.h     |  2 +-
 mm/page_io.c           | 11 +++++++----
 5 files changed, 50 insertions(+), 31 deletions(-)

diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index 1cd4f991792c..0668b2dd290e 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -363,7 +363,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);
@@ -374,17 +374,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;
@@ -398,7 +402,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/inode.c b/fs/inode.c
index 73432e64f874..d09a6f4f0335 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1577,21 +1577,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 8ef6b6daaa7a..7acaf6f55404 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -814,18 +814,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 */
 	}
@@ -1251,11 +1256,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 8a27c186ab7c..1ac714e95d6b 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2786,7 +2786,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 a451ffa9491c..9e659ffb7046 100644
--- a/mm/page_io.c
+++ b/mm/page_io.c
@@ -177,8 +177,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;
 
 		/*
@@ -193,9 +194,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.17.1

[PATCH 02/20] cachefiles: drop direct usage of ->bmap method.

[Carlos Maiolino]

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

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 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 40f7595aad10..a3ee23fe9269 100644
--- a/fs/cachefiles/rdwr.c
+++ b/fs/cachefiles/rdwr.c
@@ -400,7 +400,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;
 
@@ -416,7 +416,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 */
@@ -432,12 +431,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);
 
-	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) {
@@ -709,7 +710,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 */
@@ -726,7 +726,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
@@ -734,13 +734,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.17.1

[PATCH 03/20] ecryptfs: drop direct calls to ->bmap

[Carlos Maiolino]

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

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 cdf358b209d9..ff323dccef36 100644
--- a/fs/ecryptfs/mmap.c
+++ b/fs/ecryptfs/mmap.c
@@ -538,16 +538,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.17.1

[PATCH 04/20] iomap: Rename fiemap_ctx to fiemap_iomap_ctx

[Carlos Maiolino]

fiemap_ctx structure is local to iomap code, so, rename it to match its
scope.

A new structure named fiemap_ctx will be added which will be globally
available as part of the fiemap kernel API.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/iomap.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/iomap.c b/fs/iomap.c
index f1cb8ce5bc54..838b405a9037 100644
--- a/fs/iomap.c
+++ b/fs/iomap.c
@@ -1098,7 +1098,7 @@ vm_fault_t iomap_page_mkwrite(struct vm_fault *vmf, const struct iomap_ops *ops)
 }
 EXPORT_SYMBOL_GPL(iomap_page_mkwrite);
 
-struct fiemap_ctx {
+struct fiemap_iomap_ctx {
 	struct fiemap_extent_info *fi;
 	struct iomap prev;
 };
@@ -1137,7 +1137,7 @@ static loff_t
 iomap_fiemap_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
 		struct iomap *iomap)
 {
-	struct fiemap_ctx *ctx = data;
+	struct fiemap_iomap_ctx *ctx = data;
 	loff_t ret = length;
 
 	if (iomap->type == IOMAP_HOLE)
@@ -1158,7 +1158,7 @@ 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)
 {
-	struct fiemap_ctx ctx;
+	struct fiemap_iomap_ctx ctx;
 	loff_t ret;
 
 	memset(&ctx, 0, sizeof(ctx));
-- 
2.17.1

[PATCH 05/20] fs: Introduce fiemap_ctx data structure

[Carlos Maiolino]

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, just
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.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/bad_inode.c        |  4 +---
 fs/btrfs/inode.c      |  6 ++++--
 fs/ext2/ext2.h        |  3 +--
 fs/ext2/inode.c       |  6 ++----
 fs/ext4/ext4.h        |  3 +--
 fs/ext4/extents.c     |  9 +++++----
 fs/f2fs/data.c        |  6 ++++--
 fs/f2fs/f2fs.h        |  3 +--
 fs/gfs2/inode.c       |  6 ++++--
 fs/hpfs/file.c        |  4 ++--
 fs/ioctl.c            | 23 +++++++++++++++--------
 fs/nilfs2/inode.c     |  6 ++++--
 fs/nilfs2/nilfs.h     |  3 +--
 fs/ocfs2/extent_map.c |  6 ++++--
 fs/ocfs2/extent_map.h |  3 +--
 fs/overlayfs/inode.c  |  6 +++---
 fs/xfs/xfs_iops.c     |  9 +++++----
 include/linux/fs.h    | 27 +++++++++++++++++++--------
 18 files changed, 77 insertions(+), 56 deletions(-)

diff --git a/fs/bad_inode.c b/fs/bad_inode.c
index 8035d2a44561..b5fbe28b8d17 100644
--- a/fs/bad_inode.c
+++ b/fs/bad_inode.c
@@ -119,9 +119,7 @@ static struct posix_acl *bad_inode_get_acl(struct inode *inode, int type)
 	return ERR_PTR(-EIO);
 }
 
-static int bad_inode_fiemap(struct inode *inode,
-			    struct fiemap_extent_info *fieinfo, u64 start,
-			    u64 len)
+static int bad_inode_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
 {
 	return -EIO;
 }
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index cd247a21afc4..ebe5aae69c44 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -8612,9 +8612,11 @@ 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_ctx *f_ctx)
 {
+	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
+	u64 start = f_ctx->fc_start;
+	u64 len	  = f_ctx->fc_len;
 	int	ret;
 
 	ret = fiemap_check_flags(fieinfo, BTRFS_FIEMAP_FLAGS);
diff --git a/fs/ext2/ext2.h b/fs/ext2/ext2.h
index e770cd100a6a..dc5dec52a7c8 100644
--- a/fs/ext2/ext2.h
+++ b/fs/ext2/ext2.h
@@ -775,8 +775,7 @@ extern void ext2_evict_inode(struct inode *);
 extern int ext2_get_block(struct inode *, sector_t, struct buffer_head *, int);
 extern int ext2_setattr (struct dentry *, struct iattr *);
 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_ctx *f_ctx);
 
 /* 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 e4bb9386c045..85e7edad58a3 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_ctx *f_ctx)
 {
-	return generic_block_fiemap(inode, fieinfo, start, len,
-				    ext2_get_block);
+	return generic_block_fiemap(inode, f_ctx, 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 3f89d0ab08fc..acc1ea0e9f40 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -3140,8 +3140,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_ctx *f_ctx);
 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 240b6dea5441..beccae768dac 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -5045,9 +5045,11 @@ 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_ctx *f_ctx)
 {
+	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
+	u64 start = f_ctx->fc_start;
+	u64 len = f_ctx->fc_len;
 	ext4_lblk_t start_blk;
 	int error = 0;
 
@@ -5069,8 +5071,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, f_ctx, 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 82b6e27a21cc..0f57403feb92 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1397,9 +1397,11 @@ 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_ctx *f_ctx)
 {
+	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
+	u64 start = f_ctx->fc_start;
+	u64 len = f_ctx->fc_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 4175445be2b0..185bb39dd82c 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -3075,8 +3075,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_ctx *f_ctx);
 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 648f0ca1ad57..2a4ce713c209 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -2004,9 +2004,11 @@ 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_ctx *f_ctx)
 {
+	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
+	u64 start = f_ctx->fc_start;
+	u64 len = f_ctx->fc_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..9e31278cbfe1 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_ctx *f_ctx)
 {
-	return generic_block_fiemap(inode, fieinfo, start, len, hpfs_get_block);
+	return generic_block_fiemap(inode, f_ctx, hpfs_get_block);
 }
 
 const struct address_space_operations hpfs_aops = {
diff --git a/fs/ioctl.c b/fs/ioctl.c
index 2005529af560..ddcc4f9a9cf1 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -181,6 +181,7 @@ static int ioctl_fiemap(struct file *filp, unsigned long arg)
 	struct fiemap_extent_info fieinfo = { 0, };
 	struct inode *inode = file_inode(filp);
 	struct super_block *sb = inode->i_sb;
+	struct fiemap_ctx f_ctx;
 	u64 len;
 	int error;
 
@@ -210,7 +211,12 @@ 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);
+	f_ctx.fc_flags = fieinfo.fi_flags;
+	f_ctx.fc_data = &fieinfo;
+	f_ctx.fc_start = fiemap.fm_start;
+	f_ctx.fc_len = len;
+
+	error = inode->i_op->fiemap(inode, &f_ctx);
 	fiemap.fm_flags = fieinfo.fi_flags;
 	fiemap.fm_mapped_extents = fieinfo.fi_extents_mapped;
 	if (copy_to_user(ufiemap, &fiemap, sizeof(fiemap)))
@@ -278,10 +284,12 @@ static inline loff_t blk_to_logical(struct inode *inode, sector_t blk)
  * generic_block_fiemap if you want the locking done for you.
  */
 
-int __generic_block_fiemap(struct inode *inode,
-			   struct fiemap_extent_info *fieinfo, loff_t start,
-			   loff_t len, get_block_t *get_block)
+int __generic_block_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx,
+			   get_block_t *get_block)
 {
+	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
+	loff_t start = f_ctx->fc_start;
+	loff_t len = f_ctx->fc_len;
 	struct buffer_head map_bh;
 	sector_t start_blk, last_blk;
 	loff_t isize = i_size_read(inode);
@@ -437,13 +445,12 @@ EXPORT_SYMBOL(__generic_block_fiemap);
  * the inode's mutex lock.
  */
 
-int generic_block_fiemap(struct inode *inode,
-			 struct fiemap_extent_info *fieinfo, u64 start,
-			 u64 len, get_block_t *get_block)
+int generic_block_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx,
+			 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, f_ctx, get_block);
 	inode_unlock(inode);
 	return ret;
 }
diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
index 671085512e0f..529262f0ab7d 100644
--- a/fs/nilfs2/inode.c
+++ b/fs/nilfs2/inode.c
@@ -992,9 +992,11 @@ 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_ctx *f_ctx)
 {
+	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
+	u64 start = f_ctx->fc_start;
+	u64 len = f_ctx->fc_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..b403dae8c258 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_ctx *f_ctx);
 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..1944f5659267 100644
--- a/fs/ocfs2/extent_map.c
+++ b/fs/ocfs2/extent_map.c
@@ -749,9 +749,11 @@ 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_ctx *f_ctx)
 {
+	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
+	u64 map_start = f_ctx->fc_start;
+	u64 map_len = f_ctx->fc_len;
 	int ret, is_last;
 	u32 mapping_end, cpos;
 	unsigned int hole_size;
diff --git a/fs/ocfs2/extent_map.h b/fs/ocfs2/extent_map.h
index 1057586ec19f..6c07fdf4d438 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_ctx *f_ctx);
 
 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..e56606c1ba92 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -456,9 +456,9 @@ 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_ctx *f_ctx)
 {
+	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
 	int err;
 	struct inode *realinode = ovl_inode_real(inode);
 	const struct cred *old_cred;
@@ -471,7 +471,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, f_ctx);
 	revert_creds(old_cred);
 
 	return err;
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index f48ffd7a8d3e..d33d56fa8097 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -1093,11 +1093,12 @@ xfs_vn_update_time(
 STATIC int
 xfs_vn_fiemap(
 	struct inode		*inode,
-	struct fiemap_extent_info *fieinfo,
-	u64			start,
-	u64			length)
+	struct fiemap_ctx	*f_ctx)
 {
-	int			error;
+	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
+	u64 start  = f_ctx->fc_start;
+	u64 length = f_ctx->fc_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 1ac714e95d6b..bd8e8a7dfd59 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1688,6 +1688,9 @@ extern bool may_open_dev(const struct path *path);
 /*
  * VFS FS_IOC_FIEMAP helper definitions.
  */
+
+struct fiemap_ctx;
+
 struct fiemap_extent_info {
 	unsigned int fi_flags;		/* Flags as passed from user */
 	unsigned int fi_extents_mapped;	/* Number of mapped extents */
@@ -1695,6 +1698,18 @@ struct fiemap_extent_info {
 	struct fiemap_extent __user *fi_extents_start; /* Start of
 							fiemap_extent array */
 };
+
+typedef int (*fiemap_fill_cb)(struct fiemap_ctx *f_ctx, u64 logical,
+			      u64 phys, u64 len, u32 flags);
+
+struct fiemap_ctx {
+	unsigned int fc_flags;
+	void *fc_data;
+	fiemap_fill_cb fc_cb; /* Unused by now */
+	u64 fc_start;
+	u64 fc_len;
+};
+
 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);
@@ -1822,8 +1837,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_ctx *);
 	int (*update_time)(struct inode *, struct timespec64 *, int);
 	int (*atomic_open)(struct inode *, struct dentry *,
 			   struct file *, unsigned open_flag,
@@ -3177,13 +3191,10 @@ static inline int vfs_fstat(int fd, struct kstat *stat)
 extern const char *vfs_get_link(struct dentry *, struct delayed_call *);
 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,
+extern int __generic_block_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx,
 				  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);
+extern int generic_block_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx,
+				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.17.1

[PATCH 06/20] iomap: Update iomap_fiemap to use new fiemap_ctx structure

[Carlos Maiolino]

iomap_fiemap() isn't directly called by ioctl_fiemap(), so it doesn't
needed to be changed when fiemap_ctx has been added, update it now.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/gfs2/inode.c       | 3 +--
 fs/iomap.c            | 3 ++-
 fs/xfs/xfs_iops.c     | 4 ++--
 include/linux/iomap.h | 4 ++--
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index 2a4ce713c209..bf461e3f012c 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -2006,7 +2006,6 @@ static int gfs2_getattr(const struct path *path, struct kstat *stat,
 
 static int gfs2_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
 {
-	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
 	u64 start = f_ctx->fc_start;
 	u64 len = f_ctx->fc_len;
 	struct gfs2_inode *ip = GFS2_I(inode);
@@ -2019,7 +2018,7 @@ static int gfs2_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
 	if (ret)
 		goto out;
 
-	ret = iomap_fiemap(inode, fieinfo, start, len, &gfs2_iomap_ops);
+	ret = iomap_fiemap(inode, f_ctx, start, len, &gfs2_iomap_ops);
 
 	gfs2_glock_dq_uninit(&gh);
 
diff --git a/fs/iomap.c b/fs/iomap.c
index 838b405a9037..42b131e1c955 100644
--- a/fs/iomap.c
+++ b/fs/iomap.c
@@ -1155,9 +1155,10 @@ 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,
+int iomap_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx,
 		loff_t start, loff_t len, const struct iomap_ops *ops)
 {
+	struct fiemap_extent_info *fi = f_ctx->fc_data;
 	struct fiemap_iomap_ctx ctx;
 	loff_t ret;
 
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index d33d56fa8097..22c479862ba6 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -1103,10 +1103,10 @@ xfs_vn_fiemap(
 	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,
+		error = iomap_fiemap(inode, f_ctx, start, length,
 				&xfs_xattr_iomap_ops);
 	} else {
-		error = iomap_fiemap(inode, fieinfo, start, length,
+		error = iomap_fiemap(inode, f_ctx, start, length,
 				&xfs_iomap_ops);
 	}
 	xfs_iunlock(XFS_I(inode), XFS_IOLOCK_SHARED);
diff --git a/include/linux/iomap.h b/include/linux/iomap.h
index 9a4258154b25..7ce84be499d6 100644
--- a/include/linux/iomap.h
+++ b/include/linux/iomap.h
@@ -144,8 +144,8 @@ int iomap_truncate_page(struct inode *inode, loff_t pos, bool *did_zero,
 		const struct iomap_ops *ops);
 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);
+int iomap_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx, loff_t start,
+		 loff_t len, 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.17.1

[PATCH 07/20] fiemap: Move fiemap flags to fiemap_ctx

[Carlos Maiolino]

struct fiemap_extent_info, will be gone in a later patch, but we still need the flags.
So, move the fiemap flags up to the new context structure fiemap_ctx.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/btrfs/inode.c      |  2 +-
 fs/ext4/extents.c     |  6 +++---
 fs/f2fs/data.c        |  6 +++---
 fs/ioctl.c            | 23 +++++++++++------------
 fs/iomap.c            |  4 ++--
 fs/nilfs2/inode.c     |  2 +-
 fs/ocfs2/extent_map.c |  2 +-
 fs/overlayfs/inode.c  |  3 +--
 fs/xfs/xfs_iops.c     |  5 ++---
 include/linux/fs.h    |  5 ++---
 10 files changed, 27 insertions(+), 31 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index ebe5aae69c44..77fd8587b8e7 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -8619,7 +8619,7 @@ static int btrfs_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
 	u64 len	  = f_ctx->fc_len;
 	int	ret;
 
-	ret = fiemap_check_flags(fieinfo, BTRFS_FIEMAP_FLAGS);
+	ret = fiemap_check_flags(f_ctx, BTRFS_FIEMAP_FLAGS);
 	if (ret)
 		return ret;
 
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index beccae768dac..d52aafc34e25 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -5063,7 +5063,7 @@ int ext4_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
 			return error;
 	}
 
-	if (fieinfo->fi_flags & FIEMAP_FLAG_CACHE) {
+	if (f_ctx->fc_flags & FIEMAP_FLAG_CACHE) {
 		error = ext4_ext_precache(inode);
 		if (error)
 			return error;
@@ -5073,10 +5073,10 @@ int ext4_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
 	if (!(ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)))
 		return generic_block_fiemap(inode, f_ctx, ext4_get_block);
 
-	if (fiemap_check_flags(fieinfo, EXT4_FIEMAP_FLAGS))
+	if (fiemap_check_flags(f_ctx, EXT4_FIEMAP_FLAGS))
 		return -EBADR;
 
-	if (fieinfo->fi_flags & FIEMAP_FLAG_XATTR) {
+	if (f_ctx->fc_flags & FIEMAP_FLAG_XATTR) {
 		error = ext4_xattr_fiemap(inode, fieinfo);
 	} else {
 		ext4_lblk_t len_blks;
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 0f57403feb92..c552e2d1dfbb 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1409,19 +1409,19 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
 	u32 flags = 0;
 	int ret = 0;
 
-	if (fieinfo->fi_flags & FIEMAP_FLAG_CACHE) {
+	if (f_ctx->fc_flags & FIEMAP_FLAG_CACHE) {
 		ret = f2fs_precache_extents(inode);
 		if (ret)
 			return ret;
 	}
 
-	ret = fiemap_check_flags(fieinfo, FIEMAP_FLAG_SYNC | FIEMAP_FLAG_XATTR);
+	ret = fiemap_check_flags(f_ctx, FIEMAP_FLAG_SYNC | FIEMAP_FLAG_XATTR);
 	if (ret)
 		return ret;
 
 	inode_lock(inode);
 
-	if (fieinfo->fi_flags & FIEMAP_FLAG_XATTR) {
+	if (f_ctx->fc_flags & FIEMAP_FLAG_XATTR) {
 		ret = f2fs_xattr_fiemap(inode, fieinfo);
 		goto out;
 	}
diff --git a/fs/ioctl.c b/fs/ioctl.c
index ddcc4f9a9cf1..77af3b116972 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -139,13 +139,13 @@ EXPORT_SYMBOL(fiemap_fill_next_extent);
  *
  * Returns 0 on success, -EBADR on bad flags.
  */
-int fiemap_check_flags(struct fiemap_extent_info *fieinfo, u32 fs_flags)
+int fiemap_check_flags(struct fiemap_ctx *f_ctx, u32 fs_flags)
 {
 	u32 incompat_flags;
 
-	incompat_flags = fieinfo->fi_flags & ~(FIEMAP_FLAGS_COMPAT & fs_flags);
+	incompat_flags = f_ctx->fc_flags & ~(FIEMAP_FLAGS_COMPAT & fs_flags);
 	if (incompat_flags) {
-		fieinfo->fi_flags = incompat_flags;
+		f_ctx->fc_flags = incompat_flags;
 		return -EBADR;
 	}
 	return 0;
@@ -199,25 +199,24 @@ static int ioctl_fiemap(struct file *filp, unsigned long arg)
 	if (error)
 		return error;
 
-	fieinfo.fi_flags = fiemap.fm_flags;
 	fieinfo.fi_extents_max = fiemap.fm_extent_count;
 	fieinfo.fi_extents_start = ufiemap->fm_extents;
 
+	f_ctx.fc_flags = fiemap.fm_flags;
+	f_ctx.fc_data = &fieinfo;
+	f_ctx.fc_start = fiemap.fm_start;
+	f_ctx.fc_len = len;
+
 	if (fiemap.fm_extent_count != 0 &&
 	    !access_ok(VERIFY_WRITE, fieinfo.fi_extents_start,
 		       fieinfo.fi_extents_max * sizeof(struct fiemap_extent)))
 		return -EFAULT;
 
-	if (fieinfo.fi_flags & FIEMAP_FLAG_SYNC)
+	if (f_ctx.fc_flags & FIEMAP_FLAG_SYNC)
 		filemap_write_and_wait(inode->i_mapping);
 
-	f_ctx.fc_flags = fieinfo.fi_flags;
-	f_ctx.fc_data = &fieinfo;
-	f_ctx.fc_start = fiemap.fm_start;
-	f_ctx.fc_len = len;
-
 	error = inode->i_op->fiemap(inode, &f_ctx);
-	fiemap.fm_flags = fieinfo.fi_flags;
+	fiemap.fm_flags = f_ctx.fc_flags;
 	fiemap.fm_mapped_extents = fieinfo.fi_extents_mapped;
 	if (copy_to_user(ufiemap, &fiemap, sizeof(fiemap)))
 		error = -EFAULT;
@@ -298,7 +297,7 @@ int __generic_block_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx,
 	bool past_eof = false, whole_file = false;
 	int ret = 0;
 
-	ret = fiemap_check_flags(fieinfo, FIEMAP_FLAG_SYNC);
+	ret = fiemap_check_flags(f_ctx, FIEMAP_FLAG_SYNC);
 	if (ret)
 		return ret;
 
diff --git a/fs/iomap.c b/fs/iomap.c
index 42b131e1c955..5afca566c2a9 100644
--- a/fs/iomap.c
+++ b/fs/iomap.c
@@ -1166,11 +1166,11 @@ int iomap_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx,
 	ctx.fi = fi;
 	ctx.prev.type = IOMAP_HOLE;
 
-	ret = fiemap_check_flags(fi, FIEMAP_FLAG_SYNC);
+	ret = fiemap_check_flags(f_ctx, FIEMAP_FLAG_SYNC);
 	if (ret)
 		return ret;
 
-	if (fi->fi_flags & FIEMAP_FLAG_SYNC) {
+	if (f_ctx->fc_flags & FIEMAP_FLAG_SYNC) {
 		ret = filemap_write_and_wait(inode->i_mapping);
 		if (ret)
 			return ret;
diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
index 529262f0ab7d..a8040ed52bd5 100644
--- a/fs/nilfs2/inode.c
+++ b/fs/nilfs2/inode.c
@@ -1007,7 +1007,7 @@ int nilfs_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
 	unsigned int blkbits = inode->i_blkbits;
 	int ret, n;
 
-	ret = fiemap_check_flags(fieinfo, FIEMAP_FLAG_SYNC);
+	ret = fiemap_check_flags(f_ctx, FIEMAP_FLAG_SYNC);
 	if (ret)
 		return ret;
 
diff --git a/fs/ocfs2/extent_map.c b/fs/ocfs2/extent_map.c
index 1944f5659267..5ba98d7a7a42 100644
--- a/fs/ocfs2/extent_map.c
+++ b/fs/ocfs2/extent_map.c
@@ -762,7 +762,7 @@ int ocfs2_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
 	struct buffer_head *di_bh = NULL;
 	struct ocfs2_extent_rec rec;
 
-	ret = fiemap_check_flags(fieinfo, OCFS2_FIEMAP_FLAGS);
+	ret = fiemap_check_flags(f_ctx, OCFS2_FIEMAP_FLAGS);
 	if (ret)
 		return ret;
 
diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index e56606c1ba92..3cef3b9b1854 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -458,7 +458,6 @@ int ovl_update_time(struct inode *inode, struct timespec64 *ts, int flags)
 
 static int ovl_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
 {
-	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
 	int err;
 	struct inode *realinode = ovl_inode_real(inode);
 	const struct cred *old_cred;
@@ -468,7 +467,7 @@ static int ovl_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
 
 	old_cred = ovl_override_creds(inode->i_sb);
 
-	if (fieinfo->fi_flags & FIEMAP_FLAG_SYNC)
+	if (f_ctx->fc_flags & FIEMAP_FLAG_SYNC)
 		filemap_write_and_wait(realinode->i_mapping);
 
 	err = realinode->i_op->fiemap(realinode, f_ctx);
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index 22c479862ba6..d859558de2fe 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -1095,14 +1095,13 @@ xfs_vn_fiemap(
 	struct inode		*inode,
 	struct fiemap_ctx	*f_ctx)
 {
-	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
 	u64 start  = f_ctx->fc_start;
 	u64 length = f_ctx->fc_len;
 	int error;
 
 	xfs_ilock(XFS_I(inode), XFS_IOLOCK_SHARED);
-	if (fieinfo->fi_flags & FIEMAP_FLAG_XATTR) {
-		fieinfo->fi_flags &= ~FIEMAP_FLAG_XATTR;
+	if (f_ctx->fc_flags & FIEMAP_FLAG_XATTR) {
+		f_ctx->fc_flags &= ~FIEMAP_FLAG_XATTR;
 		error = iomap_fiemap(inode, f_ctx, start, length,
 				&xfs_xattr_iomap_ops);
 	} else {
diff --git a/include/linux/fs.h b/include/linux/fs.h
index bd8e8a7dfd59..fb060123acff 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1692,7 +1692,6 @@ extern bool may_open_dev(const struct path *path);
 struct fiemap_ctx;
 
 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
@@ -1703,7 +1702,7 @@ typedef int (*fiemap_fill_cb)(struct fiemap_ctx *f_ctx, u64 logical,
 			      u64 phys, u64 len, u32 flags);
 
 struct fiemap_ctx {
-	unsigned int fc_flags;
+	unsigned int fc_flags;	/* Flags as passed from user */
 	void *fc_data;
 	fiemap_fill_cb fc_cb; /* Unused by now */
 	u64 fc_start;
@@ -1712,7 +1711,7 @@ struct fiemap_ctx {
 
 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);
+int fiemap_check_flags(struct fiemap_ctx *f_ctx, u32 fs_flags);
 
 /*
  * File types
-- 
2.17.1

[PATCH 08/20] ext4: Remove direct usage of fiemap_extent_info

[Carlos Maiolino]

struct fiemap_extent_info will be gone in later patches, remove its direct usage
from Ext4.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/ext4/ext4.h    |  2 +-
 fs/ext4/extents.c | 19 ++++++++++---------
 fs/ext4/inline.c  |  7 ++++---
 3 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index acc1ea0e9f40..c7ad6db930f5 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -3023,7 +3023,7 @@ extern struct buffer_head *ext4_get_first_inline_block(struct inode *inode,
 					struct ext4_dir_entry_2 **parent_de,
 					int *retval);
 extern int ext4_inline_data_fiemap(struct inode *inode,
-				   struct fiemap_extent_info *fieinfo,
+				   struct fiemap_ctx *f_ctx,
 				   int *has_inline, __u64 start, __u64 len);
 
 struct iomap;
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index d52aafc34e25..11ee46aff677 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -2151,7 +2151,7 @@ int ext4_ext_insert_extent(handle_t *handle, struct inode *inode,
 
 static int ext4_fill_fiemap_extents(struct inode *inode,
 				    ext4_lblk_t block, ext4_lblk_t num,
-				    struct fiemap_extent_info *fieinfo)
+				    struct fiemap_ctx *f_ctx)
 {
 	struct ext4_ext_path *path = NULL;
 	struct ext4_extent *ex;
@@ -2277,7 +2277,8 @@ static int ext4_fill_fiemap_extents(struct inode *inode,
 		}
 
 		if (exists) {
-			err = fiemap_fill_next_extent(fieinfo,
+			err = fiemap_fill_next_extent(
+				(struct fiemap_extent_info *)f_ctx->fc_data,
 				(__u64)es.es_lblk << blksize_bits,
 				(__u64)es.es_pblk << blksize_bits,
 				(__u64)es.es_len << blksize_bits,
@@ -5011,7 +5012,7 @@ static int ext4_find_delayed_extent(struct inode *inode,
 #define EXT4_FIEMAP_FLAGS	(FIEMAP_FLAG_SYNC|FIEMAP_FLAG_XATTR)
 
 static int ext4_xattr_fiemap(struct inode *inode,
-				struct fiemap_extent_info *fieinfo)
+				struct fiemap_ctx *f_ctx)
 {
 	__u64 physical = 0;
 	__u64 length;
@@ -5040,14 +5041,14 @@ static int ext4_xattr_fiemap(struct inode *inode,
 	}
 
 	if (physical)
-		error = fiemap_fill_next_extent(fieinfo, 0, physical,
-						length, flags);
+		error = fiemap_fill_next_extent(
+			(struct fiemap_extent_info *)f_ctx->fc_data,
+			0, physical, length, flags);
 	return (error < 0 ? error : 0);
 }
 
 int ext4_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
 {
-	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
 	u64 start = f_ctx->fc_start;
 	u64 len = f_ctx->fc_len;
 	ext4_lblk_t start_blk;
@@ -5056,7 +5057,7 @@ int ext4_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
 	if (ext4_has_inline_data(inode)) {
 		int has_inline = 1;
 
-		error = ext4_inline_data_fiemap(inode, fieinfo, &has_inline,
+		error = ext4_inline_data_fiemap(inode, f_ctx, &has_inline,
 						start, len);
 
 		if (has_inline)
@@ -5077,7 +5078,7 @@ int ext4_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
 		return -EBADR;
 
 	if (f_ctx->fc_flags & FIEMAP_FLAG_XATTR) {
-		error = ext4_xattr_fiemap(inode, fieinfo);
+		error = ext4_xattr_fiemap(inode, f_ctx);
 	} else {
 		ext4_lblk_t len_blks;
 		__u64 last_blk;
@@ -5093,7 +5094,7 @@ int ext4_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
 		 * and pushing extents back to the user.
 		 */
 		error = ext4_fill_fiemap_extents(inode, start_blk,
-						 len_blks, fieinfo);
+						 len_blks, f_ctx);
 	}
 	return error;
 }
diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
index 9c4bac18cc6c..7b9b0da60d54 100644
--- a/fs/ext4/inline.c
+++ b/fs/ext4/inline.c
@@ -1856,7 +1856,7 @@ int ext4_inline_data_iomap(struct inode *inode, struct iomap *iomap)
 }
 
 int ext4_inline_data_fiemap(struct inode *inode,
-			    struct fiemap_extent_info *fieinfo,
+			    struct fiemap_ctx *f_ctx,
 			    int *has_inline, __u64 start, __u64 len)
 {
 	__u64 physical = 0;
@@ -1888,8 +1888,9 @@ int ext4_inline_data_fiemap(struct inode *inode,
 	physical += offsetof(struct ext4_inode, i_block);
 
 	if (physical)
-		error = fiemap_fill_next_extent(fieinfo, start, physical,
-						inline_len, flags);
+		error = fiemap_fill_next_extent(
+			(struct fiemap_extent_info *)f_ctx->fc_data,
+			start, physical, inline_len, flags);
 	brelse(iloc.bh);
 out:
 	up_read(&EXT4_I(inode)->xattr_sem);
-- 
2.17.1

[PATCH 09/20] f2fs: Remove direct usage of fiemap_extent_info

[Carlos Maiolino]

struct fiemap-extent_info will be gone on future patches, remove its direct usage
from f2fs

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/f2fs/data.c   | 22 +++++++++++++---------
 fs/f2fs/f2fs.h   |  4 +---
 fs/f2fs/inline.c | 10 +++++++---
 3 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index c552e2d1dfbb..8ce60b5954d8 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1327,8 +1327,7 @@ static inline loff_t blk_to_logical(struct inode *inode, sector_t blk)
 	return (blk << inode->i_blkbits);
 }
 
-static int f2fs_xattr_fiemap(struct inode *inode,
-				struct fiemap_extent_info *fieinfo)
+static int f2fs_xattr_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
 {
 	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
 	struct page *page;
@@ -1367,7 +1366,10 @@ static int f2fs_xattr_fiemap(struct inode *inode,
 		if (!xnid)
 			flags |= FIEMAP_EXTENT_LAST;
 
-		err = fiemap_fill_next_extent(fieinfo, 0, phys, len, flags);
+		err = fiemap_fill_next_extent(
+				(struct fiemap_extent_info *)f_ctx->fc_data,
+				0, phys, len, flags);
+
 		if (err || err == 1)
 			return err;
 	}
@@ -1392,14 +1394,15 @@ static int f2fs_xattr_fiemap(struct inode *inode,
 	}
 
 	if (phys)
-		err = fiemap_fill_next_extent(fieinfo, 0, phys, len, flags);
+		err = fiemap_fill_next_extent(
+				(struct fiemap_extent_info *)f_ctx->fc_data,
+				0, phys, len, flags);
 
 	return (err < 0 ? err : 0);
 }
 
 int f2fs_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
 {
-	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
 	u64 start = f_ctx->fc_start;
 	u64 len = f_ctx->fc_len;
 	struct buffer_head map_bh;
@@ -1422,12 +1425,12 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
 	inode_lock(inode);
 
 	if (f_ctx->fc_flags & FIEMAP_FLAG_XATTR) {
-		ret = f2fs_xattr_fiemap(inode, fieinfo);
+		ret = f2fs_xattr_fiemap(inode, f_ctx);
 		goto out;
 	}
 
 	if (f2fs_has_inline_data(inode)) {
-		ret = f2fs_inline_data_fiemap(inode, fieinfo, start, len);
+		ret = f2fs_inline_data_fiemap(inode, f_ctx);
 		if (ret != -EAGAIN)
 			goto out;
 	}
@@ -1462,8 +1465,9 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
 		if (f2fs_encrypted_inode(inode))
 			flags |= FIEMAP_EXTENT_DATA_ENCRYPTED;
 
-		ret = fiemap_fill_next_extent(fieinfo, logical,
-				phys, size, flags);
+		ret = fiemap_fill_next_extent(
+				(struct fiemap_extent_info *)f_ctx->fc_data,
+				logical, phys, size, flags);
 	}
 
 	if (start_blk > last_blk || ret)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 185bb39dd82c..5e4e323ea4c6 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -3354,9 +3354,7 @@ void f2fs_delete_inline_entry(struct f2fs_dir_entry *dentry,
 bool f2fs_empty_inline_dir(struct inode *dir);
 int f2fs_read_inline_dir(struct file *file, struct dir_context *ctx,
 			struct fscrypt_str *fstr);
-int f2fs_inline_data_fiemap(struct inode *inode,
-			struct fiemap_extent_info *fieinfo,
-			__u64 start, __u64 len);
+int f2fs_inline_data_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx);
 
 /*
  * shrinker.c
diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
index 5c152fe08381..353a81317fac 100644
--- a/fs/f2fs/inline.c
+++ b/fs/f2fs/inline.c
@@ -671,9 +671,10 @@ int f2fs_read_inline_dir(struct file *file, struct dir_context *ctx,
 	return err < 0 ? err : 0;
 }
 
-int f2fs_inline_data_fiemap(struct inode *inode,
-		struct fiemap_extent_info *fieinfo, __u64 start, __u64 len)
+int f2fs_inline_data_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
 {
+	u64 start = f_ctx->fc_start;
+	u64 len = f_ctx->fc_len;
 	__u64 byteaddr, ilen;
 	__u32 flags = FIEMAP_EXTENT_DATA_INLINE | FIEMAP_EXTENT_NOT_ALIGNED |
 		FIEMAP_EXTENT_LAST;
@@ -704,7 +705,10 @@ int f2fs_inline_data_fiemap(struct inode *inode,
 	byteaddr = (__u64)ni.blk_addr << inode->i_sb->s_blocksize_bits;
 	byteaddr += (char *)inline_data_addr(inode, ipage) -
 					(char *)F2FS_INODE(ipage);
-	err = fiemap_fill_next_extent(fieinfo, start, byteaddr, ilen, flags);
+	err = fiemap_fill_next_extent(
+			(struct fiemap_extent_info *)f_ctx->fc_data,
+			start, byteaddr, ilen, flags);
+
 out:
 	f2fs_put_page(ipage, 1);
 	return err;
-- 
2.17.1

[PATCH 10/20] Btrfs: Remove direct usage of fiemap_extent_info

[Carlos Maiolino]

Remove most fiemap_extent_info usage from Btrfs.

Btrfs is a special case, because it's the only filesystem which directly
access data into fiemap_extent_info, the total removal of
fiemap_extent_info will also remove its last direct user (btrfs).

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/btrfs/extent_io.c | 34 ++++++++++++++++++++--------------
 fs/btrfs/extent_io.h |  3 +--
 fs/btrfs/inode.c     |  5 +----
 3 files changed, 22 insertions(+), 20 deletions(-)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index b5d384e1f25e..8d4d02d0bf8a 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -4331,7 +4331,7 @@ struct fiemap_cache {
  *
  * Return value is the same as fiemap_fill_next_extent().
  */
-static int emit_fiemap_extent(struct fiemap_extent_info *fieinfo,
+static int emit_fiemap_extent(struct fiemap_ctx *f_ctx,
 				struct fiemap_cache *cache,
 				u64 offset, u64 phys, u64 len, u32 flags)
 {
@@ -4373,8 +4373,9 @@ static int emit_fiemap_extent(struct fiemap_extent_info *fieinfo,
 	}
 
 	/* Not mergeable, need to submit cached one */
-	ret = fiemap_fill_next_extent(fieinfo, cache->offset, cache->phys,
-				      cache->len, cache->flags);
+	ret = fiemap_fill_next_extent(
+			(struct fiemap_extent_info *)f_ctx->fc_data,
+			cache->offset, cache->phys, cache->len, cache->flags);
 	cache->cached = false;
 	if (ret)
 		return ret;
@@ -4386,8 +4387,10 @@ static int emit_fiemap_extent(struct fiemap_extent_info *fieinfo,
 	cache->flags = flags;
 try_submit_last:
 	if (cache->flags & FIEMAP_EXTENT_LAST) {
-		ret = fiemap_fill_next_extent(fieinfo, cache->offset,
-				cache->phys, cache->len, cache->flags);
+		ret = fiemap_fill_next_extent(
+				(struct fiemap_extent_info *)f_ctx->fc_data,
+				cache->offset, cache->phys, cache->len,
+				cache->flags);
 		cache->cached = false;
 	}
 	return ret;
@@ -4405,7 +4408,7 @@ static int emit_fiemap_extent(struct fiemap_extent_info *fieinfo,
  * So we must emit it before ending extent_fiemap().
  */
 static int emit_last_fiemap_cache(struct btrfs_fs_info *fs_info,
-				  struct fiemap_extent_info *fieinfo,
+				  struct fiemap_ctx *f_ctx,
 				  struct fiemap_cache *cache)
 {
 	int ret;
@@ -4413,20 +4416,18 @@ static int emit_last_fiemap_cache(struct btrfs_fs_info *fs_info,
 	if (!cache->cached)
 		return 0;
 
-	ret = fiemap_fill_next_extent(fieinfo, cache->offset, cache->phys,
-				      cache->len, cache->flags);
+	ret = fiemap_fill_next_extent(
+			(struct fiemap_extent_info *)f_ctx->fc_data,
+			cache->offset, cache->phys, cache->len, cache->flags);
 	cache->cached = false;
 	if (ret > 0)
 		ret = 0;
 	return ret;
 }
 
-int extent_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-		__u64 start, __u64 len)
+int extent_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
 {
 	int ret = 0;
-	u64 off = start;
-	u64 max = start + len;
 	u32 flags = 0;
 	u32 found_type;
 	u64 last;
@@ -4439,10 +4440,15 @@ int extent_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 	struct btrfs_path *path;
 	struct btrfs_root *root = BTRFS_I(inode)->root;
 	struct fiemap_cache cache = { 0 };
+	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
 	int end = 0;
 	u64 em_start = 0;
 	u64 em_len = 0;
 	u64 em_end = 0;
+	u64 start = f_ctx->fc_start;
+	u64 len = f_ctx->fc_len;
+	u64 off = start;
+	u64 max = start + len;
 
 	if (len == 0)
 		return -EINVAL;
@@ -4602,7 +4608,7 @@ int extent_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 			flags |= FIEMAP_EXTENT_LAST;
 			end = 1;
 		}
-		ret = emit_fiemap_extent(fieinfo, &cache, em_start, disko,
+		ret = emit_fiemap_extent(f_ctx, &cache, em_start, disko,
 					   em_len, flags);
 		if (ret) {
 			if (ret == 1)
@@ -4612,7 +4618,7 @@ int extent_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 	}
 out_free:
 	if (!ret)
-		ret = emit_last_fiemap_cache(root->fs_info, fieinfo, &cache);
+		ret = emit_last_fiemap_cache(root->fs_info, f_ctx, &cache);
 	free_extent_map(em);
 out:
 	btrfs_free_path(path);
diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
index 369daa5d4f73..f50ad9911ab5 100644
--- a/fs/btrfs/extent_io.h
+++ b/fs/btrfs/extent_io.h
@@ -409,8 +409,7 @@ int btree_write_cache_pages(struct address_space *mapping,
 			    struct writeback_control *wbc);
 int extent_readpages(struct address_space *mapping, struct list_head *pages,
 		     unsigned nr_pages);
-int extent_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-		__u64 start, __u64 len);
+int extent_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx);
 void set_page_extent_mapped(struct page *page);
 
 struct extent_buffer *alloc_extent_buffer(struct btrfs_fs_info *fs_info,
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 77fd8587b8e7..96bc6c6908fb 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -8614,16 +8614,13 @@ static ssize_t btrfs_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
 
 static int btrfs_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
 {
-	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
-	u64 start = f_ctx->fc_start;
-	u64 len	  = f_ctx->fc_len;
 	int	ret;
 
 	ret = fiemap_check_flags(f_ctx, BTRFS_FIEMAP_FLAGS);
 	if (ret)
 		return ret;
 
-	return extent_fiemap(inode, fieinfo, start, len);
+	return extent_fiemap(inode, f_ctx);
 }
 
 int btrfs_readpage(struct file *file, struct page *page)
-- 
2.17.1

[PATCH 11/20] nilfs2: Remove direct usage of fiemap_extent_info

[Carlos Maiolino]

struct fiemap-extent_info will be gone in later patches, remove its direct usage
from nilfs2

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/nilfs2/inode.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
index a8040ed52bd5..88c13f6c13cd 100644
--- a/fs/nilfs2/inode.c
+++ b/fs/nilfs2/inode.c
@@ -994,7 +994,6 @@ void nilfs_dirty_inode(struct inode *inode, int flags)
 
 int nilfs_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
 {
-	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
 	u64 start = f_ctx->fc_start;
 	u64 len = f_ctx->fc_len;
 	struct the_nilfs *nilfs = inode->i_sb->s_fs_info;
@@ -1029,7 +1028,8 @@ int nilfs_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
 			if (size) {
 				/* End of the current extent */
 				ret = fiemap_fill_next_extent(
-					fieinfo, logical, phys, size, flags);
+					(struct fiemap_extent_info *)f_ctx->fc_data,
+					logical, phys, size, flags);
 				if (ret)
 					break;
 			}
@@ -1079,7 +1079,8 @@ int nilfs_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
 					flags |= FIEMAP_EXTENT_LAST;
 
 				ret = fiemap_fill_next_extent(
-					fieinfo, logical, phys, size, flags);
+					(struct fiemap_extent_info *)f_ctx->fc_data,
+					logical, phys, size, flags);
 				if (ret)
 					break;
 				size = 0;
@@ -1094,8 +1095,8 @@ int nilfs_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
 				} else {
 					/* Terminate the current extent */
 					ret = fiemap_fill_next_extent(
-						fieinfo, logical, phys, size,
-						flags);
+						(struct fiemap_extent_info *)f_ctx->fc_data,
+						logical, phys, size, flags);
 					if (ret || blkoff > end_blkoff)
 						break;
 
-- 
2.17.1

[PATCH 12/20] ocfs2: Remove direct usage of fiemap_extent_info

[Carlos Maiolino]

struct fiemap-extent_info will be removed in later patches, remove its direct
usage from ocfs2

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/ocfs2/extent_map.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/fs/ocfs2/extent_map.c b/fs/ocfs2/extent_map.c
index 5ba98d7a7a42..cffba6e542f9 100644
--- a/fs/ocfs2/extent_map.c
+++ b/fs/ocfs2/extent_map.c
@@ -714,8 +714,7 @@ int ocfs2_extent_map_get_blocks(struct inode *inode, u64 v_blkno, u64 *p_blkno,
  * mapping per se.
  */
 static int ocfs2_fiemap_inline(struct inode *inode, struct buffer_head *di_bh,
-			       struct fiemap_extent_info *fieinfo,
-			       u64 map_start)
+			       struct fiemap_ctx *f_ctx, u64 map_start)
 {
 	int ret;
 	unsigned int id_count;
@@ -738,8 +737,9 @@ static int ocfs2_fiemap_inline(struct inode *inode, struct buffer_head *di_bh,
 			phys += offsetof(struct ocfs2_dinode,
 					 id2.i_data.id_data);
 
-		ret = fiemap_fill_next_extent(fieinfo, 0, phys, id_count,
-					      flags);
+		ret = fiemap_fill_next_extent(
+				(struct fiemap_extent_info *)f_ctx->fc_data,
+				0, phys, id_count, flags);
 		if (ret < 0)
 			return ret;
 	}
@@ -751,7 +751,6 @@ static int ocfs2_fiemap_inline(struct inode *inode, struct buffer_head *di_bh,
 
 int ocfs2_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
 {
-	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
 	u64 map_start = f_ctx->fc_start;
 	u64 map_len = f_ctx->fc_len;
 	int ret, is_last;
@@ -779,7 +778,7 @@ int ocfs2_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
 	 */
 	if ((OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL) ||
 	    ocfs2_inode_is_fast_symlink(inode)) {
-		ret = ocfs2_fiemap_inline(inode, di_bh, fieinfo, map_start);
+		ret = ocfs2_fiemap_inline(inode, di_bh, f_ctx, map_start);
 		goto out_unlock;
 	}
 
@@ -813,8 +812,9 @@ int ocfs2_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
 		phys_bytes = le64_to_cpu(rec.e_blkno) << osb->sb->s_blocksize_bits;
 		virt_bytes = (u64)le32_to_cpu(rec.e_cpos) << osb->s_clustersize_bits;
 
-		ret = fiemap_fill_next_extent(fieinfo, virt_bytes, phys_bytes,
-					      len_bytes, fe_flags);
+		ret = fiemap_fill_next_extent(
+				(struct fiemap_extent_info*)f_ctx->fc_data,
+				virt_bytes, phys_bytes, len_bytes, fe_flags);
 		if (ret)
 			break;
 
-- 
2.17.1

[PATCH 13/20] iomap: Remove direct usage of fiemap_extent_info

[Carlos Maiolino]

struct fiemap_extent_info will be removed in future patches, remove its direct
usage from iomap

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/iomap.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/fs/iomap.c b/fs/iomap.c
index 5afca566c2a9..53e6b4bf10a9 100644
--- a/fs/iomap.c
+++ b/fs/iomap.c
@@ -1099,11 +1099,11 @@ vm_fault_t iomap_page_mkwrite(struct vm_fault *vmf, const struct iomap_ops *ops)
 EXPORT_SYMBOL_GPL(iomap_page_mkwrite);
 
 struct fiemap_iomap_ctx {
-	struct fiemap_extent_info *fi;
+	struct fiemap_ctx *f_ctx;
 	struct iomap prev;
 };
 
-static int iomap_to_fiemap(struct fiemap_extent_info *fi,
+static int iomap_to_fiemap(struct fiemap_ctx *f_ctx,
 		struct iomap *iomap, u32 flags)
 {
 	switch (iomap->type) {
@@ -1128,7 +1128,9 @@ static int iomap_to_fiemap(struct fiemap_extent_info *fi,
 	if (iomap->flags & IOMAP_F_SHARED)
 		flags |= FIEMAP_EXTENT_SHARED;
 
-	return fiemap_fill_next_extent(fi, iomap->offset,
+	return fiemap_fill_next_extent(
+			(struct fiemap_extent_info *)f_ctx->fc_data,
+			iomap->offset,
 			iomap->addr != IOMAP_NULL_ADDR ? iomap->addr : 0,
 			iomap->length, flags);
 }
@@ -1143,7 +1145,7 @@ iomap_fiemap_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
 	if (iomap->type == IOMAP_HOLE)
 		return length;
 
-	ret = iomap_to_fiemap(ctx->fi, &ctx->prev, 0);
+	ret = iomap_to_fiemap(ctx->f_ctx, &ctx->prev, 0);
 	ctx->prev = *iomap;
 	switch (ret) {
 	case 0:		/* success */
@@ -1158,12 +1160,11 @@ iomap_fiemap_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
 int iomap_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx,
 		loff_t start, loff_t len, const struct iomap_ops *ops)
 {
-	struct fiemap_extent_info *fi = f_ctx->fc_data;
 	struct fiemap_iomap_ctx ctx;
 	loff_t ret;
 
 	memset(&ctx, 0, sizeof(ctx));
-	ctx.fi = fi;
+	ctx.f_ctx = f_ctx;
 	ctx.prev.type = IOMAP_HOLE;
 
 	ret = fiemap_check_flags(f_ctx, FIEMAP_FLAG_SYNC);
@@ -1192,7 +1193,7 @@ int iomap_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx,
 	}
 
 	if (ctx.prev.type != IOMAP_HOLE) {
-		ret = iomap_to_fiemap(fi, &ctx.prev, FIEMAP_EXTENT_LAST);
+		ret = iomap_to_fiemap(f_ctx, &ctx.prev, FIEMAP_EXTENT_LAST);
 		if (ret < 0)
 			return ret;
 	}
-- 
2.17.1

[PATCH 14/20] fiemap: Use fiemap_ctx as fiemap_fill_next_extent argument

[Carlos Maiolino]

Replace the current fiemap_extent_info argument in
fiemap_fill_next_extent(), by the new fiemap_ctx structure. This way we
remove (almost) all direct usage of fiemap_extent_info outside fiemap
code (except by btrfs).

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/btrfs/extent_io.c  | 16 ++++++----------
 fs/ext4/extents.c     |  8 +++-----
 fs/ext4/inline.c      |  5 ++---
 fs/f2fs/data.c        | 13 ++++---------
 fs/f2fs/inline.c      |  4 +---
 fs/ioctl.c            | 12 ++++++------
 fs/iomap.c            |  3 +--
 fs/nilfs2/inode.c     | 16 +++++++---------
 fs/ocfs2/extent_map.c |  9 +++------
 include/linux/fs.h    |  2 +-
 10 files changed, 34 insertions(+), 54 deletions(-)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 8d4d02d0bf8a..6a8bc502bc04 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -4373,9 +4373,8 @@ static int emit_fiemap_extent(struct fiemap_ctx *f_ctx,
 	}
 
 	/* Not mergeable, need to submit cached one */
-	ret = fiemap_fill_next_extent(
-			(struct fiemap_extent_info *)f_ctx->fc_data,
-			cache->offset, cache->phys, cache->len, cache->flags);
+	ret = fiemap_fill_next_extent(f_ctx, cache->offset, cache->phys,
+				      cache->len, cache->flags);
 	cache->cached = false;
 	if (ret)
 		return ret;
@@ -4387,10 +4386,8 @@ static int emit_fiemap_extent(struct fiemap_ctx *f_ctx,
 	cache->flags = flags;
 try_submit_last:
 	if (cache->flags & FIEMAP_EXTENT_LAST) {
-		ret = fiemap_fill_next_extent(
-				(struct fiemap_extent_info *)f_ctx->fc_data,
-				cache->offset, cache->phys, cache->len,
-				cache->flags);
+		ret = fiemap_fill_next_extent(f_ctx, cache->offset, cache->phys,
+					      cache->len, cache->flags);
 		cache->cached = false;
 	}
 	return ret;
@@ -4416,9 +4413,8 @@ static int emit_last_fiemap_cache(struct btrfs_fs_info *fs_info,
 	if (!cache->cached)
 		return 0;
 
-	ret = fiemap_fill_next_extent(
-			(struct fiemap_extent_info *)f_ctx->fc_data,
-			cache->offset, cache->phys, cache->len, cache->flags);
+	ret = fiemap_fill_next_extent(f_ctx, cache->offset, cache->phys,
+				      cache->len, cache->flags);
 	cache->cached = false;
 	if (ret > 0)
 		ret = 0;
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 11ee46aff677..94fdebc4d6db 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -2277,8 +2277,7 @@ static int ext4_fill_fiemap_extents(struct inode *inode,
 		}
 
 		if (exists) {
-			err = fiemap_fill_next_extent(
-				(struct fiemap_extent_info *)f_ctx->fc_data,
+			err = fiemap_fill_next_extent(f_ctx,
 				(__u64)es.es_lblk << blksize_bits,
 				(__u64)es.es_pblk << blksize_bits,
 				(__u64)es.es_len << blksize_bits,
@@ -5041,9 +5040,8 @@ static int ext4_xattr_fiemap(struct inode *inode,
 	}
 
 	if (physical)
-		error = fiemap_fill_next_extent(
-			(struct fiemap_extent_info *)f_ctx->fc_data,
-			0, physical, length, flags);
+		error = fiemap_fill_next_extent(f_ctx, 0, physical,
+						length, flags);
 	return (error < 0 ? error : 0);
 }
 
diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
index 7b9b0da60d54..e1079a1c85f4 100644
--- a/fs/ext4/inline.c
+++ b/fs/ext4/inline.c
@@ -1888,9 +1888,8 @@ int ext4_inline_data_fiemap(struct inode *inode,
 	physical += offsetof(struct ext4_inode, i_block);
 
 	if (physical)
-		error = fiemap_fill_next_extent(
-			(struct fiemap_extent_info *)f_ctx->fc_data,
-			start, physical, inline_len, flags);
+		error = fiemap_fill_next_extent(f_ctx, start, physical,
+						inline_len, flags);
 	brelse(iloc.bh);
 out:
 	up_read(&EXT4_I(inode)->xattr_sem);
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 8ce60b5954d8..2470f8b346db 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1366,9 +1366,7 @@ static int f2fs_xattr_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
 		if (!xnid)
 			flags |= FIEMAP_EXTENT_LAST;
 
-		err = fiemap_fill_next_extent(
-				(struct fiemap_extent_info *)f_ctx->fc_data,
-				0, phys, len, flags);
+		err = fiemap_fill_next_extent(f_ctx, 0, phys, len, flags);
 
 		if (err || err == 1)
 			return err;
@@ -1394,9 +1392,7 @@ static int f2fs_xattr_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
 	}
 
 	if (phys)
-		err = fiemap_fill_next_extent(
-				(struct fiemap_extent_info *)f_ctx->fc_data,
-				0, phys, len, flags);
+		err = fiemap_fill_next_extent(f_ctx, 0, phys, len, flags);
 
 	return (err < 0 ? err : 0);
 }
@@ -1465,9 +1461,8 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
 		if (f2fs_encrypted_inode(inode))
 			flags |= FIEMAP_EXTENT_DATA_ENCRYPTED;
 
-		ret = fiemap_fill_next_extent(
-				(struct fiemap_extent_info *)f_ctx->fc_data,
-				logical, phys, size, flags);
+		ret = fiemap_fill_next_extent(f_ctx, logical, phys,
+					      size, flags);
 	}
 
 	if (start_blk > last_blk || ret)
diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
index 353a81317fac..0c4534880a99 100644
--- a/fs/f2fs/inline.c
+++ b/fs/f2fs/inline.c
@@ -705,9 +705,7 @@ int f2fs_inline_data_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
 	byteaddr = (__u64)ni.blk_addr << inode->i_sb->s_blocksize_bits;
 	byteaddr += (char *)inline_data_addr(inode, ipage) -
 					(char *)F2FS_INODE(ipage);
-	err = fiemap_fill_next_extent(
-			(struct fiemap_extent_info *)f_ctx->fc_data,
-			start, byteaddr, ilen, flags);
+	err = fiemap_fill_next_extent(f_ctx, start, byteaddr, ilen, flags);
 
 out:
 	f2fs_put_page(ipage, 1);
diff --git a/fs/ioctl.c b/fs/ioctl.c
index 77af3b116972..27f79b29cb07 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -86,9 +86,10 @@ static int ioctl_fibmap(struct file *filp, int __user *p)
 #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_next_extent(struct fiemap_ctx *f_ctx, u64 logical,
 			    u64 phys, u64 len, u32 flags)
 {
+	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
 	struct fiemap_extent extent;
 	struct fiemap_extent __user *dest = fieinfo->fi_extents_start;
 
@@ -286,7 +287,6 @@ static inline loff_t blk_to_logical(struct inode *inode, sector_t blk)
 int __generic_block_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx,
 			   get_block_t *get_block)
 {
-	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
 	loff_t start = f_ctx->fc_start;
 	loff_t len = f_ctx->fc_len;
 	struct buffer_head map_bh;
@@ -354,11 +354,11 @@ int __generic_block_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx,
 			 */
 			if (past_eof && size) {
 				flags = FIEMAP_EXTENT_MERGED|FIEMAP_EXTENT_LAST;
-				ret = fiemap_fill_next_extent(fieinfo, logical,
+				ret = fiemap_fill_next_extent(f_ctx, logical,
 							      phys, size,
 							      flags);
 			} else if (size) {
-				ret = fiemap_fill_next_extent(fieinfo, logical,
+				ret = fiemap_fill_next_extent(f_ctx, logical,
 							      phys, size, flags);
 				size = 0;
 			}
@@ -383,7 +383,7 @@ int __generic_block_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx,
 			 * and break
 			 */
 			if (start_blk > last_blk && !whole_file) {
-				ret = fiemap_fill_next_extent(fieinfo, logical,
+				ret = fiemap_fill_next_extent(f_ctx, logical,
 							      phys, size,
 							      flags);
 				break;
@@ -394,7 +394,7 @@ int __generic_block_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx,
 			 * to add, so add it.
 			 */
 			if (size) {
-				ret = fiemap_fill_next_extent(fieinfo, logical,
+				ret = fiemap_fill_next_extent(f_ctx, logical,
 							      phys, size,
 							      flags);
 				if (ret)
diff --git a/fs/iomap.c b/fs/iomap.c
index 53e6b4bf10a9..8d4fca1c7ce1 100644
--- a/fs/iomap.c
+++ b/fs/iomap.c
@@ -1128,8 +1128,7 @@ static int iomap_to_fiemap(struct fiemap_ctx *f_ctx,
 	if (iomap->flags & IOMAP_F_SHARED)
 		flags |= FIEMAP_EXTENT_SHARED;
 
-	return fiemap_fill_next_extent(
-			(struct fiemap_extent_info *)f_ctx->fc_data,
+	return fiemap_fill_next_extent(f_ctx,
 			iomap->offset,
 			iomap->addr != IOMAP_NULL_ADDR ? iomap->addr : 0,
 			iomap->length, flags);
diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
index 88c13f6c13cd..bb92c08904b7 100644
--- a/fs/nilfs2/inode.c
+++ b/fs/nilfs2/inode.c
@@ -1027,9 +1027,8 @@ int nilfs_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
 		if (delalloc_blklen && blkoff == delalloc_blkoff) {
 			if (size) {
 				/* End of the current extent */
-				ret = fiemap_fill_next_extent(
-					(struct fiemap_extent_info *)f_ctx->fc_data,
-					logical, phys, size, flags);
+				ret = fiemap_fill_next_extent(f_ctx, logical,
+							     phys, size, flags);
 				if (ret)
 					break;
 			}
@@ -1078,9 +1077,8 @@ int nilfs_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
 				if (past_eof)
 					flags |= FIEMAP_EXTENT_LAST;
 
-				ret = fiemap_fill_next_extent(
-					(struct fiemap_extent_info *)f_ctx->fc_data,
-					logical, phys, size, flags);
+				ret = fiemap_fill_next_extent(f_ctx, logical,
+							     phys, size, flags);
 				if (ret)
 					break;
 				size = 0;
@@ -1094,9 +1092,9 @@ int nilfs_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
 					size += n << blkbits;
 				} else {
 					/* Terminate the current extent */
-					ret = fiemap_fill_next_extent(
-						(struct fiemap_extent_info *)f_ctx->fc_data,
-						logical, phys, size, flags);
+					ret = fiemap_fill_next_extent(f_ctx,
+							logical, phys, size,
+							flags);
 					if (ret || blkoff > end_blkoff)
 						break;
 
diff --git a/fs/ocfs2/extent_map.c b/fs/ocfs2/extent_map.c
index cffba6e542f9..53847d2bf9d2 100644
--- a/fs/ocfs2/extent_map.c
+++ b/fs/ocfs2/extent_map.c
@@ -737,9 +737,7 @@ static int ocfs2_fiemap_inline(struct inode *inode, struct buffer_head *di_bh,
 			phys += offsetof(struct ocfs2_dinode,
 					 id2.i_data.id_data);
 
-		ret = fiemap_fill_next_extent(
-				(struct fiemap_extent_info *)f_ctx->fc_data,
-				0, phys, id_count, flags);
+		ret = fiemap_fill_next_extent(f_ctx, 0, phys, id_count, flags);
 		if (ret < 0)
 			return ret;
 	}
@@ -812,9 +810,8 @@ int ocfs2_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
 		phys_bytes = le64_to_cpu(rec.e_blkno) << osb->sb->s_blocksize_bits;
 		virt_bytes = (u64)le32_to_cpu(rec.e_cpos) << osb->s_clustersize_bits;
 
-		ret = fiemap_fill_next_extent(
-				(struct fiemap_extent_info*)f_ctx->fc_data,
-				virt_bytes, phys_bytes, len_bytes, fe_flags);
+		ret = fiemap_fill_next_extent(f_ctx, virt_bytes, phys_bytes,
+					      len_bytes, fe_flags);
 		if (ret)
 			break;
 
diff --git a/include/linux/fs.h b/include/linux/fs.h
index fb060123acff..945cfb3e06e4 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1709,7 +1709,7 @@ struct fiemap_ctx {
 	u64 fc_len;
 };
 
-int fiemap_fill_next_extent(struct fiemap_extent_info *info, u64 logical,
+int fiemap_fill_next_extent(struct fiemap_ctx *f_ctx, u64 logical,
 			    u64 phys, u64 len, u32 flags);
 int fiemap_check_flags(struct fiemap_ctx *f_ctx, u32 fs_flags);
 
-- 
2.17.1

[PATCH 15/20] fiemap: Start using new callback from fiemap_ctx

[Carlos Maiolino]

Now that fiemap methods are updated to work based on the new fiemap_ctx
structure, update the code to use the new callback.
The current fiemap_fill_next_extent will be used for user calls of
->fiemap methods, as its done today, but passed in as a callback to
fiemap_ctx. So, rename it to ifemap_fill_usr_extent().

The 'new' fiemap_fill_next_extent() will now be just a helper to call
fiemap_ctx callback.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/ioctl.c         | 42 +++++++++++++++++++++++++-----------------
 include/linux/fs.h |  2 +-
 2 files changed, 26 insertions(+), 18 deletions(-)

diff --git a/fs/ioctl.c b/fs/ioctl.c
index 27f79b29cb07..85a7aec40e3d 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -68,25 +68,10 @@ static int ioctl_fibmap(struct file *filp, int __user *p)
 	return put_user(res, p);
 }
 
-/**
- * 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_ctx *f_ctx, u64 logical,
+int fiemap_fill_usr_extent(struct fiemap_ctx *f_ctx, u64 logical,
 			    u64 phys, u64 len, u32 flags)
 {
 	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
@@ -124,8 +109,29 @@ int fiemap_fill_next_extent(struct fiemap_ctx *f_ctx, u64 logical,
 		return 1;
 	return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0;
 }
-EXPORT_SYMBOL(fiemap_fill_next_extent);
 
+/**
+ * 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_ctx *f_ctx, u64 logical,
+			    u64 phys, u64 len, u32 flags)
+{
+	return f_ctx->fc_cb(f_ctx, logical, phys, len, flags);
+}
+EXPORT_SYMBOL(fiemap_fill_next_extent);
 /**
  * fiemap_check_flags - check validity of requested flags for fiemap
  * @fieinfo:	Fiemap context passed into ->fiemap
@@ -208,6 +214,8 @@ static int ioctl_fiemap(struct file *filp, unsigned long arg)
 	f_ctx.fc_start = fiemap.fm_start;
 	f_ctx.fc_len = len;
 
+	f_ctx.fc_cb = fiemap_fill_usr_extent;
+
 	if (fiemap.fm_extent_count != 0 &&
 	    !access_ok(VERIFY_WRITE, fieinfo.fi_extents_start,
 		       fieinfo.fi_extents_max * sizeof(struct fiemap_extent)))
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 945cfb3e06e4..9a538bc0dac2 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1704,7 +1704,7 @@ typedef int (*fiemap_fill_cb)(struct fiemap_ctx *f_ctx, u64 logical,
 struct fiemap_ctx {
 	unsigned int fc_flags;	/* Flags as passed from user */
 	void *fc_data;
-	fiemap_fill_cb fc_cb; /* Unused by now */
+	fiemap_fill_cb fc_cb;
 	u64 fc_start;
 	u64 fc_len;
 };
-- 
2.17.1

[PATCH 16/20] fibmap: Use bmap instead of ->bmap method in ioctl_fibmap

[Carlos Maiolino]

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>
---
 fs/ioctl.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/fs/ioctl.c b/fs/ioctl.c
index 85a7aec40e3d..cbc1b21c4f7c 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -53,19 +53,23 @@ 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, 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(block, p);
+	if (error)
+		return error;
+
+	error = bmap(inode, (sector_t *)&block);
+	if (error)
+		return error;
+
+	error = put_user(block, p);
+
+	return error;
 }
 
 #define SET_UNKNOWN_FLAGS	(FIEMAP_EXTENT_DELALLOC)
-- 
2.17.1

[PATCH 17/20] fiemap: Get rid of fiemap_extent_info

[Carlos Maiolino]

With the goal of using FIEMAP infra-structure for FIBMAP ioctl, and with
the updates being done previously into the FIEMAP interface, the
fiemap_extent_info structure is no longer necessary, so, move
fi_extents_mapped and fi_extents_max up to fiemap_ctx. and use
fiemap_ctx.fc_data to hold a pointer to struct fiemap_extent.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/btrfs/extent_io.c  |  3 +--
 fs/ioctl.c            | 29 +++++++++++++----------------
 include/linux/fs.h    |  9 ++-------
 include/linux/iomap.h |  1 -
 4 files changed, 16 insertions(+), 26 deletions(-)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 6a8bc502bc04..88b8a4416dfc 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -4436,7 +4436,6 @@ int extent_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
 	struct btrfs_path *path;
 	struct btrfs_root *root = BTRFS_I(inode)->root;
 	struct fiemap_cache cache = { 0 };
-	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
 	int end = 0;
 	u64 em_start = 0;
 	u64 em_len = 0;
@@ -4561,7 +4560,7 @@ int extent_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
 		} else if (em->block_start == EXTENT_MAP_DELALLOC) {
 			flags |= (FIEMAP_EXTENT_DELALLOC |
 				  FIEMAP_EXTENT_UNKNOWN);
-		} else if (fieinfo->fi_extents_max) {
+		} else if (f_ctx->fc_extents_max) {
 			u64 bytenr = em->block_start -
 				(em->start - em->orig_start);
 
diff --git a/fs/ioctl.c b/fs/ioctl.c
index cbc1b21c4f7c..71d11201a06b 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -78,17 +78,16 @@ static int ioctl_fibmap(struct file *filp, int __user *p)
 int fiemap_fill_usr_extent(struct fiemap_ctx *f_ctx, u64 logical,
 			    u64 phys, u64 len, u32 flags)
 {
-	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
 	struct fiemap_extent extent;
-	struct fiemap_extent __user *dest = fieinfo->fi_extents_start;
+	struct fiemap_extent __user *dest = f_ctx->fc_data;
 
 	/* only count the extents */
-	if (fieinfo->fi_extents_max == 0) {
-		fieinfo->fi_extents_mapped++;
+	if (f_ctx->fc_extents_max == 0) {
+		f_ctx->fc_extents_mapped++;
 		return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0;
 	}
 
-	if (fieinfo->fi_extents_mapped >= fieinfo->fi_extents_max)
+	if (f_ctx->fc_extents_mapped >= f_ctx->fc_extents_max)
 		return 1;
 
 	if (flags & SET_UNKNOWN_FLAGS)
@@ -104,12 +103,12 @@ int fiemap_fill_usr_extent(struct fiemap_ctx *f_ctx, u64 logical,
 	extent.fe_length = len;
 	extent.fe_flags = flags;
 
-	dest += fieinfo->fi_extents_mapped;
+	dest += f_ctx->fc_extents_mapped;
 	if (copy_to_user(dest, &extent, sizeof(extent)))
 		return -EFAULT;
 
-	fieinfo->fi_extents_mapped++;
-	if (fieinfo->fi_extents_mapped == fieinfo->fi_extents_max)
+	f_ctx->fc_extents_mapped++;
+	if (f_ctx->fc_extents_mapped == f_ctx->fc_extents_max)
 		return 1;
 	return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0;
 }
@@ -189,7 +188,6 @@ static int ioctl_fiemap(struct file *filp, unsigned long arg)
 {
 	struct fiemap fiemap;
 	struct fiemap __user *ufiemap = (struct fiemap __user *) arg;
-	struct fiemap_extent_info fieinfo = { 0, };
 	struct inode *inode = file_inode(filp);
 	struct super_block *sb = inode->i_sb;
 	struct fiemap_ctx f_ctx;
@@ -210,19 +208,18 @@ static int ioctl_fiemap(struct file *filp, unsigned long arg)
 	if (error)
 		return error;
 
-	fieinfo.fi_extents_max = fiemap.fm_extent_count;
-	fieinfo.fi_extents_start = ufiemap->fm_extents;
-
+	f_ctx.fc_extents_max = fiemap.fm_extent_count;
+	f_ctx.fc_data = ufiemap->fm_extents;
+	f_ctx.fc_extents_mapped = 0;
 	f_ctx.fc_flags = fiemap.fm_flags;
-	f_ctx.fc_data = &fieinfo;
 	f_ctx.fc_start = fiemap.fm_start;
 	f_ctx.fc_len = len;
 
 	f_ctx.fc_cb = fiemap_fill_usr_extent;
 
 	if (fiemap.fm_extent_count != 0 &&
-	    !access_ok(VERIFY_WRITE, fieinfo.fi_extents_start,
-		       fieinfo.fi_extents_max * sizeof(struct fiemap_extent)))
+	    !access_ok(VERIFY_WRITE, f_ctx.fc_data,
+		       f_ctx.fc_extents_max * sizeof(struct fiemap_extent)))
 		return -EFAULT;
 
 	if (f_ctx.fc_flags & FIEMAP_FLAG_SYNC)
@@ -230,7 +227,7 @@ static int ioctl_fiemap(struct file *filp, unsigned long arg)
 
 	error = inode->i_op->fiemap(inode, &f_ctx);
 	fiemap.fm_flags = f_ctx.fc_flags;
-	fiemap.fm_mapped_extents = fieinfo.fi_extents_mapped;
+	fiemap.fm_mapped_extents = f_ctx.fc_extents_mapped;
 	if (copy_to_user(ufiemap, &fiemap, sizeof(fiemap)))
 		error = -EFAULT;
 
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 9a538bc0dac2..4c6dee908a38 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1691,18 +1691,13 @@ extern bool may_open_dev(const struct path *path);
 
 struct fiemap_ctx;
 
-struct fiemap_extent_info {
-	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 */
-};
-
 typedef int (*fiemap_fill_cb)(struct fiemap_ctx *f_ctx, u64 logical,
 			      u64 phys, u64 len, u32 flags);
 
 struct fiemap_ctx {
 	unsigned int fc_flags;	/* Flags as passed from user */
+	unsigned int fc_extents_mapped;	/* Number of mapped extents */
+	unsigned int fc_extents_max;	/* Size of fiemap_extent array */
 	void *fc_data;
 	fiemap_fill_cb fc_cb;
 	u64 fc_start;
diff --git a/include/linux/iomap.h b/include/linux/iomap.h
index 7ce84be499d6..21643bcf6104 100644
--- a/include/linux/iomap.h
+++ b/include/linux/iomap.h
@@ -9,7 +9,6 @@
 #include <linux/mm_types.h>
 
 struct address_space;
-struct fiemap_extent_info;
 struct inode;
 struct iov_iter;
 struct kiocb;
-- 
2.17.1

[PATCH 18/20] Use FIEMAP for FIBMAP calls

[Carlos Maiolino]

Enables the usage of FIEMAP ioctl infrastructure to handle FIBMAP calls,
from this point, ->bmap() methods can start to be removed.

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

This patch can be improved, since fiemap_fill_kernel_extent and
fiemap_fill_usr_extent shares a lot of common code, which still is WIP.

 fs/inode.c         | 35 ++++++++++++++++++++++++++++++-----
 fs/ioctl.c         | 31 +++++++++++++++++++++++++++++++
 include/linux/fs.h |  2 ++
 3 files changed, 63 insertions(+), 5 deletions(-)

diff --git a/fs/inode.c b/fs/inode.c
index d09a6f4f0335..389c2165959c 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1591,11 +1591,36 @@ EXPORT_SYMBOL(iput);
  */
 int bmap(struct inode *inode, sector_t *block)
 {
-	if (!inode->i_mapping->a_ops->bmap)
-		return -EINVAL;
-
-	*block = inode->i_mapping->a_ops->bmap(inode->i_mapping, *block);
-	return 0;
+	struct fiemap_ctx f_ctx;
+	struct fiemap_extent fextent;
+	u64 start = *block << inode->i_blkbits;
+	int error = -EINVAL;
+
+	if (inode->i_op->fiemap) {
+		fextent.fe_logical = 0;
+		fextent.fe_physical = 0;
+		f_ctx.fc_extents_max = 1;
+		f_ctx.fc_extents_mapped = 0;
+		f_ctx.fc_data = &fextent;
+		f_ctx.fc_start = start;
+		f_ctx.fc_len = 1;
+		f_ctx.fc_flags = 0;
+		f_ctx.fc_cb = fiemap_fill_kernel_extent;
+
+		error = inode->i_op->fiemap(inode, &f_ctx);
+
+		if (error)
+			goto out;
+
+		*block = (fextent.fe_physical +
+			(start - fextent.fe_logical)) >> inode->i_blkbits;
+
+	} else if (inode->i_mapping->a_ops->bmap) {
+		*block = inode->i_mapping->a_ops->bmap(inode->i_mapping, *block);
+		error = 0;
+	}
+out:
+	return error;
 }
 EXPORT_SYMBOL(bmap);
 
diff --git a/fs/ioctl.c b/fs/ioctl.c
index 71d11201a06b..dce710699b82 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -113,6 +113,37 @@ int fiemap_fill_usr_extent(struct fiemap_ctx *f_ctx, u64 logical,
 	return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0;
 }
 
+int fiemap_fill_kernel_extent(struct fiemap_ctx *f_ctx, u64 logical,
+			      u64 phys, u64 len, u32 flags)
+{
+	struct fiemap_extent *extent = f_ctx->fc_data;
+
+	if (f_ctx->fc_extents_max == 0) {
+		f_ctx->fc_extents_mapped++;
+		return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0;
+	}
+
+	if (f_ctx->fc_extents_mapped >= f_ctx->fc_extents_max)
+		return 1;
+
+	if (flags & SET_UNKNOWN_FLAGS)
+		flags |= FIEMAP_EXTENT_UNKNOWN;
+	if (flags & SET_NO_UNMOUNTED_IO_FLAGS)
+		flags |= FIEMAP_EXTENT_ENCODED;
+	if (flags & SET_NOT_ALIGNED_FLAGS)
+		flags |= FIEMAP_EXTENT_NOT_ALIGNED;
+
+	extent->fe_logical = logical;
+	extent->fe_physical = phys;
+	extent->fe_length = len;
+	extent->fe_flags = flags;
+
+	f_ctx->fc_extents_mapped++;
+
+	if (f_ctx->fc_extents_mapped == f_ctx->fc_extents_max)
+		return 1;
+	return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0;
+}
 /**
  * fiemap_fill_next_extent - Fiemap helper function
  * @fieinfo:	Fiemap context passed into ->fiemap
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 4c6dee908a38..7f623a434cb0 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1704,6 +1704,8 @@ struct fiemap_ctx {
 	u64 fc_len;
 };
 
+int fiemap_fill_kernel_extent(struct fiemap_ctx *f_ctx, u64 logical,
+			      u64 phys, u64 len, u32 flags);
 int fiemap_fill_next_extent(struct fiemap_ctx *f_ctx, u64 logical,
 			    u64 phys, u64 len, u32 flags);
 int fiemap_check_flags(struct fiemap_ctx *f_ctx, u32 fs_flags);
-- 
2.17.1

[PATCH 19/20] xfs: Get rid of ->bmap

[Carlos Maiolino]

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

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 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 338b9d9984e0..26f5bb80d007 100644
--- a/fs/xfs/xfs_aops.c
+++ b/fs/xfs/xfs_aops.c
@@ -963,29 +963,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_reflink_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,
@@ -1024,7 +1001,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 3043e5ed6495..d836b9b84aae 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -618,7 +618,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.17.1

[PATCH 20/20] ext4: Get rid of ->bmap interface

[Carlos Maiolino]

Only user of ext4_bmap is FIBMAP interface, which is now using ->fiemap
infrastructure.
Time to get rid of ->bmap

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/ext4/inode.c | 73 -------------------------------------------------
 1 file changed, 73 deletions(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 05f01fbd9c7f..87960b1734ac 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3275,75 +3275,6 @@ int ext4_alloc_da_blocks(struct inode *inode)
 	return filemap_flush(inode->i_mapping);
 }
 
-/*
- * bmap() is special.  It gets used by applications such as lilo and by
- * the swapper to find the on-disk block of a specific piece of data.
- *
- * Naturally, this is dangerous if the block concerned is still in the
- * journal.  If somebody makes a swapfile on an ext4 data-journaling
- * filesystem and enables swap, then they may get a nasty shock when the
- * data getting swapped to that swapfile suddenly gets overwritten by
- * the original zero's written out previously to the journal and
- * awaiting writeback in the kernel's buffer cache.
- *
- * So, if we see any bmap calls here on a modified, data-journaled file,
- * take extra steps to flush any blocks which might be in the cache.
- */
-static sector_t ext4_bmap(struct address_space *mapping, sector_t block)
-{
-	struct inode *inode = mapping->host;
-	journal_t *journal;
-	int err;
-
-	/*
-	 * We can get here for an inline file via the FIBMAP ioctl
-	 */
-	if (ext4_has_inline_data(inode))
-		return 0;
-
-	if (mapping_tagged(mapping, PAGECACHE_TAG_DIRTY) &&
-			test_opt(inode->i_sb, DELALLOC)) {
-		/*
-		 * With delalloc we want to sync the file
-		 * so that we can make sure we allocate
-		 * blocks for file
-		 */
-		filemap_write_and_wait(mapping);
-	}
-
-	if (EXT4_JOURNAL(inode) &&
-	    ext4_test_inode_state(inode, EXT4_STATE_JDATA)) {
-		/*
-		 * This is a REALLY heavyweight approach, but the use of
-		 * bmap on dirty files is expected to be extremely rare:
-		 * only if we run lilo or swapon on a freshly made file
-		 * do we expect this to happen.
-		 *
-		 * (bmap requires CAP_SYS_RAWIO so this does not
-		 * represent an unprivileged user DOS attack --- we'd be
-		 * in trouble if mortal users could trigger this path at
-		 * will.)
-		 *
-		 * NB. EXT4_STATE_JDATA is not set on files other than
-		 * regular files.  If somebody wants to bmap a directory
-		 * or symlink and gets confused because the buffer
-		 * hasn't yet been flushed to disk, they deserve
-		 * everything they get.
-		 */
-
-		ext4_clear_inode_state(inode, EXT4_STATE_JDATA);
-		journal = EXT4_JOURNAL(inode);
-		jbd2_journal_lock_updates(journal);
-		err = jbd2_journal_flush(journal);
-		jbd2_journal_unlock_updates(journal);
-
-		if (err)
-			return 0;
-	}
-
-	return generic_block_bmap(mapping, block, ext4_get_block);
-}
-
 static int ext4_readpage(struct file *file, struct page *page)
 {
 	int ret = -EAGAIN;
@@ -3937,7 +3868,6 @@ static const struct address_space_operations ext4_aops = {
 	.write_begin		= ext4_write_begin,
 	.write_end		= ext4_write_end,
 	.set_page_dirty		= ext4_set_page_dirty,
-	.bmap			= ext4_bmap,
 	.invalidatepage		= ext4_invalidatepage,
 	.releasepage		= ext4_releasepage,
 	.direct_IO		= ext4_direct_IO,
@@ -3954,7 +3884,6 @@ static const struct address_space_operations ext4_journalled_aops = {
 	.write_begin		= ext4_write_begin,
 	.write_end		= ext4_journalled_write_end,
 	.set_page_dirty		= ext4_journalled_set_page_dirty,
-	.bmap			= ext4_bmap,
 	.invalidatepage		= ext4_journalled_invalidatepage,
 	.releasepage		= ext4_releasepage,
 	.direct_IO		= ext4_direct_IO,
@@ -3970,7 +3899,6 @@ static const struct address_space_operations ext4_da_aops = {
 	.write_begin		= ext4_da_write_begin,
 	.write_end		= ext4_da_write_end,
 	.set_page_dirty		= ext4_set_page_dirty,
-	.bmap			= ext4_bmap,
 	.invalidatepage		= ext4_da_invalidatepage,
 	.releasepage		= ext4_releasepage,
 	.direct_IO		= ext4_direct_IO,
@@ -3983,7 +3911,6 @@ static const struct address_space_operations ext4_dax_aops = {
 	.writepages		= ext4_dax_writepages,
 	.direct_IO		= noop_direct_IO,
 	.set_page_dirty		= noop_set_page_dirty,
-	.bmap			= ext4_bmap,
 	.invalidatepage		= noop_invalidatepage,
 };
 
-- 
2.17.1

Re: [PATCH 09/20] f2fs: Remove direct usage of fiemap_extent_info

[Chao Yu]

On 2018/10/30 21:18, Carlos Maiolino wrote:
> struct fiemap-extent_info will be gone on future patches, remove its direct usage
> from f2fs
> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,

Re: [PATCH 07/20] fiemap: Move fiemap flags to fiemap_ctx

[Andreas Dilger]

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

On Oct 30, 2018, at 7:18 AM, Carlos Maiolino <cmaiolino@redhat.com> wrote:
> 
> struct fiemap_extent_info, will be gone in a later patch, but we still need the flags.
> So, move the fiemap flags up to the new context structure fiemap_ctx.

Doesn't this mostly boil down to renaming fiemap_extent_info to fiemap_ctx, if you
are moving all of the fields from one to the other?

> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index ebe5aae69c44..77fd8587b8e7 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -8619,7 +8619,7 @@ static int btrfs_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
> 	u64 len	  = f_ctx->fc_len;
> 	int	ret;
> 
> -	ret = fiemap_check_flags(fieinfo, BTRFS_FIEMAP_FLAGS);
> +	ret = fiemap_check_flags(f_ctx, BTRFS_FIEMAP_FLAGS);

The other minor nit about this patch series is that "f_ctx" is not a very
descriptive variable name.  How about "fiemap_ctx"?  A bit longer, but it
is immediately clear to the reader what it is.

Cheers, Andreas

> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index beccae768dac..d52aafc34e25 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -5063,7 +5063,7 @@ int ext4_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
> 			return error;
> 	}
> 
> -	if (fieinfo->fi_flags & FIEMAP_FLAG_CACHE) {
> +	if (f_ctx->fc_flags & FIEMAP_FLAG_CACHE) {
> 		error = ext4_ext_precache(inode);
> 		if (error)
> 			return error;
> @@ -5073,10 +5073,10 @@ int ext4_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
> 	if (!(ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)))
> 		return generic_block_fiemap(inode, f_ctx, ext4_get_block);
> 
> -	if (fiemap_check_flags(fieinfo, EXT4_FIEMAP_FLAGS))
> +	if (fiemap_check_flags(f_ctx, EXT4_FIEMAP_FLAGS))
> 		return -EBADR;
> 
> -	if (fieinfo->fi_flags & FIEMAP_FLAG_XATTR) {
> +	if (f_ctx->fc_flags & FIEMAP_FLAG_XATTR) {
> 		error = ext4_xattr_fiemap(inode, fieinfo);
> 	} else {
> 		ext4_lblk_t len_blks;
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 0f57403feb92..c552e2d1dfbb 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1409,19 +1409,19 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
> 	u32 flags = 0;
> 	int ret = 0;
> 
> -	if (fieinfo->fi_flags & FIEMAP_FLAG_CACHE) {
> +	if (f_ctx->fc_flags & FIEMAP_FLAG_CACHE) {
> 		ret = f2fs_precache_extents(inode);
> 		if (ret)
> 			return ret;
> 	}
> 
> -	ret = fiemap_check_flags(fieinfo, FIEMAP_FLAG_SYNC | FIEMAP_FLAG_XATTR);
> +	ret = fiemap_check_flags(f_ctx, FIEMAP_FLAG_SYNC | FIEMAP_FLAG_XATTR);
> 	if (ret)
> 		return ret;
> 
> 	inode_lock(inode);
> 
> -	if (fieinfo->fi_flags & FIEMAP_FLAG_XATTR) {
> +	if (f_ctx->fc_flags & FIEMAP_FLAG_XATTR) {
> 		ret = f2fs_xattr_fiemap(inode, fieinfo);
> 		goto out;
> 	}
> diff --git a/fs/ioctl.c b/fs/ioctl.c
> index ddcc4f9a9cf1..77af3b116972 100644
> --- a/fs/ioctl.c
> +++ b/fs/ioctl.c
> @@ -139,13 +139,13 @@ EXPORT_SYMBOL(fiemap_fill_next_extent);
>  *
>  * Returns 0 on success, -EBADR on bad flags.
>  */
> -int fiemap_check_flags(struct fiemap_extent_info *fieinfo, u32 fs_flags)
> +int fiemap_check_flags(struct fiemap_ctx *f_ctx, u32 fs_flags)
> {
> 	u32 incompat_flags;
> 
> -	incompat_flags = fieinfo->fi_flags & ~(FIEMAP_FLAGS_COMPAT & fs_flags);
> +	incompat_flags = f_ctx->fc_flags & ~(FIEMAP_FLAGS_COMPAT & fs_flags);
> 	if (incompat_flags) {
> -		fieinfo->fi_flags = incompat_flags;
> +		f_ctx->fc_flags = incompat_flags;
> 		return -EBADR;
> 	}
> 	return 0;
> @@ -199,25 +199,24 @@ static int ioctl_fiemap(struct file *filp, unsigned long arg)
> 	if (error)
> 		return error;
> 
> -	fieinfo.fi_flags = fiemap.fm_flags;
> 	fieinfo.fi_extents_max = fiemap.fm_extent_count;
> 	fieinfo.fi_extents_start = ufiemap->fm_extents;
> 
> +	f_ctx.fc_flags = fiemap.fm_flags;
> +	f_ctx.fc_data = &fieinfo;
> +	f_ctx.fc_start = fiemap.fm_start;
> +	f_ctx.fc_len = len;
> +
> 	if (fiemap.fm_extent_count != 0 &&
> 	    !access_ok(VERIFY_WRITE, fieinfo.fi_extents_start,
> 		       fieinfo.fi_extents_max * sizeof(struct fiemap_extent)))
> 		return -EFAULT;
> 
> -	if (fieinfo.fi_flags & FIEMAP_FLAG_SYNC)
> +	if (f_ctx.fc_flags & FIEMAP_FLAG_SYNC)
> 		filemap_write_and_wait(inode->i_mapping);
> 
> -	f_ctx.fc_flags = fieinfo.fi_flags;
> -	f_ctx.fc_data = &fieinfo;
> -	f_ctx.fc_start = fiemap.fm_start;
> -	f_ctx.fc_len = len;
> -
> 	error = inode->i_op->fiemap(inode, &f_ctx);
> -	fiemap.fm_flags = fieinfo.fi_flags;
> +	fiemap.fm_flags = f_ctx.fc_flags;
> 	fiemap.fm_mapped_extents = fieinfo.fi_extents_mapped;
> 	if (copy_to_user(ufiemap, &fiemap, sizeof(fiemap)))
> 		error = -EFAULT;
> @@ -298,7 +297,7 @@ int __generic_block_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx,
> 	bool past_eof = false, whole_file = false;
> 	int ret = 0;
> 
> -	ret = fiemap_check_flags(fieinfo, FIEMAP_FLAG_SYNC);
> +	ret = fiemap_check_flags(f_ctx, FIEMAP_FLAG_SYNC);
> 	if (ret)
> 		return ret;
> 
> diff --git a/fs/iomap.c b/fs/iomap.c
> index 42b131e1c955..5afca566c2a9 100644
> --- a/fs/iomap.c
> +++ b/fs/iomap.c
> @@ -1166,11 +1166,11 @@ int iomap_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx,
> 	ctx.fi = fi;
> 	ctx.prev.type = IOMAP_HOLE;
> 
> -	ret = fiemap_check_flags(fi, FIEMAP_FLAG_SYNC);
> +	ret = fiemap_check_flags(f_ctx, FIEMAP_FLAG_SYNC);
> 	if (ret)
> 		return ret;
> 
> -	if (fi->fi_flags & FIEMAP_FLAG_SYNC) {
> +	if (f_ctx->fc_flags & FIEMAP_FLAG_SYNC) {
> 		ret = filemap_write_and_wait(inode->i_mapping);
> 		if (ret)
> 			return ret;
> diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
> index 529262f0ab7d..a8040ed52bd5 100644
> --- a/fs/nilfs2/inode.c
> +++ b/fs/nilfs2/inode.c
> @@ -1007,7 +1007,7 @@ int nilfs_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
> 	unsigned int blkbits = inode->i_blkbits;
> 	int ret, n;
> 
> -	ret = fiemap_check_flags(fieinfo, FIEMAP_FLAG_SYNC);
> +	ret = fiemap_check_flags(f_ctx, FIEMAP_FLAG_SYNC);
> 	if (ret)
> 		return ret;
> 
> diff --git a/fs/ocfs2/extent_map.c b/fs/ocfs2/extent_map.c
> index 1944f5659267..5ba98d7a7a42 100644
> --- a/fs/ocfs2/extent_map.c
> +++ b/fs/ocfs2/extent_map.c
> @@ -762,7 +762,7 @@ int ocfs2_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
> 	struct buffer_head *di_bh = NULL;
> 	struct ocfs2_extent_rec rec;
> 
> -	ret = fiemap_check_flags(fieinfo, OCFS2_FIEMAP_FLAGS);
> +	ret = fiemap_check_flags(f_ctx, OCFS2_FIEMAP_FLAGS);
> 	if (ret)
> 		return ret;
> 
> diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
> index e56606c1ba92..3cef3b9b1854 100644
> --- a/fs/overlayfs/inode.c
> +++ b/fs/overlayfs/inode.c
> @@ -458,7 +458,6 @@ int ovl_update_time(struct inode *inode, struct timespec64 *ts, int flags)
> 
> static int ovl_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
> {
> -	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
> 	int err;
> 	struct inode *realinode = ovl_inode_real(inode);
> 	const struct cred *old_cred;
> @@ -468,7 +467,7 @@ static int ovl_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
> 
> 	old_cred = ovl_override_creds(inode->i_sb);
> 
> -	if (fieinfo->fi_flags & FIEMAP_FLAG_SYNC)
> +	if (f_ctx->fc_flags & FIEMAP_FLAG_SYNC)
> 		filemap_write_and_wait(realinode->i_mapping);
> 
> 	err = realinode->i_op->fiemap(realinode, f_ctx);
> diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> index 22c479862ba6..d859558de2fe 100644
> --- a/fs/xfs/xfs_iops.c
> +++ b/fs/xfs/xfs_iops.c
> @@ -1095,14 +1095,13 @@ xfs_vn_fiemap(
> 	struct inode		*inode,
> 	struct fiemap_ctx	*f_ctx)
> {
> -	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
> 	u64 start  = f_ctx->fc_start;
> 	u64 length = f_ctx->fc_len;
> 	int error;
> 
> 	xfs_ilock(XFS_I(inode), XFS_IOLOCK_SHARED);
> -	if (fieinfo->fi_flags & FIEMAP_FLAG_XATTR) {
> -		fieinfo->fi_flags &= ~FIEMAP_FLAG_XATTR;
> +	if (f_ctx->fc_flags & FIEMAP_FLAG_XATTR) {
> +		f_ctx->fc_flags &= ~FIEMAP_FLAG_XATTR;
> 		error = iomap_fiemap(inode, f_ctx, start, length,
> 				&xfs_xattr_iomap_ops);
> 	} else {
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index bd8e8a7dfd59..fb060123acff 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1692,7 +1692,6 @@ extern bool may_open_dev(const struct path *path);
> struct fiemap_ctx;
> 
> 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
> @@ -1703,7 +1702,7 @@ typedef int (*fiemap_fill_cb)(struct fiemap_ctx *f_ctx, u64 logical,
> 			      u64 phys, u64 len, u32 flags);
> 
> struct fiemap_ctx {
> -	unsigned int fc_flags;
> +	unsigned int fc_flags;	/* Flags as passed from user */
> 	void *fc_data;
> 	fiemap_fill_cb fc_cb; /* Unused by now */
> 	u64 fc_start;
> @@ -1712,7 +1711,7 @@ struct fiemap_ctx {
> 
> 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);
> +int fiemap_check_flags(struct fiemap_ctx *f_ctx, u32 fs_flags);
> 
> /*
>  * File types
> --
> 2.17.1
> 


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

Re: [PATCH 08/20] ext4: Remove direct usage of fiemap_extent_info

[Andreas Dilger]

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

On Oct 30, 2018, at 7:18 AM, Carlos Maiolino <cmaiolino@redhat.com> wrote:
> 
> struct fiemap_extent_info will be gone in later patches, remove its direct usage
> from Ext4.
> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---
> fs/ext4/ext4.h    |  2 +-
> fs/ext4/extents.c | 19 ++++++++++---------
> fs/ext4/inline.c  |  7 ++++---
> 3 files changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index acc1ea0e9f40..c7ad6db930f5 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -3023,7 +3023,7 @@ extern struct buffer_head *ext4_get_first_inline_block(struct inode *inode,
> 					struct ext4_dir_entry_2 **parent_de,
> 					int *retval);
> extern int ext4_inline_data_fiemap(struct inode *inode,
> -				   struct fiemap_extent_info *fieinfo,
> +				   struct fiemap_ctx *f_ctx,
> 				   int *has_inline, __u64 start, __u64 len);
> 
> struct iomap;
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index d52aafc34e25..11ee46aff677 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -2151,7 +2151,7 @@ int ext4_ext_insert_extent(handle_t *handle, struct inode *inode,
> 
> static int ext4_fill_fiemap_extents(struct inode *inode,
> 				    ext4_lblk_t block, ext4_lblk_t num,
> -				    struct fiemap_extent_info *fieinfo)
> +				    struct fiemap_ctx *f_ctx)
> {
> 	struct ext4_ext_path *path = NULL;
> 	struct ext4_extent *ex;
> @@ -2277,7 +2277,8 @@ static int ext4_fill_fiemap_extents(struct inode *inode,
> 		}
> 
> 		if (exists) {
> -			err = fiemap_fill_next_extent(fieinfo,
> +			err = fiemap_fill_next_extent(
> +				(struct fiemap_extent_info *)f_ctx->fc_data,

No need to cast void pointer.  This also makes me wonder why you didn't name
fc_data as fc_extent_info instead of making it a void pointer?  I didn't see
any users where it isn't a pointer to struct fiemap_extent_info?

The same is true of all the other filesystem-specific patches - there is no
need to cast from a void *.  Is there a later user where this is used as a
generic data pointer, or is it always going to be a fiemap_extent_info, and
later a fiemap_ctx structure?

> @@ -5040,14 +5041,14 @@ static int ext4_xattr_fiemap(struct inode *inode,
> 	}
> 
> 	if (physical)
> -		error = fiemap_fill_next_extent(fieinfo, 0, physical,
> -						length, flags);
> +		error = fiemap_fill_next_extent(
> +			(struct fiemap_extent_info *)f_ctx->fc_data,

Same...

> diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
> index 9c4bac18cc6c..7b9b0da60d54 100644
> --- a/fs/ext4/inline.c
> +++ b/fs/ext4/inline.c
> @@ -1888,8 +1888,9 @@ int ext4_inline_data_fiemap(struct inode *inode,
> 	physical += offsetof(struct ext4_inode, i_block);
> 
> 	if (physical)
> -		error = fiemap_fill_next_extent(fieinfo, start, physical,
> -						inline_len, flags);
> +		error = fiemap_fill_next_extent(
> +			(struct fiemap_extent_info *)f_ctx->fc_data,

Same...


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

Re: [PATCH 15/20] fiemap: Start using new callback from fiemap_ctx

[Andreas Dilger]

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

On Oct 30, 2018, at 7:18 AM, Carlos Maiolino <cmaiolino@redhat.com> wrote:
> 
> Now that fiemap methods are updated to work based on the new fiemap_ctx
> structure, update the code to use the new callback.
> The current fiemap_fill_next_extent will be used for user calls of
> ->fiemap methods, as its done today, but passed in as a callback to
> fiemap_ctx. So, rename it to ifemap_fill_usr_extent().

Minor typo here s/ifemap/fiemap/

Couldn't this series have (mostly?) been achieved by just adding the
callback to fiemap_extent_info and using that everywhere?

Cheers, Andreas

> The 'new' fiemap_fill_next_extent() will now be just a helper to call
> fiemap_ctx callback.
> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---
> fs/ioctl.c         | 42 +++++++++++++++++++++++++-----------------
> include/linux/fs.h |  2 +-
> 2 files changed, 26 insertions(+), 18 deletions(-)
> 
> diff --git a/fs/ioctl.c b/fs/ioctl.c
> index 27f79b29cb07..85a7aec40e3d 100644
> --- a/fs/ioctl.c
> +++ b/fs/ioctl.c
> @@ -68,25 +68,10 @@ static int ioctl_fibmap(struct file *filp, int __user *p)
> 	return put_user(res, p);
> }
> 
> -/**
> - * 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_ctx *f_ctx, u64 logical,
> +int fiemap_fill_usr_extent(struct fiemap_ctx *f_ctx, u64 logical,
> 			    u64 phys, u64 len, u32 flags)
> {
> 	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
> @@ -124,8 +109,29 @@ int fiemap_fill_next_extent(struct fiemap_ctx *f_ctx, u64 logical,
> 		return 1;
> 	return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0;
> }
> -EXPORT_SYMBOL(fiemap_fill_next_extent);
> 
> +/**
> + * 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_ctx *f_ctx, u64 logical,
> +			    u64 phys, u64 len, u32 flags)
> +{
> +	return f_ctx->fc_cb(f_ctx, logical, phys, len, flags);
> +}
> +EXPORT_SYMBOL(fiemap_fill_next_extent);
> /**
>  * fiemap_check_flags - check validity of requested flags for fiemap
>  * @fieinfo:	Fiemap context passed into ->fiemap
> @@ -208,6 +214,8 @@ static int ioctl_fiemap(struct file *filp, unsigned long arg)
> 	f_ctx.fc_start = fiemap.fm_start;
> 	f_ctx.fc_len = len;
> 
> +	f_ctx.fc_cb = fiemap_fill_usr_extent;
> +
> 	if (fiemap.fm_extent_count != 0 &&
> 	    !access_ok(VERIFY_WRITE, fieinfo.fi_extents_start,
> 		       fieinfo.fi_extents_max * sizeof(struct fiemap_extent)))
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 945cfb3e06e4..9a538bc0dac2 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1704,7 +1704,7 @@ typedef int (*fiemap_fill_cb)(struct fiemap_ctx *f_ctx, u64 logical,
> struct fiemap_ctx {
> 	unsigned int fc_flags;	/* Flags as passed from user */
> 	void *fc_data;
> -	fiemap_fill_cb fc_cb; /* Unused by now */
> +	fiemap_fill_cb fc_cb;
> 	u64 fc_start;
> 	u64 fc_len;
> };
> --
> 2.17.1
> 


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

Re: [PATCH 17/20] fiemap: Get rid of fiemap_extent_info

[Andreas Dilger]

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

On Oct 30, 2018, at 7:18 AM, Carlos Maiolino <cmaiolino@redhat.com> wrote:
> 
> With the goal of using FIEMAP infra-structure for FIBMAP ioctl, and with
> the updates being done previously into the FIEMAP interface, the
> fiemap_extent_info structure is no longer necessary, so, move
> fi_extents_mapped and fi_extents_max up to fiemap_ctx. and use
> fiemap_ctx.fc_data to hold a pointer to struct fiemap_extent.

Having the "void *fc_data" pointer point to fiemap_extent is not
necessarily intuitive.  Why not just make it directly point to it:

      struct fiemap_extent *fc_extent;

Cheers, Andreas

> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---
> fs/btrfs/extent_io.c  |  3 +--
> fs/ioctl.c            | 29 +++++++++++++----------------
> include/linux/fs.h    |  9 ++-------
> include/linux/iomap.h |  1 -
> 4 files changed, 16 insertions(+), 26 deletions(-)
> 
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 6a8bc502bc04..88b8a4416dfc 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -4436,7 +4436,6 @@ int extent_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
> 	struct btrfs_path *path;
> 	struct btrfs_root *root = BTRFS_I(inode)->root;
> 	struct fiemap_cache cache = { 0 };
> -	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
> 	int end = 0;
> 	u64 em_start = 0;
> 	u64 em_len = 0;
> @@ -4561,7 +4560,7 @@ int extent_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
> 		} else if (em->block_start == EXTENT_MAP_DELALLOC) {
> 			flags |= (FIEMAP_EXTENT_DELALLOC |
> 				  FIEMAP_EXTENT_UNKNOWN);
> -		} else if (fieinfo->fi_extents_max) {
> +		} else if (f_ctx->fc_extents_max) {
> 			u64 bytenr = em->block_start -
> 				(em->start - em->orig_start);
> 
> diff --git a/fs/ioctl.c b/fs/ioctl.c
> index cbc1b21c4f7c..71d11201a06b 100644
> --- a/fs/ioctl.c
> +++ b/fs/ioctl.c
> @@ -78,17 +78,16 @@ static int ioctl_fibmap(struct file *filp, int __user *p)
> int fiemap_fill_usr_extent(struct fiemap_ctx *f_ctx, u64 logical,
> 			    u64 phys, u64 len, u32 flags)
> {
> -	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
> 	struct fiemap_extent extent;
> -	struct fiemap_extent __user *dest = fieinfo->fi_extents_start;
> +	struct fiemap_extent __user *dest = f_ctx->fc_data;
> 
> 	/* only count the extents */
> -	if (fieinfo->fi_extents_max == 0) {
> -		fieinfo->fi_extents_mapped++;
> +	if (f_ctx->fc_extents_max == 0) {
> +		f_ctx->fc_extents_mapped++;
> 		return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0;
> 	}
> 
> -	if (fieinfo->fi_extents_mapped >= fieinfo->fi_extents_max)
> +	if (f_ctx->fc_extents_mapped >= f_ctx->fc_extents_max)
> 		return 1;
> 
> 	if (flags & SET_UNKNOWN_FLAGS)
> @@ -104,12 +103,12 @@ int fiemap_fill_usr_extent(struct fiemap_ctx *f_ctx, u64 logical,
> 	extent.fe_length = len;
> 	extent.fe_flags = flags;
> 
> -	dest += fieinfo->fi_extents_mapped;
> +	dest += f_ctx->fc_extents_mapped;
> 	if (copy_to_user(dest, &extent, sizeof(extent)))
> 		return -EFAULT;
> 
> -	fieinfo->fi_extents_mapped++;
> -	if (fieinfo->fi_extents_mapped == fieinfo->fi_extents_max)
> +	f_ctx->fc_extents_mapped++;
> +	if (f_ctx->fc_extents_mapped == f_ctx->fc_extents_max)
> 		return 1;
> 	return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0;
> }
> @@ -189,7 +188,6 @@ static int ioctl_fiemap(struct file *filp, unsigned long arg)
> {
> 	struct fiemap fiemap;
> 	struct fiemap __user *ufiemap = (struct fiemap __user *) arg;
> -	struct fiemap_extent_info fieinfo = { 0, };
> 	struct inode *inode = file_inode(filp);
> 	struct super_block *sb = inode->i_sb;
> 	struct fiemap_ctx f_ctx;
> @@ -210,19 +208,18 @@ static int ioctl_fiemap(struct file *filp, unsigned long arg)
> 	if (error)
> 		return error;
> 
> -	fieinfo.fi_extents_max = fiemap.fm_extent_count;
> -	fieinfo.fi_extents_start = ufiemap->fm_extents;
> -
> +	f_ctx.fc_extents_max = fiemap.fm_extent_count;
> +	f_ctx.fc_data = ufiemap->fm_extents;
> +	f_ctx.fc_extents_mapped = 0;
> 	f_ctx.fc_flags = fiemap.fm_flags;
> -	f_ctx.fc_data = &fieinfo;
> 	f_ctx.fc_start = fiemap.fm_start;
> 	f_ctx.fc_len = len;
> 
> 	f_ctx.fc_cb = fiemap_fill_usr_extent;
> 
> 	if (fiemap.fm_extent_count != 0 &&
> -	    !access_ok(VERIFY_WRITE, fieinfo.fi_extents_start,
> -		       fieinfo.fi_extents_max * sizeof(struct fiemap_extent)))
> +	    !access_ok(VERIFY_WRITE, f_ctx.fc_data,
> +		       f_ctx.fc_extents_max * sizeof(struct fiemap_extent)))
> 		return -EFAULT;
> 
> 	if (f_ctx.fc_flags & FIEMAP_FLAG_SYNC)
> @@ -230,7 +227,7 @@ static int ioctl_fiemap(struct file *filp, unsigned long arg)
> 
> 	error = inode->i_op->fiemap(inode, &f_ctx);
> 	fiemap.fm_flags = f_ctx.fc_flags;
> -	fiemap.fm_mapped_extents = fieinfo.fi_extents_mapped;
> +	fiemap.fm_mapped_extents = f_ctx.fc_extents_mapped;
> 	if (copy_to_user(ufiemap, &fiemap, sizeof(fiemap)))
> 		error = -EFAULT;
> 
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 9a538bc0dac2..4c6dee908a38 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1691,18 +1691,13 @@ extern bool may_open_dev(const struct path *path);
> 
> struct fiemap_ctx;
> 
> -struct fiemap_extent_info {
> -	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 */
> -};
> -
> typedef int (*fiemap_fill_cb)(struct fiemap_ctx *f_ctx, u64 logical,
> 			      u64 phys, u64 len, u32 flags);
> 
> struct fiemap_ctx {
> 	unsigned int fc_flags;	/* Flags as passed from user */
> +	unsigned int fc_extents_mapped;	/* Number of mapped extents */
> +	unsigned int fc_extents_max;	/* Size of fiemap_extent array */
> 	void *fc_data;
> 	fiemap_fill_cb fc_cb;
> 	u64 fc_start;
> diff --git a/include/linux/iomap.h b/include/linux/iomap.h
> index 7ce84be499d6..21643bcf6104 100644
> --- a/include/linux/iomap.h
> +++ b/include/linux/iomap.h
> @@ -9,7 +9,6 @@
> #include <linux/mm_types.h>
> 
> struct address_space;
> -struct fiemap_extent_info;
> struct inode;
> struct iov_iter;
> struct kiocb;
> --
> 2.17.1
> 


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

Re: [PATCH 18/20] Use FIEMAP for FIBMAP calls

[Andreas Dilger]

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

On Oct 30, 2018, at 7:18 AM, Carlos Maiolino <cmaiolino@redhat.com> wrote:
> 
> Enables the usage of FIEMAP ioctl infrastructure to handle FIBMAP calls,
> from this point, ->bmap() methods can start to be removed.
> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---
> 
> This patch can be improved, since fiemap_fill_kernel_extent and
> fiemap_fill_usr_extent shares a lot of common code, which still is WIP.
> 
> fs/inode.c         | 35 ++++++++++++++++++++++++++++++-----
> fs/ioctl.c         | 31 +++++++++++++++++++++++++++++++
> include/linux/fs.h |  2 ++
> 3 files changed, 63 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/inode.c b/fs/inode.c
> index d09a6f4f0335..389c2165959c 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -1591,11 +1591,36 @@ EXPORT_SYMBOL(iput);
>  */
> int bmap(struct inode *inode, sector_t *block)
> {
> -	if (!inode->i_mapping->a_ops->bmap)
> -		return -EINVAL;
> -
> -	*block = inode->i_mapping->a_ops->bmap(inode->i_mapping, *block);
> -	return 0;
> +	struct fiemap_ctx f_ctx;
> +	struct fiemap_extent fextent;
> +	u64 start = *block << inode->i_blkbits;
> +	int error = -EINVAL;
> +
> +	if (inode->i_op->fiemap) {
> +		fextent.fe_logical = 0;
> +		fextent.fe_physical = 0;
> +		f_ctx.fc_extents_max = 1;
> +		f_ctx.fc_extents_mapped = 0;
> +		f_ctx.fc_data = &fextent;
> +		f_ctx.fc_start = start;
> +		f_ctx.fc_len = 1;

Should this be "fc_len = 1 << inode->i_blkbits" to map a single block?
On the one hand, this might return multiple blocks, but on the other
hand FIBMAP shouldn't be allowed if that is the case so this code should
detect that situation and consider it an error.

Cheers, Andreas

> +		f_ctx.fc_flags = 0;
> +		f_ctx.fc_cb = fiemap_fill_kernel_extent;
> +
> +		error = inode->i_op->fiemap(inode, &f_ctx);
> +
> +		if (error)
> +			goto out;
> +
> +		*block = (fextent.fe_physical +
> +			(start - fextent.fe_logical)) >> inode->i_blkbits;
> +
> +	} else if (inode->i_mapping->a_ops->bmap) {
> +		*block = inode->i_mapping->a_ops->bmap(inode->i_mapping, *block);
> +		error = 0;
> +	}
> +out:
> +	return error;
> }
> EXPORT_SYMBOL(bmap);
> 
> diff --git a/fs/ioctl.c b/fs/ioctl.c
> index 71d11201a06b..dce710699b82 100644
> --- a/fs/ioctl.c
> +++ b/fs/ioctl.c
> @@ -113,6 +113,37 @@ int fiemap_fill_usr_extent(struct fiemap_ctx *f_ctx, u64 logical,
> 	return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0;
> }
> 
> +int fiemap_fill_kernel_extent(struct fiemap_ctx *f_ctx, u64 logical,
> +			      u64 phys, u64 len, u32 flags)
> +{
> +	struct fiemap_extent *extent = f_ctx->fc_data;
> +
> +	if (f_ctx->fc_extents_max == 0) {
> +		f_ctx->fc_extents_mapped++;
> +		return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0;
> +	}
> +
> +	if (f_ctx->fc_extents_mapped >= f_ctx->fc_extents_max)
> +		return 1;
> +
> +	if (flags & SET_UNKNOWN_FLAGS)
> +		flags |= FIEMAP_EXTENT_UNKNOWN;
> +	if (flags & SET_NO_UNMOUNTED_IO_FLAGS)
> +		flags |= FIEMAP_EXTENT_ENCODED;
> +	if (flags & SET_NOT_ALIGNED_FLAGS)
> +		flags |= FIEMAP_EXTENT_NOT_ALIGNED;
> +
> +	extent->fe_logical = logical;
> +	extent->fe_physical = phys;
> +	extent->fe_length = len;
> +	extent->fe_flags = flags;
> +
> +	f_ctx->fc_extents_mapped++;
> +
> +	if (f_ctx->fc_extents_mapped == f_ctx->fc_extents_max)
> +		return 1;
> +	return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0;
> +}
> /**
>  * fiemap_fill_next_extent - Fiemap helper function
>  * @fieinfo:	Fiemap context passed into ->fiemap
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 4c6dee908a38..7f623a434cb0 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1704,6 +1704,8 @@ struct fiemap_ctx {
> 	u64 fc_len;
> };
> 
> +int fiemap_fill_kernel_extent(struct fiemap_ctx *f_ctx, u64 logical,
> +			      u64 phys, u64 len, u32 flags);
> int fiemap_fill_next_extent(struct fiemap_ctx *f_ctx, u64 logical,
> 			    u64 phys, u64 len, u32 flags);
> int fiemap_check_flags(struct fiemap_ctx *f_ctx, u32 fs_flags);
> --
> 2.17.1
> 


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

Re: [PATCH 07/20] fiemap: Move fiemap flags to fiemap_ctx

[Carlos Maiolino]

Hi Andreas,

thanks for the review.

On Mon, Nov 05, 2018 at 03:12:07PM -0700, Andreas Dilger wrote:
> On Oct 30, 2018, at 7:18 AM, Carlos Maiolino <cmaiolino@redhat.com> wrote:
> > 
> > struct fiemap_extent_info, will be gone in a later patch, but we still need the flags.
> > So, move the fiemap flags up to the new context structure fiemap_ctx.
> 
> Doesn't this mostly boil down to renaming fiemap_extent_info to fiemap_ctx, if you
> are moving all of the fields from one to the other?

Not really, fiemap_extent_info is removed at the end of the series, and by doing
it directly, I'd need to write a big patch changing all users of
fiemap_extent_info in a single patch, or break the kernel build between patches.
I usually try to avoid a single patch touching several different filesystems, so
that's why I basically slowly replaced fiemap_extent_info, until it get removed
at the end of the series.
> 
> > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> > index ebe5aae69c44..77fd8587b8e7 100644
> > --- a/fs/btrfs/inode.c
> > +++ b/fs/btrfs/inode.c
> > @@ -8619,7 +8619,7 @@ static int btrfs_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
> > 	u64 len	  = f_ctx->fc_len;
> > 	int	ret;
> > 
> > -	ret = fiemap_check_flags(fieinfo, BTRFS_FIEMAP_FLAGS);
> > +	ret = fiemap_check_flags(f_ctx, BTRFS_FIEMAP_FLAGS);
> 
> The other minor nit about this patch series is that "f_ctx" is not a very
> descriptive variable name.  How about "fiemap_ctx"?  A bit longer, but it
> is immediately clear to the reader what it is.

I don't really mind, f_ctx, fmap_ctx, etc. having the pointer the same name as
the struct doesn't look good to me, but particularly I don't mind.

> 
> Cheers, Andreas
> 
> > diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> > index beccae768dac..d52aafc34e25 100644
> > --- a/fs/ext4/extents.c
> > +++ b/fs/ext4/extents.c
> > @@ -5063,7 +5063,7 @@ int ext4_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
> > 			return error;
> > 	}
> > 
> > -	if (fieinfo->fi_flags & FIEMAP_FLAG_CACHE) {
> > +	if (f_ctx->fc_flags & FIEMAP_FLAG_CACHE) {
> > 		error = ext4_ext_precache(inode);
> > 		if (error)
> > 			return error;
> > @@ -5073,10 +5073,10 @@ int ext4_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
> > 	if (!(ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)))
> > 		return generic_block_fiemap(inode, f_ctx, ext4_get_block);
> > 
> > -	if (fiemap_check_flags(fieinfo, EXT4_FIEMAP_FLAGS))
> > +	if (fiemap_check_flags(f_ctx, EXT4_FIEMAP_FLAGS))
> > 		return -EBADR;
> > 
> > -	if (fieinfo->fi_flags & FIEMAP_FLAG_XATTR) {
> > +	if (f_ctx->fc_flags & FIEMAP_FLAG_XATTR) {
> > 		error = ext4_xattr_fiemap(inode, fieinfo);
> > 	} else {
> > 		ext4_lblk_t len_blks;
> > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > index 0f57403feb92..c552e2d1dfbb 100644
> > --- a/fs/f2fs/data.c
> > +++ b/fs/f2fs/data.c
> > @@ -1409,19 +1409,19 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
> > 	u32 flags = 0;
> > 	int ret = 0;
> > 
> > -	if (fieinfo->fi_flags & FIEMAP_FLAG_CACHE) {
> > +	if (f_ctx->fc_flags & FIEMAP_FLAG_CACHE) {
> > 		ret = f2fs_precache_extents(inode);
> > 		if (ret)
> > 			return ret;
> > 	}
> > 
> > -	ret = fiemap_check_flags(fieinfo, FIEMAP_FLAG_SYNC | FIEMAP_FLAG_XATTR);
> > +	ret = fiemap_check_flags(f_ctx, FIEMAP_FLAG_SYNC | FIEMAP_FLAG_XATTR);
> > 	if (ret)
> > 		return ret;
> > 
> > 	inode_lock(inode);
> > 
> > -	if (fieinfo->fi_flags & FIEMAP_FLAG_XATTR) {
> > +	if (f_ctx->fc_flags & FIEMAP_FLAG_XATTR) {
> > 		ret = f2fs_xattr_fiemap(inode, fieinfo);
> > 		goto out;
> > 	}
> > diff --git a/fs/ioctl.c b/fs/ioctl.c
> > index ddcc4f9a9cf1..77af3b116972 100644
> > --- a/fs/ioctl.c
> > +++ b/fs/ioctl.c
> > @@ -139,13 +139,13 @@ EXPORT_SYMBOL(fiemap_fill_next_extent);
> >  *
> >  * Returns 0 on success, -EBADR on bad flags.
> >  */
> > -int fiemap_check_flags(struct fiemap_extent_info *fieinfo, u32 fs_flags)
> > +int fiemap_check_flags(struct fiemap_ctx *f_ctx, u32 fs_flags)
> > {
> > 	u32 incompat_flags;
> > 
> > -	incompat_flags = fieinfo->fi_flags & ~(FIEMAP_FLAGS_COMPAT & fs_flags);
> > +	incompat_flags = f_ctx->fc_flags & ~(FIEMAP_FLAGS_COMPAT & fs_flags);
> > 	if (incompat_flags) {
> > -		fieinfo->fi_flags = incompat_flags;
> > +		f_ctx->fc_flags = incompat_flags;
> > 		return -EBADR;
> > 	}
> > 	return 0;
> > @@ -199,25 +199,24 @@ static int ioctl_fiemap(struct file *filp, unsigned long arg)
> > 	if (error)
> > 		return error;
> > 
> > -	fieinfo.fi_flags = fiemap.fm_flags;
> > 	fieinfo.fi_extents_max = fiemap.fm_extent_count;
> > 	fieinfo.fi_extents_start = ufiemap->fm_extents;
> > 
> > +	f_ctx.fc_flags = fiemap.fm_flags;
> > +	f_ctx.fc_data = &fieinfo;
> > +	f_ctx.fc_start = fiemap.fm_start;
> > +	f_ctx.fc_len = len;
> > +
> > 	if (fiemap.fm_extent_count != 0 &&
> > 	    !access_ok(VERIFY_WRITE, fieinfo.fi_extents_start,
> > 		       fieinfo.fi_extents_max * sizeof(struct fiemap_extent)))
> > 		return -EFAULT;
> > 
> > -	if (fieinfo.fi_flags & FIEMAP_FLAG_SYNC)
> > +	if (f_ctx.fc_flags & FIEMAP_FLAG_SYNC)
> > 		filemap_write_and_wait(inode->i_mapping);
> > 
> > -	f_ctx.fc_flags = fieinfo.fi_flags;
> > -	f_ctx.fc_data = &fieinfo;
> > -	f_ctx.fc_start = fiemap.fm_start;
> > -	f_ctx.fc_len = len;
> > -
> > 	error = inode->i_op->fiemap(inode, &f_ctx);
> > -	fiemap.fm_flags = fieinfo.fi_flags;
> > +	fiemap.fm_flags = f_ctx.fc_flags;
> > 	fiemap.fm_mapped_extents = fieinfo.fi_extents_mapped;
> > 	if (copy_to_user(ufiemap, &fiemap, sizeof(fiemap)))
> > 		error = -EFAULT;
> > @@ -298,7 +297,7 @@ int __generic_block_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx,
> > 	bool past_eof = false, whole_file = false;
> > 	int ret = 0;
> > 
> > -	ret = fiemap_check_flags(fieinfo, FIEMAP_FLAG_SYNC);
> > +	ret = fiemap_check_flags(f_ctx, FIEMAP_FLAG_SYNC);
> > 	if (ret)
> > 		return ret;
> > 
> > diff --git a/fs/iomap.c b/fs/iomap.c
> > index 42b131e1c955..5afca566c2a9 100644
> > --- a/fs/iomap.c
> > +++ b/fs/iomap.c
> > @@ -1166,11 +1166,11 @@ int iomap_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx,
> > 	ctx.fi = fi;
> > 	ctx.prev.type = IOMAP_HOLE;
> > 
> > -	ret = fiemap_check_flags(fi, FIEMAP_FLAG_SYNC);
> > +	ret = fiemap_check_flags(f_ctx, FIEMAP_FLAG_SYNC);
> > 	if (ret)
> > 		return ret;
> > 
> > -	if (fi->fi_flags & FIEMAP_FLAG_SYNC) {
> > +	if (f_ctx->fc_flags & FIEMAP_FLAG_SYNC) {
> > 		ret = filemap_write_and_wait(inode->i_mapping);
> > 		if (ret)
> > 			return ret;
> > diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
> > index 529262f0ab7d..a8040ed52bd5 100644
> > --- a/fs/nilfs2/inode.c
> > +++ b/fs/nilfs2/inode.c
> > @@ -1007,7 +1007,7 @@ int nilfs_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
> > 	unsigned int blkbits = inode->i_blkbits;
> > 	int ret, n;
> > 
> > -	ret = fiemap_check_flags(fieinfo, FIEMAP_FLAG_SYNC);
> > +	ret = fiemap_check_flags(f_ctx, FIEMAP_FLAG_SYNC);
> > 	if (ret)
> > 		return ret;
> > 
> > diff --git a/fs/ocfs2/extent_map.c b/fs/ocfs2/extent_map.c
> > index 1944f5659267..5ba98d7a7a42 100644
> > --- a/fs/ocfs2/extent_map.c
> > +++ b/fs/ocfs2/extent_map.c
> > @@ -762,7 +762,7 @@ int ocfs2_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
> > 	struct buffer_head *di_bh = NULL;
> > 	struct ocfs2_extent_rec rec;
> > 
> > -	ret = fiemap_check_flags(fieinfo, OCFS2_FIEMAP_FLAGS);
> > +	ret = fiemap_check_flags(f_ctx, OCFS2_FIEMAP_FLAGS);
> > 	if (ret)
> > 		return ret;
> > 
> > diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
> > index e56606c1ba92..3cef3b9b1854 100644
> > --- a/fs/overlayfs/inode.c
> > +++ b/fs/overlayfs/inode.c
> > @@ -458,7 +458,6 @@ int ovl_update_time(struct inode *inode, struct timespec64 *ts, int flags)
> > 
> > static int ovl_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
> > {
> > -	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
> > 	int err;
> > 	struct inode *realinode = ovl_inode_real(inode);
> > 	const struct cred *old_cred;
> > @@ -468,7 +467,7 @@ static int ovl_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
> > 
> > 	old_cred = ovl_override_creds(inode->i_sb);
> > 
> > -	if (fieinfo->fi_flags & FIEMAP_FLAG_SYNC)
> > +	if (f_ctx->fc_flags & FIEMAP_FLAG_SYNC)
> > 		filemap_write_and_wait(realinode->i_mapping);
> > 
> > 	err = realinode->i_op->fiemap(realinode, f_ctx);
> > diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> > index 22c479862ba6..d859558de2fe 100644
> > --- a/fs/xfs/xfs_iops.c
> > +++ b/fs/xfs/xfs_iops.c
> > @@ -1095,14 +1095,13 @@ xfs_vn_fiemap(
> > 	struct inode		*inode,
> > 	struct fiemap_ctx	*f_ctx)
> > {
> > -	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
> > 	u64 start  = f_ctx->fc_start;
> > 	u64 length = f_ctx->fc_len;
> > 	int error;
> > 
> > 	xfs_ilock(XFS_I(inode), XFS_IOLOCK_SHARED);
> > -	if (fieinfo->fi_flags & FIEMAP_FLAG_XATTR) {
> > -		fieinfo->fi_flags &= ~FIEMAP_FLAG_XATTR;
> > +	if (f_ctx->fc_flags & FIEMAP_FLAG_XATTR) {
> > +		f_ctx->fc_flags &= ~FIEMAP_FLAG_XATTR;
> > 		error = iomap_fiemap(inode, f_ctx, start, length,
> > 				&xfs_xattr_iomap_ops);
> > 	} else {
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index bd8e8a7dfd59..fb060123acff 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -1692,7 +1692,6 @@ extern bool may_open_dev(const struct path *path);
> > struct fiemap_ctx;
> > 
> > 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
> > @@ -1703,7 +1702,7 @@ typedef int (*fiemap_fill_cb)(struct fiemap_ctx *f_ctx, u64 logical,
> > 			      u64 phys, u64 len, u32 flags);
> > 
> > struct fiemap_ctx {
> > -	unsigned int fc_flags;
> > +	unsigned int fc_flags;	/* Flags as passed from user */
> > 	void *fc_data;
> > 	fiemap_fill_cb fc_cb; /* Unused by now */
> > 	u64 fc_start;
> > @@ -1712,7 +1711,7 @@ struct fiemap_ctx {
> > 
> > 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);
> > +int fiemap_check_flags(struct fiemap_ctx *f_ctx, u32 fs_flags);
> > 
> > /*
> >  * File types
> > --
> > 2.17.1
> > 
> 
> 
> Cheers, Andreas
> 
> 
> 
> 
> 



-- 
Carlos

Re: [PATCH 08/20] ext4: Remove direct usage of fiemap_extent_info

[Carlos Maiolino]

> > static int ext4_fill_fiemap_extents(struct inode *inode,
> > 				    ext4_lblk_t block, ext4_lblk_t num,
> > -				    struct fiemap_extent_info *fieinfo)
> > +				    struct fiemap_ctx *f_ctx)
> > {
> > 	struct ext4_ext_path *path = NULL;
> > 	struct ext4_extent *ex;
> > @@ -2277,7 +2277,8 @@ static int ext4_fill_fiemap_extents(struct inode *inode,
> > 		}
> > 
> > 		if (exists) {
> > -			err = fiemap_fill_next_extent(fieinfo,
> > +			err = fiemap_fill_next_extent(
> > +				(struct fiemap_extent_info *)f_ctx->fc_data,
> 
> No need to cast void pointer.  This also makes me wonder why you didn't name
> fc_data as fc_extent_info instead of making it a void pointer?  I didn't see
> any users where it isn't a pointer to struct fiemap_extent_info?

On later patches, the void pointer is used to point to a userspace address or to
a kernel address, depending on the context. (userspace if FIEMAP, kernelspace if
FIBMAP).

> 
> The same is true of all the other filesystem-specific patches - there is no
> need to cast from a void *.  Is there a later user where this is used as a
> generic data pointer, or is it always going to be a fiemap_extent_info, and
> later a fiemap_ctx structure?

I believe I made the mistake to not quote the previous thread which led to this
patch, so, quoting it now:

https://www.spinics.net/lists/linux-fsdevel/msg131786.html

More specifically, I think the detailed information about the approach, was
discussed in the 2nd patch of the above RFC series:

https://www.spinics.net/lists/linux-fsdevel/msg131788.html


> 
> > @@ -5040,14 +5041,14 @@ static int ext4_xattr_fiemap(struct inode *inode,
> > 	}
> > 
> > 	if (physical)
> > -		error = fiemap_fill_next_extent(fieinfo, 0, physical,
> > -						length, flags);
> > +		error = fiemap_fill_next_extent(
> > +			(struct fiemap_extent_info *)f_ctx->fc_data,
> 
> Same...
> 

About the casts, I didn't remember if GCC allowed implicit casting when passing
it into functions (I remember variable assignment implicit cast though), so,
better safe than sorry, but I can remove it without problems in follow-up
patches.

Thanks

> > diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
> > index 9c4bac18cc6c..7b9b0da60d54 100644
> > --- a/fs/ext4/inline.c
> > +++ b/fs/ext4/inline.c
> > @@ -1888,8 +1888,9 @@ int ext4_inline_data_fiemap(struct inode *inode,
> > 	physical += offsetof(struct ext4_inode, i_block);
> > 
> > 	if (physical)
> > -		error = fiemap_fill_next_extent(fieinfo, start, physical,
> > -						inline_len, flags);
> > +		error = fiemap_fill_next_extent(
> > +			(struct fiemap_extent_info *)f_ctx->fc_data,
> 
> Same...
> 
> 
> Cheers, Andreas
> 
> 
> 
> 
> 



-- 
Carlos

Re: [PATCH 15/20] fiemap: Start using new callback from fiemap_ctx

[Carlos Maiolino]

On Mon, Nov 05, 2018 at 03:14:02PM -0700, Andreas Dilger wrote:
> On Oct 30, 2018, at 7:18 AM, Carlos Maiolino <cmaiolino@redhat.com> wrote:
> > 
> > Now that fiemap methods are updated to work based on the new fiemap_ctx
> > structure, update the code to use the new callback.
> > The current fiemap_fill_next_extent will be used for user calls of
> > ->fiemap methods, as its done today, but passed in as a callback to
> > fiemap_ctx. So, rename it to ifemap_fill_usr_extent().
> 
> Minor typo here s/ifemap/fiemap/
> 
> Couldn't this series have (mostly?) been achieved by just adding the
> callback to fiemap_extent_info and using that everywhere?

Not really, because fiemap_extent_info has (had now) a userspace pointer, which
isn't used in FIBMAP calls in later patches, so the idea of using callbacks
started as a need to use different code workflow, depending on the user of (now
deprecated) fiemap_extent_info

> 
> Cheers, Andreas
> 
> > The 'new' fiemap_fill_next_extent() will now be just a helper to call
> > fiemap_ctx callback.
> > 
> > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> > ---
> > fs/ioctl.c         | 42 +++++++++++++++++++++++++-----------------
> > include/linux/fs.h |  2 +-
> > 2 files changed, 26 insertions(+), 18 deletions(-)
> > 
> > diff --git a/fs/ioctl.c b/fs/ioctl.c
> > index 27f79b29cb07..85a7aec40e3d 100644
> > --- a/fs/ioctl.c
> > +++ b/fs/ioctl.c
> > @@ -68,25 +68,10 @@ static int ioctl_fibmap(struct file *filp, int __user *p)
> > 	return put_user(res, p);
> > }
> > 
> > -/**
> > - * 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_ctx *f_ctx, u64 logical,
> > +int fiemap_fill_usr_extent(struct fiemap_ctx *f_ctx, u64 logical,
> > 			    u64 phys, u64 len, u32 flags)
> > {
> > 	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
> > @@ -124,8 +109,29 @@ int fiemap_fill_next_extent(struct fiemap_ctx *f_ctx, u64 logical,
> > 		return 1;
> > 	return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0;
> > }
> > -EXPORT_SYMBOL(fiemap_fill_next_extent);
> > 
> > +/**
> > + * 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_ctx *f_ctx, u64 logical,
> > +			    u64 phys, u64 len, u32 flags)
> > +{
> > +	return f_ctx->fc_cb(f_ctx, logical, phys, len, flags);
> > +}
> > +EXPORT_SYMBOL(fiemap_fill_next_extent);
> > /**
> >  * fiemap_check_flags - check validity of requested flags for fiemap
> >  * @fieinfo:	Fiemap context passed into ->fiemap
> > @@ -208,6 +214,8 @@ static int ioctl_fiemap(struct file *filp, unsigned long arg)
> > 	f_ctx.fc_start = fiemap.fm_start;
> > 	f_ctx.fc_len = len;
> > 
> > +	f_ctx.fc_cb = fiemap_fill_usr_extent;
> > +
> > 	if (fiemap.fm_extent_count != 0 &&
> > 	    !access_ok(VERIFY_WRITE, fieinfo.fi_extents_start,
> > 		       fieinfo.fi_extents_max * sizeof(struct fiemap_extent)))
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index 945cfb3e06e4..9a538bc0dac2 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -1704,7 +1704,7 @@ typedef int (*fiemap_fill_cb)(struct fiemap_ctx *f_ctx, u64 logical,
> > struct fiemap_ctx {
> > 	unsigned int fc_flags;	/* Flags as passed from user */
> > 	void *fc_data;
> > -	fiemap_fill_cb fc_cb; /* Unused by now */
> > +	fiemap_fill_cb fc_cb;
> > 	u64 fc_start;
> > 	u64 fc_len;
> > };
> > --
> > 2.17.1
> > 
> 
> 
> Cheers, Andreas
> 
> 
> 
> 
> 



-- 
Carlos

Re: [PATCH 17/20] fiemap: Get rid of fiemap_extent_info

[Carlos Maiolino]

On Mon, Nov 05, 2018 at 03:14:38PM -0700, Andreas Dilger wrote:
> On Oct 30, 2018, at 7:18 AM, Carlos Maiolino <cmaiolino@redhat.com> wrote:
> > 
> > With the goal of using FIEMAP infra-structure for FIBMAP ioctl, and with
> > the updates being done previously into the FIEMAP interface, the
> > fiemap_extent_info structure is no longer necessary, so, move
> > fi_extents_mapped and fi_extents_max up to fiemap_ctx. and use
> > fiemap_ctx.fc_data to hold a pointer to struct fiemap_extent.
> 
> Having the "void *fc_data" pointer point to fiemap_extent is not
> necessarily intuitive.  Why not just make it directly point to it:
> 
>       struct fiemap_extent *fc_extent;

Both data structures pointed by *fc_data contains a fiemap_extent struct, but
one is a __user annotated address while other is a kernel space address, I don't
know if I can use a non __user pointer as you suggested to store the address of
a __user pointer, I actually don't think I can without getting warnings, but I
honestly don't remember.

> 
> Cheers, Andreas
> 
> > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> > ---
> > fs/btrfs/extent_io.c  |  3 +--
> > fs/ioctl.c            | 29 +++++++++++++----------------
> > include/linux/fs.h    |  9 ++-------
> > include/linux/iomap.h |  1 -
> > 4 files changed, 16 insertions(+), 26 deletions(-)
> > 
> > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> > index 6a8bc502bc04..88b8a4416dfc 100644
> > --- a/fs/btrfs/extent_io.c
> > +++ b/fs/btrfs/extent_io.c
> > @@ -4436,7 +4436,6 @@ int extent_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
> > 	struct btrfs_path *path;
> > 	struct btrfs_root *root = BTRFS_I(inode)->root;
> > 	struct fiemap_cache cache = { 0 };
> > -	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
> > 	int end = 0;
> > 	u64 em_start = 0;
> > 	u64 em_len = 0;
> > @@ -4561,7 +4560,7 @@ int extent_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
> > 		} else if (em->block_start == EXTENT_MAP_DELALLOC) {
> > 			flags |= (FIEMAP_EXTENT_DELALLOC |
> > 				  FIEMAP_EXTENT_UNKNOWN);
> > -		} else if (fieinfo->fi_extents_max) {
> > +		} else if (f_ctx->fc_extents_max) {
> > 			u64 bytenr = em->block_start -
> > 				(em->start - em->orig_start);
> > 
> > diff --git a/fs/ioctl.c b/fs/ioctl.c
> > index cbc1b21c4f7c..71d11201a06b 100644
> > --- a/fs/ioctl.c
> > +++ b/fs/ioctl.c
> > @@ -78,17 +78,16 @@ static int ioctl_fibmap(struct file *filp, int __user *p)
> > int fiemap_fill_usr_extent(struct fiemap_ctx *f_ctx, u64 logical,
> > 			    u64 phys, u64 len, u32 flags)
> > {
> > -	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
> > 	struct fiemap_extent extent;
> > -	struct fiemap_extent __user *dest = fieinfo->fi_extents_start;
> > +	struct fiemap_extent __user *dest = f_ctx->fc_data;
> > 
> > 	/* only count the extents */
> > -	if (fieinfo->fi_extents_max == 0) {
> > -		fieinfo->fi_extents_mapped++;
> > +	if (f_ctx->fc_extents_max == 0) {
> > +		f_ctx->fc_extents_mapped++;
> > 		return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0;
> > 	}
> > 
> > -	if (fieinfo->fi_extents_mapped >= fieinfo->fi_extents_max)
> > +	if (f_ctx->fc_extents_mapped >= f_ctx->fc_extents_max)
> > 		return 1;
> > 
> > 	if (flags & SET_UNKNOWN_FLAGS)
> > @@ -104,12 +103,12 @@ int fiemap_fill_usr_extent(struct fiemap_ctx *f_ctx, u64 logical,
> > 	extent.fe_length = len;
> > 	extent.fe_flags = flags;
> > 
> > -	dest += fieinfo->fi_extents_mapped;
> > +	dest += f_ctx->fc_extents_mapped;
> > 	if (copy_to_user(dest, &extent, sizeof(extent)))
> > 		return -EFAULT;
> > 
> > -	fieinfo->fi_extents_mapped++;
> > -	if (fieinfo->fi_extents_mapped == fieinfo->fi_extents_max)
> > +	f_ctx->fc_extents_mapped++;
> > +	if (f_ctx->fc_extents_mapped == f_ctx->fc_extents_max)
> > 		return 1;
> > 	return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0;
> > }
> > @@ -189,7 +188,6 @@ static int ioctl_fiemap(struct file *filp, unsigned long arg)
> > {
> > 	struct fiemap fiemap;
> > 	struct fiemap __user *ufiemap = (struct fiemap __user *) arg;
> > -	struct fiemap_extent_info fieinfo = { 0, };
> > 	struct inode *inode = file_inode(filp);
> > 	struct super_block *sb = inode->i_sb;
> > 	struct fiemap_ctx f_ctx;
> > @@ -210,19 +208,18 @@ static int ioctl_fiemap(struct file *filp, unsigned long arg)
> > 	if (error)
> > 		return error;
> > 
> > -	fieinfo.fi_extents_max = fiemap.fm_extent_count;
> > -	fieinfo.fi_extents_start = ufiemap->fm_extents;
> > -
> > +	f_ctx.fc_extents_max = fiemap.fm_extent_count;
> > +	f_ctx.fc_data = ufiemap->fm_extents;
> > +	f_ctx.fc_extents_mapped = 0;
> > 	f_ctx.fc_flags = fiemap.fm_flags;
> > -	f_ctx.fc_data = &fieinfo;
> > 	f_ctx.fc_start = fiemap.fm_start;
> > 	f_ctx.fc_len = len;
> > 
> > 	f_ctx.fc_cb = fiemap_fill_usr_extent;
> > 
> > 	if (fiemap.fm_extent_count != 0 &&
> > -	    !access_ok(VERIFY_WRITE, fieinfo.fi_extents_start,
> > -		       fieinfo.fi_extents_max * sizeof(struct fiemap_extent)))
> > +	    !access_ok(VERIFY_WRITE, f_ctx.fc_data,
> > +		       f_ctx.fc_extents_max * sizeof(struct fiemap_extent)))
> > 		return -EFAULT;
> > 
> > 	if (f_ctx.fc_flags & FIEMAP_FLAG_SYNC)
> > @@ -230,7 +227,7 @@ static int ioctl_fiemap(struct file *filp, unsigned long arg)
> > 
> > 	error = inode->i_op->fiemap(inode, &f_ctx);
> > 	fiemap.fm_flags = f_ctx.fc_flags;
> > -	fiemap.fm_mapped_extents = fieinfo.fi_extents_mapped;
> > +	fiemap.fm_mapped_extents = f_ctx.fc_extents_mapped;
> > 	if (copy_to_user(ufiemap, &fiemap, sizeof(fiemap)))
> > 		error = -EFAULT;
> > 
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index 9a538bc0dac2..4c6dee908a38 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -1691,18 +1691,13 @@ extern bool may_open_dev(const struct path *path);
> > 
> > struct fiemap_ctx;
> > 
> > -struct fiemap_extent_info {
> > -	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 */
> > -};
> > -
> > typedef int (*fiemap_fill_cb)(struct fiemap_ctx *f_ctx, u64 logical,
> > 			      u64 phys, u64 len, u32 flags);
> > 
> > struct fiemap_ctx {
> > 	unsigned int fc_flags;	/* Flags as passed from user */
> > +	unsigned int fc_extents_mapped;	/* Number of mapped extents */
> > +	unsigned int fc_extents_max;	/* Size of fiemap_extent array */
> > 	void *fc_data;
> > 	fiemap_fill_cb fc_cb;
> > 	u64 fc_start;
> > diff --git a/include/linux/iomap.h b/include/linux/iomap.h
> > index 7ce84be499d6..21643bcf6104 100644
> > --- a/include/linux/iomap.h
> > +++ b/include/linux/iomap.h
> > @@ -9,7 +9,6 @@
> > #include <linux/mm_types.h>
> > 
> > struct address_space;
> > -struct fiemap_extent_info;
> > struct inode;
> > struct iov_iter;
> > struct kiocb;
> > --
> > 2.17.1
> > 
> 
> 
> Cheers, Andreas
> 
> 
> 
> 
> 



-- 
Carlos

Re: [PATCH 18/20] Use FIEMAP for FIBMAP calls

[Carlos Maiolino]

> > +	if (inode->i_op->fiemap) {
> > +		fextent.fe_logical = 0;
> > +		fextent.fe_physical = 0;
> > +		f_ctx.fc_extents_max = 1;
> > +		f_ctx.fc_extents_mapped = 0;
> > +		f_ctx.fc_data = &fextent;
> > +		f_ctx.fc_start = start;
> > +		f_ctx.fc_len = 1;
> 
> Should this be "fc_len = 1 << inode->i_blkbits" to map a single block?

D'oh, yeah, I just re-read fiemap implementation, and yeah, len is supposed to
be byte-sized, not block sized, so you are right. It doesn't really make much
difference here, due the purpose of the fiemap call here, but still it's wrong.
I'll update it on a future review.

> On the one hand, this might return multiple blocks, but on the other
> hand FIBMAP shouldn't be allowed if that is the case so this code should
> detect that situation and consider it an error.

Well, FIEMAP interface is used internally to get the block mapping requested by
the user via FIBMAP, so, yes, FIEMAP  interface will return the whole extent,
but to the user, the ioctl will return just the mapping of the requested block.

Which is basically what we do here:

> > +		*block = (fextent.fe_physical +
> > +			(start - fextent.fe_logical)) >> inode->i_blkbits;
> > +

We only return a single address back, not the whole extent map.


Again, thanks for the review, much appreciated

-- 
Carlos

Re: [PATCH 03/20] ecryptfs: drop direct calls to ->bmap

[Christoph Hellwig]

On Tue, Oct 30, 2018 at 02:18:06PM +0100, Carlos Maiolino wrote:
> Replace direct ->bmap calls by bmap() method.
> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 04/20] iomap: Rename fiemap_ctx to fiemap_iomap_ctx

[Christoph Hellwig]

On Tue, Oct 30, 2018 at 02:18:07PM +0100, Carlos Maiolino wrote:
> fiemap_ctx structure is local to iomap code, so, rename it to match its
> scope.
> 
> A new structure named fiemap_ctx will be added which will be globally
> available as part of the fiemap kernel API.

Looks fine,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 05/20] fs: Introduce fiemap_ctx data structure

[Christoph Hellwig]

On Tue, Oct 30, 2018 at 02:18:08PM +0100, 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, just
> 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.
> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---
>  fs/bad_inode.c        |  4 +---
>  fs/btrfs/inode.c      |  6 ++++--
>  fs/ext2/ext2.h        |  3 +--
>  fs/ext2/inode.c       |  6 ++----
>  fs/ext4/ext4.h        |  3 +--
>  fs/ext4/extents.c     |  9 +++++----
>  fs/f2fs/data.c        |  6 ++++--
>  fs/f2fs/f2fs.h        |  3 +--
>  fs/gfs2/inode.c       |  6 ++++--
>  fs/hpfs/file.c        |  4 ++--
>  fs/ioctl.c            | 23 +++++++++++++++--------
>  fs/nilfs2/inode.c     |  6 ++++--
>  fs/nilfs2/nilfs.h     |  3 +--
>  fs/ocfs2/extent_map.c |  6 ++++--
>  fs/ocfs2/extent_map.h |  3 +--
>  fs/overlayfs/inode.c  |  6 +++---
>  fs/xfs/xfs_iops.c     |  9 +++++----
>  include/linux/fs.h    | 27 +++++++++++++++++++--------
>  18 files changed, 77 insertions(+), 56 deletions(-)
> 
> diff --git a/fs/bad_inode.c b/fs/bad_inode.c
> index 8035d2a44561..b5fbe28b8d17 100644
> --- a/fs/bad_inode.c
> +++ b/fs/bad_inode.c
> @@ -119,9 +119,7 @@ static struct posix_acl *bad_inode_get_acl(struct inode *inode, int type)
>  	return ERR_PTR(-EIO);
>  }
>  
> -static int bad_inode_fiemap(struct inode *inode,
> -			    struct fiemap_extent_info *fieinfo, u64 start,
> -			    u64 len)
> +static int bad_inode_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
>  {
>  	return -EIO;
>  }
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index cd247a21afc4..ebe5aae69c44 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -8612,9 +8612,11 @@ 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_ctx *f_ctx)
>  {
> +	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
> +	u64 start = f_ctx->fc_start;
> +	u64 len	  = f_ctx->fc_len;
>  	int	ret;
>  
>  	ret = fiemap_check_flags(fieinfo, BTRFS_FIEMAP_FLAGS);
> diff --git a/fs/ext2/ext2.h b/fs/ext2/ext2.h
> index e770cd100a6a..dc5dec52a7c8 100644
> --- a/fs/ext2/ext2.h
> +++ b/fs/ext2/ext2.h
> @@ -775,8 +775,7 @@ extern void ext2_evict_inode(struct inode *);
>  extern int ext2_get_block(struct inode *, sector_t, struct buffer_head *, int);
>  extern int ext2_setattr (struct dentry *, struct iattr *);
>  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_ctx *f_ctx);
>  
>  /* 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 e4bb9386c045..85e7edad58a3 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_ctx *f_ctx)
>  {
> -	return generic_block_fiemap(inode, fieinfo, start, len,
> -				    ext2_get_block);
> +	return generic_block_fiemap(inode, f_ctx, 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 3f89d0ab08fc..acc1ea0e9f40 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -3140,8 +3140,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_ctx *f_ctx);
>  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 240b6dea5441..beccae768dac 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -5045,9 +5045,11 @@ 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_ctx *f_ctx)
>  {
> +	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
> +	u64 start = f_ctx->fc_start;
> +	u64 len = f_ctx->fc_len;
>  	ext4_lblk_t start_blk;
>  	int error = 0;
>  
> @@ -5069,8 +5071,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, f_ctx, 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 82b6e27a21cc..0f57403feb92 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1397,9 +1397,11 @@ 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_ctx *f_ctx)
>  {
> +	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
> +	u64 start = f_ctx->fc_start;
> +	u64 len = f_ctx->fc_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 4175445be2b0..185bb39dd82c 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -3075,8 +3075,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_ctx *f_ctx);
>  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 648f0ca1ad57..2a4ce713c209 100644
> --- a/fs/gfs2/inode.c
> +++ b/fs/gfs2/inode.c
> @@ -2004,9 +2004,11 @@ 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_ctx *f_ctx)
>  {
> +	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
> +	u64 start = f_ctx->fc_start;
> +	u64 len = f_ctx->fc_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..9e31278cbfe1 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_ctx *f_ctx)
>  {
> -	return generic_block_fiemap(inode, fieinfo, start, len, hpfs_get_block);
> +	return generic_block_fiemap(inode, f_ctx, hpfs_get_block);
>  }
>  
>  const struct address_space_operations hpfs_aops = {
> diff --git a/fs/ioctl.c b/fs/ioctl.c
> index 2005529af560..ddcc4f9a9cf1 100644
> --- a/fs/ioctl.c
> +++ b/fs/ioctl.c
> @@ -181,6 +181,7 @@ static int ioctl_fiemap(struct file *filp, unsigned long arg)
>  	struct fiemap_extent_info fieinfo = { 0, };
>  	struct inode *inode = file_inode(filp);
>  	struct super_block *sb = inode->i_sb;
> +	struct fiemap_ctx f_ctx;
>  	u64 len;
>  	int error;
>  
> @@ -210,7 +211,12 @@ 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);
> +	f_ctx.fc_flags = fieinfo.fi_flags;
> +	f_ctx.fc_data = &fieinfo;
> +	f_ctx.fc_start = fiemap.fm_start;
> +	f_ctx.fc_len = len;
> +
> +	error = inode->i_op->fiemap(inode, &f_ctx);
>  	fiemap.fm_flags = fieinfo.fi_flags;
>  	fiemap.fm_mapped_extents = fieinfo.fi_extents_mapped;
>  	if (copy_to_user(ufiemap, &fiemap, sizeof(fiemap)))
> @@ -278,10 +284,12 @@ static inline loff_t blk_to_logical(struct inode *inode, sector_t blk)
>   * generic_block_fiemap if you want the locking done for you.
>   */
>  
> -int __generic_block_fiemap(struct inode *inode,
> -			   struct fiemap_extent_info *fieinfo, loff_t start,
> -			   loff_t len, get_block_t *get_block)
> +int __generic_block_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx,
> +			   get_block_t *get_block)
>  {
> +	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
> +	loff_t start = f_ctx->fc_start;
> +	loff_t len = f_ctx->fc_len;
>  	struct buffer_head map_bh;
>  	sector_t start_blk, last_blk;
>  	loff_t isize = i_size_read(inode);
> @@ -437,13 +445,12 @@ EXPORT_SYMBOL(__generic_block_fiemap);
>   * the inode's mutex lock.
>   */
>  
> -int generic_block_fiemap(struct inode *inode,
> -			 struct fiemap_extent_info *fieinfo, u64 start,
> -			 u64 len, get_block_t *get_block)
> +int generic_block_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx,
> +			 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, f_ctx, get_block);
>  	inode_unlock(inode);
>  	return ret;
>  }
> diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
> index 671085512e0f..529262f0ab7d 100644
> --- a/fs/nilfs2/inode.c
> +++ b/fs/nilfs2/inode.c
> @@ -992,9 +992,11 @@ 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_ctx *f_ctx)
>  {
> +	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
> +	u64 start = f_ctx->fc_start;
> +	u64 len = f_ctx->fc_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..b403dae8c258 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_ctx *f_ctx);
>  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..1944f5659267 100644
> --- a/fs/ocfs2/extent_map.c
> +++ b/fs/ocfs2/extent_map.c
> @@ -749,9 +749,11 @@ 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_ctx *f_ctx)
>  {
> +	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
> +	u64 map_start = f_ctx->fc_start;
> +	u64 map_len = f_ctx->fc_len;
>  	int ret, is_last;
>  	u32 mapping_end, cpos;
>  	unsigned int hole_size;
> diff --git a/fs/ocfs2/extent_map.h b/fs/ocfs2/extent_map.h
> index 1057586ec19f..6c07fdf4d438 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_ctx *f_ctx);
>  
>  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..e56606c1ba92 100644
> --- a/fs/overlayfs/inode.c
> +++ b/fs/overlayfs/inode.c
> @@ -456,9 +456,9 @@ 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_ctx *f_ctx)
>  {
> +	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
>  	int err;
>  	struct inode *realinode = ovl_inode_real(inode);
>  	const struct cred *old_cred;
> @@ -471,7 +471,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, f_ctx);
>  	revert_creds(old_cred);
>  
>  	return err;
> diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> index f48ffd7a8d3e..d33d56fa8097 100644
> --- a/fs/xfs/xfs_iops.c
> +++ b/fs/xfs/xfs_iops.c
> @@ -1093,11 +1093,12 @@ xfs_vn_update_time(
>  STATIC int
>  xfs_vn_fiemap(
>  	struct inode		*inode,
> -	struct fiemap_extent_info *fieinfo,
> -	u64			start,
> -	u64			length)
> +	struct fiemap_ctx	*f_ctx)
>  {
> -	int			error;
> +	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
> +	u64 start  = f_ctx->fc_start;
> +	u64 length = f_ctx->fc_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 1ac714e95d6b..bd8e8a7dfd59 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1688,6 +1688,9 @@ extern bool may_open_dev(const struct path *path);
>  /*
>   * VFS FS_IOC_FIEMAP helper definitions.
>   */
> +
> +struct fiemap_ctx;
> +
>  struct fiemap_extent_info {
>  	unsigned int fi_flags;		/* Flags as passed from user */
>  	unsigned int fi_extents_mapped;	/* Number of mapped extents */
> @@ -1695,6 +1698,18 @@ struct fiemap_extent_info {
>  	struct fiemap_extent __user *fi_extents_start; /* Start of
>  							fiemap_extent array */
>  };
> +
> +typedef int (*fiemap_fill_cb)(struct fiemap_ctx *f_ctx, u64 logical,
> +			      u64 phys, u64 len, u32 flags);
> +
> +struct fiemap_ctx {
> +	unsigned int fc_flags;
> +	void *fc_data;
> +	fiemap_fill_cb fc_cb; /* Unused by now */

Please only add field when you actually start using them.

Also I much prefer aligning the names of the struct members to ease
readability, e.g.:

struct fiemap_ctx {
	unsigned int		fc_flags;
	u64			fc_start;
	u64			fc_len;
};

Otherwise this looks fine to me:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 06/20] iomap: Update iomap_fiemap to use new fiemap_ctx structure

[Christoph Hellwig]

On Tue, Oct 30, 2018 at 02:18:09PM +0100, Carlos Maiolino wrote:
> iomap_fiemap() isn't directly called by ioctl_fiemap(), so it doesn't
> needed to be changed when fiemap_ctx has been added, update it now.

That sentence has some grammar issues.

How about simply:

"Update iomap_fiemap to take a struct fiemap_ctx".

> -	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
>  	u64 start = f_ctx->fc_start;
>  	u64 len = f_ctx->fc_len;
>  	struct gfs2_inode *ip = GFS2_I(inode);
> @@ -2019,7 +2018,7 @@ static int gfs2_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
>  	if (ret)
>  		goto out;
>  
> -	ret = iomap_fiemap(inode, fieinfo, start, len, &gfs2_iomap_ops);
> +	ret = iomap_fiemap(inode, f_ctx, start, len, &gfs2_iomap_ops);

I think we should get rid of the start and len argument as well.

Re: [PATCH 07/20] fiemap: Move fiemap flags to fiemap_ctx

[Christoph Hellwig]

On Tue, Nov 06, 2018 at 09:37:26AM +0100, Carlos Maiolino wrote:
> Not really, fiemap_extent_info is removed at the end of the series, and by doing
> it directly, I'd need to write a big patch changing all users of
> fiemap_extent_info in a single patch, or break the kernel build between patches.
> I usually try to avoid a single patch touching several different filesystems, so
> that's why I basically slowly replaced fiemap_extent_info, until it get removed
> at the end of the series.

Sounds sensible.  I also always found fiemap_extent_info rather long
and cumbersome.

> > The other minor nit about this patch series is that "f_ctx" is not a very
> > descriptive variable name.  How about "fiemap_ctx"?  A bit longer, but it
> > is immediately clear to the reader what it is.
> 
> I don't really mind, f_ctx, fmap_ctx, etc. having the pointer the same name as
> the struct doesn't look good to me, but particularly I don't mind.

Just ctx sounds fine to me.

Re: [PATCH 07/20] fiemap: Move fiemap flags to fiemap_ctx

[Christoph Hellwig]

>  struct fiemap_ctx {
> -	unsigned int fc_flags;
> +	unsigned int fc_flags;	/* Flags as passed from user */

If you really need a comment here maybe it should be:

	unsigned int		fc_flags; /* FIEMAP_FLAG_* */

Otherwise this generally looks fine to me.

Re: [PATCH 14/20] fiemap: Use fiemap_ctx as fiemap_fill_next_extent argument

[Christoph Hellwig]

On Tue, Oct 30, 2018 at 02:18:17PM +0100, Carlos Maiolino wrote:
> Replace the current fiemap_extent_info argument in
> fiemap_fill_next_extent(), by the new fiemap_ctx structure. This way we
> remove (almost) all direct usage of fiemap_extent_info outside fiemap
> code (except by btrfs).

Given that the previous prep patches already touched basically
all the fiemap_fill_next_extent I'd be tempted to merge them into
this patch for a little less churn.

Re: [PATCH 15/20] fiemap: Start using new callback from fiemap_ctx

[Christoph Hellwig]

On Tue, Nov 06, 2018 at 09:52:03AM +0100, Carlos Maiolino wrote:
> Not really, because fiemap_extent_info has (had now) a userspace pointer, which
> isn't used in FIBMAP calls in later patches, so the idea of using callbacks
> started as a need to use different code workflow, depending on the user of (now
> deprecated) fiemap_extent_info

Well, we could also change that member of fiemap_extent_info if we
really wanted to.  Not sure that would be the better route, though.

Re: [PATCH 15/20] fiemap: Start using new callback from fiemap_ctx

[Christoph Hellwig]

> +int fiemap_fill_usr_extent(struct fiemap_ctx *f_ctx, u64 logical,

Please spell out user.

> +int fiemap_fill_next_extent(struct fiemap_ctx *f_ctx, u64 logical,
> +			    u64 phys, u64 len, u32 flags)
> +{
> +	return f_ctx->fc_cb(f_ctx, logical, phys, len, flags);
> +}
> +EXPORT_SYMBOL(fiemap_fill_next_extent);

Do we really need this wrapper?  Which reminds me that we usually
pass the callbacks as direct function arguments.  Any good reason it
is in the context here?

Re: [PATCH 16/20] fibmap: Use bmap instead of ->bmap method in ioctl_fibmap

[Christoph Hellwig]

> +	error = bmap(inode, (sector_t *)&block);

You can't cast and int * to a sector_t * without causing problems.
You'll need a local variable of type sector_t that the value
gets assigned to and read back from instead.

Also shouldn't this patch be earlier in the series?

Re: [PATCH 17/20] fiemap: Get rid of fiemap_extent_info

[Christoph Hellwig]

On Tue, Oct 30, 2018 at 02:18:20PM +0100, Carlos Maiolino wrote:
> With the goal of using FIEMAP infra-structure for FIBMAP ioctl, and with
> the updates being done previously into the FIEMAP interface, the
> fiemap_extent_info structure is no longer necessary, so, move
> fi_extents_mapped and fi_extents_max up to fiemap_ctx. and use
> fiemap_ctx.fc_data to hold a pointer to struct fiemap_extent.
> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---
>  fs/btrfs/extent_io.c  |  3 +--
>  fs/ioctl.c            | 29 +++++++++++++----------------
>  include/linux/fs.h    |  9 ++-------
>  include/linux/iomap.h |  1 -
>  4 files changed, 16 insertions(+), 26 deletions(-)
> 
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 6a8bc502bc04..88b8a4416dfc 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -4436,7 +4436,6 @@ int extent_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
>  	struct btrfs_path *path;
>  	struct btrfs_root *root = BTRFS_I(inode)->root;
>  	struct fiemap_cache cache = { 0 };
> -	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
>  	int end = 0;
>  	u64 em_start = 0;
>  	u64 em_len = 0;
> @@ -4561,7 +4560,7 @@ int extent_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
>  		} else if (em->block_start == EXTENT_MAP_DELALLOC) {
>  			flags |= (FIEMAP_EXTENT_DELALLOC |
>  				  FIEMAP_EXTENT_UNKNOWN);
> -		} else if (fieinfo->fi_extents_max) {
> +		} else if (f_ctx->fc_extents_max) {
>  			u64 bytenr = em->block_start -
>  				(em->start - em->orig_start);

Shouldn't this be earlier, when or just after when we introduce 
f_ctx->fc_extents_max?

>  		return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0;

Can you use a plain old if statment here to make the code a little
more readable?

		if (flags & FIEMAP_EXTENT_LAST)
			return 1;
		return 0;

>  struct fiemap_ctx {
>  	unsigned int fc_flags;	/* Flags as passed from user */
> +	unsigned int fc_extents_mapped;	/* Number of mapped extents */
> +	unsigned int fc_extents_max;	/* Size of fiemap_extent array */
>  	void *fc_data;
>  	fiemap_fill_cb fc_cb;
>  	u64 fc_start;

This makes me wonder if we shouldn't just have kept the existing
structure and massaged it into the new form.  The two just look
very, very similar..

Re: [PATCH 18/20] Use FIEMAP for FIBMAP calls

[Christoph Hellwig]

On Tue, Oct 30, 2018 at 02:18:21PM +0100, Carlos Maiolino wrote:
> +	if (inode->i_op->fiemap) {
> +		fextent.fe_logical = 0;
> +		fextent.fe_physical = 0;
> +		f_ctx.fc_extents_max = 1;
> +		f_ctx.fc_extents_mapped = 0;
> +		f_ctx.fc_data = &fextent;
> +		f_ctx.fc_start = start;
> +		f_ctx.fc_len = 1;
> +		f_ctx.fc_flags = 0;
> +		f_ctx.fc_cb = fiemap_fill_kernel_extent;
> +
> +		error = inode->i_op->fiemap(inode, &f_ctx);
> +
> +		if (error)
> +			goto out;
> +
> +		*block = (fextent.fe_physical +
> +			(start - fextent.fe_logical)) >> inode->i_blkbits;
> +

I think this code needs to be split into a helper.

Also we need to check for various "dangerous" flags and fail the
call, e.g. FIEMAP_EXTENT_ENCODED, FIEMAP_EXTENT_DELALLOC,
FIEMAP_EXTENT_DATA_ENCRYPTED, FIEMAP_EXTENT_DATA_INLINE,
FIEMAP_EXTENT_DATA_TAIL, FIEMAP_EXTENT_UNWRITTEN,
FIEMAP_EXTENT_SHARED.

> +	} else if (inode->i_mapping->a_ops->bmap) {
> +		*block = inode->i_mapping->a_ops->bmap(inode->i_mapping, *block);

Too long line.

> +		error = 0;
> +	}
> +out:
> +	return error;

No need for a goto label just to return an error.

> +		return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0;

Plain old if statement, please.

> +	return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0;

Same here.  But in this case we could actually use a goto to share
the code.

Re: [PATCH 20/20] ext4: Get rid of ->bmap interface

[Christoph Hellwig]

> -	if (EXT4_JOURNAL(inode) &&
> -	    ext4_test_inode_state(inode, EXT4_STATE_JDATA)) {
> -		/*
> -		 * This is a REALLY heavyweight approach, but the use of
> -		 * bmap on dirty files is expected to be extremely rare:
> -		 * only if we run lilo or swapon on a freshly made file
> -		 * do we expect this to happen.
> -		 *
> -		 * (bmap requires CAP_SYS_RAWIO so this does not
> -		 * represent an unprivileged user DOS attack --- we'd be
> -		 * in trouble if mortal users could trigger this path at
> -		 * will.)
> -		 *
> -		 * NB. EXT4_STATE_JDATA is not set on files other than
> -		 * regular files.  If somebody wants to bmap a directory
> -		 * or symlink and gets confused because the buffer
> -		 * hasn't yet been flushed to disk, they deserve
> -		 * everything they get.
> -		 */

Does the ext4 fiemap code have this magic handling?  If not are
we sure we can kill it?

Re: [PATCH 15/20] fiemap: Start using new callback from fiemap_ctx

[Carlos Maiolino]

On Fri, Nov 16, 2018 at 04:55:42PM +0100, Christoph Hellwig wrote:
> On Tue, Nov 06, 2018 at 09:52:03AM +0100, Carlos Maiolino wrote:
> > Not really, because fiemap_extent_info has (had now) a userspace pointer, which
> > isn't used in FIBMAP calls in later patches, so the idea of using callbacks
> > started as a need to use different code workflow, depending on the user of (now
> > deprecated) fiemap_extent_info
> 
> Well, we could also change that member of fiemap_extent_info if we
> really wanted to.  Not sure that would be the better route, though.

I think there are some downsides in keep using fiemap_extent_info. I'd rather
prefer use this new fiemap_ctx, which we can use to embed most of the arguments
needed for fiemap, like the flags which were passed separated.
At all, most of the fields would fit either in fiemap_extent_info, or the new
fiemap_ctx. I'd prefer the latter, due simplicity.
We could probably simplify it using the void pointer directly into
fiemap_extent_info and using flags to discern between user/kernel space pointer.
But that would drop one of the goals of this patchset, which is adding a new and
better interface for FIEMAP. Preferentially, I'd stick with fiemap_ctx.

Cheers and thanks for the review
-- 
Carlos

Re: [PATCH 15/20] fiemap: Start using new callback from fiemap_ctx

[Carlos Maiolino]

On Fri, Nov 16, 2018 at 04:57:21PM +0100, Christoph Hellwig wrote:
> > +int fiemap_fill_usr_extent(struct fiemap_ctx *f_ctx, u64 logical,
> 
> Please spell out user.

Sure, will do.

> 
> > +int fiemap_fill_next_extent(struct fiemap_ctx *f_ctx, u64 logical,
> > +			    u64 phys, u64 len, u32 flags)
> > +{
> > +	return f_ctx->fc_cb(f_ctx, logical, phys, len, flags);
> > +}
> > +EXPORT_SYMBOL(fiemap_fill_next_extent);
> 
> Do we really need this wrapper?  Which reminds me that we usually
> pass the callbacks as direct function arguments.  Any good reason it
> is in the context here?

The reason I left the wrapper is to avoid replacing all
fiemap_fill_next_extent() calls by the callback, may not be the best idea
though.

About using the callback in the context, I thought it would be the easiest way
to pass the callbacks around, without adding a function pointer to many
functions.
The callback is set in ioctl_fiemap() (for user context) or bmap(), if we pass
the callback via function argument, many filesystems would need to juggle around
with the function pointer, once, some of them does not call the
fiemap_fill_next_extent directly from the fiemap method, but into other
sub-functions, for example, in ext4, we would need the function pointer in
ext4_fiemap, and other functions like ext4_fill_fiemap_extents() and
ext4_xattr_fiemap(), etc.

We could even call fiemap_fill_{usr,kernel}_extent() directly from
fiemap_fill_next_extent, and use a flag to differentiate both use cases, but
this will kill the transparent implementation IMHO.

-- 
Carlos

Re: [PATCH 16/20] fibmap: Use bmap instead of ->bmap method in ioctl_fibmap

[Carlos Maiolino]

On Fri, Nov 16, 2018 at 04:58:53PM +0100, Christoph Hellwig wrote:
> > +	error = bmap(inode, (sector_t *)&block);
> 
> You can't cast and int * to a sector_t * without causing problems.
> You'll need a local variable of type sector_t that the value
> gets assigned to and read back from instead.

Ok, thanks for the information, I'll update it, but, could you detail a bit more
what kind of problem you are referring to? I'm curious to understand why and
which kind of problems it can cause.

> Also shouldn't this patch be earlier in the series?

I opted to move this patch to the end of the series, when most of the fiemap
rework is done, I can move it to the beginning of the series if it's better.

-- 
Carlos

Re: [PATCH 17/20] fiemap: Get rid of fiemap_extent_info

[Carlos Maiolino]

> >  	struct fiemap_cache cache = { 0 };
> > -	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
> >  	int end = 0;
> >  	u64 em_start = 0;
> >  	u64 em_len = 0;
> > @@ -4561,7 +4560,7 @@ int extent_fiemap(struct inode *inode, struct fiemap_ctx *f_ctx)
> >  		} else if (em->block_start == EXTENT_MAP_DELALLOC) {
> >  			flags |= (FIEMAP_EXTENT_DELALLOC |
> >  				  FIEMAP_EXTENT_UNKNOWN);
> > -		} else if (fieinfo->fi_extents_max) {
> > +		} else if (f_ctx->fc_extents_max) {
> >  			u64 bytenr = em->block_start -
> >  				(em->start - em->orig_start);
> 
> Shouldn't this be earlier, when or just after when we introduce 
> f_ctx->fc_extents_max?

hmm, not sure if I understand you here, fc_extents_max is added in this very
patch, well, actually moved from fiemap_extent_info, to the new fiemap_ctx.

> 
> >  		return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0;
> 
> Can you use a plain old if statment here to make the code a little
> more readable?
> 
> 		if (flags & FIEMAP_EXTENT_LAST)
> 			return 1;
> 		return 0;

Sure, no problem.

> 
> >  struct fiemap_ctx {
> >  	unsigned int fc_flags;	/* Flags as passed from user */
> > +	unsigned int fc_extents_mapped;	/* Number of mapped extents */
> > +	unsigned int fc_extents_max;	/* Size of fiemap_extent array */
> >  	void *fc_data;
> >  	fiemap_fill_cb fc_cb;
> >  	u64 fc_start;
> 
> This makes me wonder if we shouldn't just have kept the existing
> structure and massaged it into the new form.  The two just look
> very, very similar..

I can give it a try, most of the work is done, so it won't take so long to do
write a different version re-using fiemap_extent_info. I'll give it a shot.

-- 
Carlos

Re: [PATCH 18/20] Use FIEMAP for FIBMAP calls

[Carlos Maiolino]

On Fri, Nov 16, 2018 at 05:06:43PM +0100, Christoph Hellwig wrote:
> On Tue, Oct 30, 2018 at 02:18:21PM +0100, Carlos Maiolino wrote:
> > +	if (inode->i_op->fiemap) {
> > +		fextent.fe_logical = 0;
> > +		fextent.fe_physical = 0;
> > +		f_ctx.fc_extents_max = 1;
> > +		f_ctx.fc_extents_mapped = 0;
> > +		f_ctx.fc_data = &fextent;
> > +		f_ctx.fc_start = start;
> > +		f_ctx.fc_len = 1;
> > +		f_ctx.fc_flags = 0;
> > +		f_ctx.fc_cb = fiemap_fill_kernel_extent;
> > +
> > +		error = inode->i_op->fiemap(inode, &f_ctx);
> > +
> > +		if (error)
> > +			goto out;
> > +
> > +		*block = (fextent.fe_physical +
> > +			(start - fextent.fe_logical)) >> inode->i_blkbits;
> > +
> 
> I think this code needs to be split into a helper.
> 

Yup, my idea is to try to reduce as much as possible the shared code between
usr/kernel helpers, I just didn't want to spend more time thinking about it
without having a review of the overall design :P

> Also we need to check for various "dangerous" flags and fail the
> call, e.g. FIEMAP_EXTENT_ENCODED, FIEMAP_EXTENT_DELALLOC,
> FIEMAP_EXTENT_DATA_ENCRYPTED, FIEMAP_EXTENT_DATA_INLINE,
> FIEMAP_EXTENT_DATA_TAIL, FIEMAP_EXTENT_UNWRITTEN,
> FIEMAP_EXTENT_SHARED.
> 
> > +	} else if (inode->i_mapping->a_ops->bmap) {
> > +		*block = inode->i_mapping->a_ops->bmap(inode->i_mapping, *block);
> 
> Too long line.
> 
> > +		error = 0;
> > +	}
> > +out:
> > +	return error;
> 
> No need for a goto label just to return an error.
> 
> > +		return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0;
> 
> Plain old if statement, please.
> 
> > +	return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0;
> 
> Same here.  But in this case we could actually use a goto to share
> the code.

Ok, fair enough.

Thanks a lot for the whole review.

Cheers

-- 
Carlos

Re: [PATCH 18/20] Use FIEMAP for FIBMAP calls

[Christoph Hellwig]

On Mon, Nov 19, 2018 at 01:50:28PM +0100, Carlos Maiolino wrote:
> On Fri, Nov 16, 2018 at 05:06:43PM +0100, Christoph Hellwig wrote:
> > On Tue, Oct 30, 2018 at 02:18:21PM +0100, Carlos Maiolino wrote:
> > > +	if (inode->i_op->fiemap) {
> > > +		fextent.fe_logical = 0;
> > > +		fextent.fe_physical = 0;
> > > +		f_ctx.fc_extents_max = 1;
> > > +		f_ctx.fc_extents_mapped = 0;
> > > +		f_ctx.fc_data = &fextent;
> > > +		f_ctx.fc_start = start;
> > > +		f_ctx.fc_len = 1;
> > > +		f_ctx.fc_flags = 0;
> > > +		f_ctx.fc_cb = fiemap_fill_kernel_extent;
> > > +
> > > +		error = inode->i_op->fiemap(inode, &f_ctx);
> > > +
> > > +		if (error)
> > > +			goto out;
> > > +
> > > +		*block = (fextent.fe_physical +
> > > +			(start - fextent.fe_logical)) >> inode->i_blkbits;
> > > +
> > 
> > I think this code needs to be split into a helper.
> > 
> 
> Yup, my idea is to try to reduce as much as possible the shared code between
> usr/kernel helpers, I just didn't want to spend more time thinking about it
> without having a review of the overall design :P

Well, the code in this branch should not change at all by being moved
into a helper function..