[PATCH 0/9 V5] New ->fiemap infrastructure and ->bmap removal

[PATCH 0/9 V5] New ->fiemap infrastructure and ->bmap removal

[Carlos Maiolino]

Hi.

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

This V5, compared with V4 is properly rebased against 5.3, and addresses the
issues raised in V4 discussion.

Details of what changed on each patch are in their specific changelogs.



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

 drivers/md/md-bitmap.c |  16 ++++--
 fs/bad_inode.c         |   3 +-
 fs/btrfs/inode.c       |   5 +-
 fs/cachefiles/rdwr.c   |  27 ++++-----
 fs/ecryptfs/mmap.c     |  16 ++----
 fs/ext2/ext2.h         |   3 +-
 fs/ext2/inode.c        |   6 +-
 fs/ext4/ext4.h         |   3 +-
 fs/ext4/extents.c      |  15 +++--
 fs/f2fs/data.c         |  31 +++++++---
 fs/f2fs/f2fs.h         |   3 +-
 fs/gfs2/inode.c        |   9 ++-
 fs/hpfs/file.c         |   4 +-
 fs/inode.c             | 105 +++++++++++++++++++++++++++++----
 fs/ioctl.c             | 128 ++++++++++++++++++++++++++---------------
 fs/iomap/fiemap.c      |   6 +-
 fs/jbd2/journal.c      |  22 ++++---
 fs/nilfs2/inode.c      |   5 +-
 fs/nilfs2/nilfs.h      |   3 +-
 fs/ocfs2/extent_map.c  |  13 ++++-
 fs/ocfs2/extent_map.h  |   3 +-
 fs/overlayfs/inode.c   |   5 +-
 fs/xfs/xfs_aops.c      |  24 --------
 fs/xfs/xfs_iops.c      |  20 ++++---
 fs/xfs/xfs_trace.h     |   1 -
 include/linux/fs.h     |  35 +++++++----
 include/linux/iomap.h  |   2 +-
 mm/page_io.c           |  11 ++--
 28 files changed, 334 insertions(+), 190 deletions(-)

-- 
2.20.1

[PATCH 1/9] 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.

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

Changelog:

	V5:
		- Rebasing against 5.3 required changes to the f2fs
		  check_swap_activate() function

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

diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index b092c7b5282f..bfb7fddd4f1b 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -364,7 +364,7 @@ static int read_page(struct file *file, unsigned long index,
 	int ret = 0;
 	struct inode *inode = file_inode(file);
 	struct buffer_head *bh;
-	sector_t block;
+	sector_t block, blk_cur;
 
 	pr_debug("read bitmap file (%dB @ %llu)\n", (int)PAGE_SIZE,
 		 (unsigned long long)index << PAGE_SHIFT);
@@ -375,17 +375,21 @@ static int read_page(struct file *file, unsigned long index,
 		goto out;
 	}
 	attach_page_buffers(page, bh);
-	block = index << (PAGE_SHIFT - inode->i_blkbits);
+	blk_cur = index << (PAGE_SHIFT - inode->i_blkbits);
 	while (bh) {
+		block = blk_cur;
+
 		if (count == 0)
 			bh->b_blocknr = 0;
 		else {
-			bh->b_blocknr = bmap(inode, block);
-			if (bh->b_blocknr == 0) {
-				/* Cannot use this file! */
+			ret = bmap(inode, &block);
+			if (ret || !block) {
 				ret = -EINVAL;
+				bh->b_blocknr = 0;
 				goto out;
 			}
+
+			bh->b_blocknr = block;
 			bh->b_bdev = inode->i_sb->s_bdev;
 			if (count < (1<<inode->i_blkbits))
 				count = 0;
@@ -399,7 +403,7 @@ static int read_page(struct file *file, unsigned long index,
 			set_buffer_mapped(bh);
 			submit_bh(REQ_OP_READ, 0, bh);
 		}
-		block++;
+		blk_cur++;
 		bh = bh->b_this_page;
 	}
 	page->index = index;
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index abbf14e9bd72..4f5afff39f2a 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -2979,12 +2979,16 @@ static int check_swap_activate(struct file *swap_file, unsigned int max)
 	while ((probe_block + blocks_per_page) <= last_block && page_no < max) {
 		unsigned block_in_page;
 		sector_t first_block;
+		sector_t block = 0;
+		int	 err = 0;
 
 		cond_resched();
 
-		first_block = bmap(inode, probe_block);
-		if (first_block == 0)
+		block = probe_block;
+		err = bmap(inode, &block);
+		if (err || !block)
 			goto bad_bmap;
+		first_block = block;
 
 		/*
 		 * It must be PAGE_SIZE aligned on-disk
@@ -2996,11 +3000,13 @@ static int check_swap_activate(struct file *swap_file, unsigned int max)
 
 		for (block_in_page = 1; block_in_page < blocks_per_page;
 					block_in_page++) {
-			sector_t block;
 
-			block = bmap(inode, probe_block + block_in_page);
-			if (block == 0)
+			block = probe_block + block_in_page;
+			err = bmap(inode, &block);
+
+			if (err || !block)
 				goto bad_bmap;
+
 			if (block != first_block + block_in_page) {
 				/* Discontiguity */
 				probe_block++;
diff --git a/fs/inode.c b/fs/inode.c
index 0f1e3b563c47..5dfe5e4bfa50 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1590,21 +1590,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 953990eb70a9..c35252897f40 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -797,18 +797,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 */
 	}
@@ -1234,11 +1239,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 56b8e358af5c..934d7c57eec6 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2803,7 +2803,7 @@ static inline ssize_t generic_write_sync(struct kiocb *iocb, ssize_t count)
 extern void emergency_sync(void);
 extern void emergency_remount(void);
 #ifdef CONFIG_BLOCK
-extern sector_t bmap(struct inode *, sector_t);
+extern int bmap(struct inode *, sector_t *);
 #endif
 extern int notify_change(struct dentry *, struct iattr *, struct inode **);
 extern int inode_permission(struct inode *, int);
diff --git a/mm/page_io.c b/mm/page_io.c
index 24ee600f9131..dfc55616ff17 100644
--- a/mm/page_io.c
+++ b/mm/page_io.c
@@ -176,8 +176,9 @@ int generic_swapfile_activate(struct swap_info_struct *sis,
 
 		cond_resched();
 
-		first_block = bmap(inode, probe_block);
-		if (first_block == 0)
+		first_block = probe_block;
+		ret = bmap(inode, &first_block);
+		if (ret || !first_block)
 			goto bad_bmap;
 
 		/*
@@ -192,9 +193,11 @@ int generic_swapfile_activate(struct swap_info_struct *sis,
 					block_in_page++) {
 			sector_t block;
 
-			block = bmap(inode, probe_block + block_in_page);
-			if (block == 0)
+			block = probe_block + block_in_page;
+			ret = bmap(inode, &block);
+			if (ret || !block)
 				goto bad_bmap;
+
 			if (block != first_block + block_in_page) {
 				/* Discontiguity */
 				probe_block++;
-- 
2.20.1

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

[PATCH 3/9] ecryptfs: drop direct calls to ->bmap

[Carlos Maiolino]

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

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

 fs/ecryptfs/mmap.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

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

[PATCH 4/9] 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>
---

Changelog:

	V4:
		- Ensure ioctl_fibmap() returns 0 in case of error returned from
		  bmap(). Otherwise we'll be changing the user interface (which
		  returns 0 in case of error)
	V3:
		- Rename usr_blk to ur_block

	V2:
		- Use a local sector_t variable to asign the block number
		  instead of using direct casting.

 fs/ioctl.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/fs/ioctl.c b/fs/ioctl.c
index fef3a6bf7c78..6b589c873bc2 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -53,19 +53,28 @@ EXPORT_SYMBOL(vfs_ioctl);
 
 static int ioctl_fibmap(struct file *filp, int __user *p)
 {
-	struct address_space *mapping = filp->f_mapping;
-	int res, block;
+	struct inode *inode = file_inode(filp);
+	int error, ur_block;
+	sector_t block;
 
-	/* do we support this mess? */
-	if (!mapping->a_ops->bmap)
-		return -EINVAL;
 	if (!capable(CAP_SYS_RAWIO))
 		return -EPERM;
-	res = get_user(block, p);
-	if (res)
-		return res;
-	res = mapping->a_ops->bmap(mapping, block);
-	return put_user(res, p);
+
+	error = get_user(ur_block, p);
+	if (error)
+		return error;
+
+	block = ur_block;
+	error = bmap(inode, &block);
+
+	if (error)
+		ur_block = 0;
+	else
+		ur_block = block;
+
+	error = put_user(ur_block, p);
+
+	return error;
 }
 
 /**
-- 
2.20.1

[PATCH 5/9] fs: Move start and length fiemap fields into fiemap_extent_info

[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, aggregate
everything into a single data structure, and pass it along.

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

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

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

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

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

[PATCH 6/9] iomap: Remove length and start fields from iomap_fiemap

[Carlos Maiolino]

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

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

---
Changelog:
	V5:
		- Rebased against 5.3
		- Fix small conflict with previous patch
		  due indentation in xfs_vn_fiemap

 fs/gfs2/inode.c       | 4 +---
 fs/iomap/fiemap.c     | 4 +++-
 fs/xfs/xfs_iops.c     | 8 ++------
 include/linux/iomap.h | 2 +-
 4 files changed, 7 insertions(+), 11 deletions(-)

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

[PATCH 7/9] fiemap: Use a callback to fill fiemap extents

[Carlos Maiolino]

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

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

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

---
	V5:
		- Rebased against 5.3 with no changes
	V3:
		- Rebase to current linux-next
		- Fix conflict on rebase
		- Merge this patch into patch 07 from V2
		- Rename fi_extents_start to fi_cb_data

	V2:
		- Now based on the rework on fiemap_extent_info (previous was
		  based on fiemap_ctx)

 fs/ioctl.c         | 45 ++++++++++++++++++++++++++-------------------
 include/linux/fs.h | 12 +++++++++---
 2 files changed, 35 insertions(+), 22 deletions(-)

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

[PATCH 8/9] Use FIEMAP for FIBMAP calls

[Carlos Maiolino]

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

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

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

The new FIEMAP_KERNEL_FIBMAP flag, is used to tell the filesystem
->fiemap interface, that the call is coming from ioctl_fibmap. The
addition of this new flag, requires an update to fiemap_check_flags(),
so it doesn't treat FIBMAP requests as invalid.

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

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

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

 fs/ext4/extents.c     |  7 +++-
 fs/f2fs/data.c        | 10 +++++-
 fs/gfs2/inode.c       |  6 +++-
 fs/inode.c            | 81 +++++++++++++++++++++++++++++++++++++++++--
 fs/ioctl.c            | 40 ++++++++++++++-------
 fs/iomap/fiemap.c     |  2 +-
 fs/ocfs2/extent_map.c |  8 ++++-
 fs/xfs/xfs_iops.c     |  6 ++++
 include/linux/fs.h    |  4 +++
 9 files changed, 145 insertions(+), 19 deletions(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 300339d1cd62..06160279a842 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -5018,7 +5018,9 @@ static int ext4_find_delayed_extent(struct inode *inode,
 	return next_del;
 }
 /* fiemap flags we can handle specified here */
-#define EXT4_FIEMAP_FLAGS	(FIEMAP_FLAG_SYNC|FIEMAP_FLAG_XATTR)
+#define EXT4_FIEMAP_FLAGS	(FIEMAP_FLAG_SYNC | \
+				 FIEMAP_FLAG_XATTR| \
+				 FIEMAP_KERNEL_FIBMAP)
 
 static int ext4_xattr_fiemap(struct inode *inode,
 				struct fiemap_extent_info *fieinfo)
@@ -5065,6 +5067,9 @@ int ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
 	if (ext4_has_inline_data(inode)) {
 		int has_inline = 1;
 
+		if (fieinfo->fi_flags & FIEMAP_KERNEL_FIBMAP)
+			return -EINVAL;
+
 		error = ext4_inline_data_fiemap(inode, fieinfo, &has_inline,
 						start, len);
 
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 83e8c2d4a7a9..439863c84b34 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1476,6 +1476,9 @@ static int f2fs_xattr_fiemap(struct inode *inode,
 	return (err < 0 ? err : 0);
 }
 
+#define F2FS_FIEMAP_COMPAT	(FIEMAP_FLAG_SYNC | \
+				 FIEMAP_FLAG_XATTR| \
+				 FIEMAP_KERNEL_FIBMAP)
 int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
 {
 	u64 start = fieinfo->fi_start;
@@ -1493,7 +1496,7 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
 			return ret;
 	}
 
-	ret = fiemap_check_flags(fieinfo, FIEMAP_FLAG_SYNC | FIEMAP_FLAG_XATTR);
+	ret = fiemap_check_flags(fieinfo, F2FS_FIEMAP_COMPAT);
 	if (ret)
 		return ret;
 
@@ -1505,6 +1508,11 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
 	}
 
 	if (f2fs_has_inline_data(inode)) {
+
+		ret = -EINVAL;
+		if (fieinfo->fi_flags & FIEMAP_KERNEL_FIBMAP)
+			goto out;
+
 		ret = f2fs_inline_data_fiemap(inode, fieinfo, start, len);
 		if (ret != -EAGAIN)
 			goto out;
diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index dc192dfd8941..a8d1c0a29b45 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -2013,7 +2013,11 @@ static int gfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
 	if (ret)
 		goto out;
 
-	ret = iomap_fiemap(inode, fieinfo, &gfs2_iomap_ops);
+	if (gfs2_is_stuffed(ip) &&
+	    (fieinfo->fi_flags & FIEMAP_KERNEL_FIBMAP))
+		ret = -EINVAL;
+	else
+		ret = iomap_fiemap(inode, fieinfo, &gfs2_iomap_ops);
 
 	gfs2_glock_dq_uninit(&gh);
 
diff --git a/fs/inode.c b/fs/inode.c
index 5dfe5e4bfa50..7a7a9f179639 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1588,6 +1588,78 @@ void iput(struct inode *inode)
 }
 EXPORT_SYMBOL(iput);
 
+static int fiemap_fill_kernel_extent(struct fiemap_extent_info *fieinfo,
+			u64 logical, u64 phys, u64 len, u32 flags)
+{
+	struct fiemap_extent *extent = fieinfo->fi_cb_data;
+
+	/* only count the extents */
+	if (fieinfo->fi_cb_data == 0) {
+		fieinfo->fi_extents_mapped++;
+		goto out;
+	}
+
+	if (fieinfo->fi_extents_mapped >= fieinfo->fi_extents_max)
+		return 1;
+
+	if (flags & FIEMAP_EXTENT_DELALLOC)
+		flags |= FIEMAP_EXTENT_UNKNOWN;
+	if (flags & FIEMAP_EXTENT_DATA_ENCRYPTED)
+		flags |= FIEMAP_EXTENT_ENCODED;
+	if (flags & (FIEMAP_EXTENT_DATA_TAIL | FIEMAP_EXTENT_DATA_INLINE))
+		flags |= FIEMAP_EXTENT_NOT_ALIGNED;
+
+	extent->fe_logical = logical;
+	extent->fe_physical = phys;
+	extent->fe_length = len;
+	extent->fe_flags = flags;
+
+	fieinfo->fi_extents_mapped++;
+
+	if (fieinfo->fi_extents_mapped == fieinfo->fi_extents_max)
+		return 1;
+
+out:
+	if (flags & FIEMAP_EXTENT_LAST)
+		return 1;
+	return 0;
+}
+
+static int bmap_fiemap(struct inode *inode, sector_t *block)
+{
+	struct fiemap_extent_info fieinfo = { 0, };
+	struct fiemap_extent fextent;
+	u64 start = *block << inode->i_blkbits;
+	int error = -EINVAL;
+
+	fextent.fe_logical = 0;
+	fextent.fe_physical = 0;
+	fieinfo.fi_extents_max = 1;
+	fieinfo.fi_extents_mapped = 0;
+	fieinfo.fi_cb_data = &fextent;
+	fieinfo.fi_start = start;
+	fieinfo.fi_len = 1 << inode->i_blkbits;
+	fieinfo.fi_cb = fiemap_fill_kernel_extent;
+	fieinfo.fi_flags = (FIEMAP_KERNEL_FIBMAP | FIEMAP_FLAG_SYNC);
+
+	error = inode->i_op->fiemap(inode, &fieinfo);
+
+	if (error)
+		return error;
+
+	if (fieinfo.fi_flags & (FIEMAP_EXTENT_UNKNOWN |
+				FIEMAP_EXTENT_ENCODED |
+				FIEMAP_EXTENT_DATA_INLINE |
+				FIEMAP_EXTENT_UNWRITTEN |
+				FIEMAP_EXTENT_SHARED))
+		return -EINVAL;
+
+	*block = (fextent.fe_physical +
+		  (start - fextent.fe_logical)) >> inode->i_blkbits;
+
+	return error;
+}
+
 /**
  *	bmap	- find a block number in a file
  *	@inode:  inode owning the block number being requested
@@ -1604,10 +1676,15 @@ EXPORT_SYMBOL(iput);
  */
 int bmap(struct inode *inode, sector_t *block)
 {
-	if (!inode->i_mapping->a_ops->bmap)
+	if (inode->i_op->fiemap)
+		return bmap_fiemap(inode, block);
+
+	if (inode->i_mapping->a_ops->bmap)
+		*block = inode->i_mapping->a_ops->bmap(inode->i_mapping,
+						       *block);
+	else
 		return -EINVAL;
 
-	*block = inode->i_mapping->a_ops->bmap(inode->i_mapping, *block);
 	return 0;
 }
 EXPORT_SYMBOL(bmap);
diff --git a/fs/ioctl.c b/fs/ioctl.c
index d72696c222de..0759ac6e4c7e 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -77,11 +77,8 @@ static int ioctl_fibmap(struct file *filp, int __user *p)
 	return error;
 }
 
-#define SET_UNKNOWN_FLAGS	(FIEMAP_EXTENT_DELALLOC)
-#define SET_NO_UNMOUNTED_IO_FLAGS	(FIEMAP_EXTENT_DATA_ENCRYPTED)
-#define SET_NOT_ALIGNED_FLAGS	(FIEMAP_EXTENT_DATA_TAIL|FIEMAP_EXTENT_DATA_INLINE)
-int fiemap_fill_user_extent(struct fiemap_extent_info *fieinfo, u64 logical,
-			    u64 phys, u64 len, u32 flags)
+static int fiemap_fill_user_extent(struct fiemap_extent_info *fieinfo,
+			u64 logical, u64 phys, u64 len, u32 flags)
 {
 	struct fiemap_extent extent;
 	struct fiemap_extent __user *dest = fieinfo->fi_cb_data;
@@ -89,17 +86,17 @@ int fiemap_fill_user_extent(struct fiemap_extent_info *fieinfo, u64 logical,
 	/* only count the extents */
 	if (fieinfo->fi_extents_max == 0) {
 		fieinfo->fi_extents_mapped++;
-		return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0;
+		goto out;
 	}
 
 	if (fieinfo->fi_extents_mapped >= fieinfo->fi_extents_max)
 		return 1;
 
-	if (flags & SET_UNKNOWN_FLAGS)
+	if (flags & FIEMAP_EXTENT_DELALLOC)
 		flags |= FIEMAP_EXTENT_UNKNOWN;
-	if (flags & SET_NO_UNMOUNTED_IO_FLAGS)
+	if (flags & FIEMAP_EXTENT_DATA_ENCRYPTED)
 		flags |= FIEMAP_EXTENT_ENCODED;
-	if (flags & SET_NOT_ALIGNED_FLAGS)
+	if (flags & (FIEMAP_EXTENT_DATA_TAIL | FIEMAP_EXTENT_DATA_INLINE))
 		flags |= FIEMAP_EXTENT_NOT_ALIGNED;
 
 	memset(&extent, 0, sizeof(extent));
@@ -115,7 +112,11 @@ int fiemap_fill_user_extent(struct fiemap_extent_info *fieinfo, u64 logical,
 	fieinfo->fi_extents_mapped++;
 	if (fieinfo->fi_extents_mapped == fieinfo->fi_extents_max)
 		return 1;
-	return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0;
+
+out:
+	if (flags & FIEMAP_EXTENT_LAST)
+		return 1;
+	return 0;
 }
 
 /**
@@ -151,13 +152,23 @@ EXPORT_SYMBOL(fiemap_fill_next_extent);
  * flags, the invalid values will be written into the fieinfo structure, and
  * -EBADR is returned, which tells ioctl_fiemap() to return those values to
  * userspace. For this reason, a return code of -EBADR should be preserved.
+ * In case ->fiemap is being used for FIBMAP calls, and the filesystem does not
+ * support it, return -EINVAL.
  *
- * Returns 0 on success, -EBADR on bad flags.
+ * Returns 0 on success, -EBADR on bad flags, -EINVAL for an unsupported FIBMAP
+ * request.
  */
 int fiemap_check_flags(struct fiemap_extent_info *fieinfo, u32 fs_flags)
 {
 	u32 incompat_flags;
 
+	if (fieinfo->fi_flags & FIEMAP_KERNEL_FIBMAP) {
+		if (fs_flags & FIEMAP_KERNEL_FIBMAP)
+			return 0;
+
+		return -EINVAL;
+	}
+
 	incompat_flags = fieinfo->fi_flags & ~(FIEMAP_FLAGS_COMPAT & fs_flags);
 	if (incompat_flags) {
 		fieinfo->fi_flags = incompat_flags;
@@ -208,6 +219,10 @@ static int ioctl_fiemap(struct file *filp, unsigned long arg)
 	if (fiemap.fm_extent_count > FIEMAP_MAX_EXTENTS)
 		return -EINVAL;
 
+	/* Userspace has no access to this flag */
+	if (fiemap.fm_flags & FIEMAP_KERNEL_FIBMAP)
+		return -EINVAL;
+
 	error = fiemap_check_ranges(sb, fiemap.fm_start, fiemap.fm_length,
 				    &len);
 	if (error)
@@ -318,7 +333,8 @@ int __generic_block_fiemap(struct inode *inode,
 	bool past_eof = false, whole_file = false;
 	int ret = 0;
 
-	ret = fiemap_check_flags(fieinfo, FIEMAP_FLAG_SYNC);
+	ret = fiemap_check_flags(fieinfo,
+				 FIEMAP_FLAG_SYNC | FIEMAP_KERNEL_FIBMAP);
 	if (ret)
 		return ret;
 
diff --git a/fs/iomap/fiemap.c b/fs/iomap/fiemap.c
index 03f214f5df94..7116c451e67a 100644
--- a/fs/iomap/fiemap.c
+++ b/fs/iomap/fiemap.c
@@ -76,7 +76,7 @@ int iomap_fiemap(struct inode *inode, struct fiemap_extent_info *fi,
 	ctx.fi = fi;
 	ctx.prev.type = IOMAP_HOLE;
 
-	ret = fiemap_check_flags(fi, FIEMAP_FLAG_SYNC);
+	ret = fiemap_check_flags(fi, FIEMAP_FLAG_SYNC | FIEMAP_KERNEL_FIBMAP);
 	if (ret)
 		return ret;
 
diff --git a/fs/ocfs2/extent_map.c b/fs/ocfs2/extent_map.c
index 1a5b6af62ee0..7d8ff6888c0d 100644
--- a/fs/ocfs2/extent_map.c
+++ b/fs/ocfs2/extent_map.c
@@ -734,7 +734,7 @@ static int ocfs2_fiemap_inline(struct inode *inode, struct buffer_head *di_bh,
 	return 0;
 }
 
-#define OCFS2_FIEMAP_FLAGS	(FIEMAP_FLAG_SYNC)
+#define OCFS2_FIEMAP_FLAGS	(FIEMAP_FLAG_SYNC | FIEMAP_KERNEL_FIBMAP)
 
 int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
 {
@@ -743,6 +743,7 @@ int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
 	unsigned int hole_size;
 	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
 	u64 len_bytes, phys_bytes, virt_bytes;
+
 	struct buffer_head *di_bh = NULL;
 	struct ocfs2_extent_rec rec;
 	u64 map_start = fieinfo->fi_start;
@@ -752,6 +753,11 @@ int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
 	if (ret)
 		return ret;
 
+	if (fieinfo->fi_flags & FIEMAP_KERNEL_FIBMAP) {
+		if (ocfs2_is_refcount_inode(inode))
+			return -EINVAL;
+	}
+
 	ret = ocfs2_inode_lock(inode, &di_bh, 0);
 	if (ret) {
 		mlog_errno(ret);
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index 514c4620a9e8..e43742fe101a 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -20,6 +20,7 @@
 #include "xfs_symlink.h"
 #include "xfs_dir2.h"
 #include "xfs_iomap.h"
+#include "xfs_reflink.h"
 
 #include <linux/xattr.h>
 #include <linux/posix_acl.h>
@@ -1102,8 +1103,13 @@ xfs_vn_fiemap(
 	struct inode			*inode,
 	struct fiemap_extent_info	*fieinfo)
 {
+	struct xfs_inode		*ip = XFS_I(inode);
 	int				error;
 
+	if (fieinfo->fi_flags & FIEMAP_KERNEL_FIBMAP)
+		if (xfs_is_cow_inode(ip) || XFS_IS_REALTIME_INODE(ip))
+			return -EINVAL;
+
 	xfs_ilock(XFS_I(inode), XFS_IOLOCK_SHARED);
 	if (fieinfo->fi_flags & FIEMAP_FLAG_XATTR) {
 		fieinfo->fi_flags &= ~FIEMAP_FLAG_XATTR;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 5f37f13be260..30aa65fc5ac4 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1716,6 +1716,10 @@ extern bool may_open_dev(const struct path *path);
 typedef int (*fiemap_fill_cb)(struct fiemap_extent_info *fieinfo, u64 logical,
 			      u64 phys, u64 len, u32 flags);
 
+#define FIEMAP_KERNEL_FIBMAP 0x10000000		/* FIBMAP call through FIEMAP
+						   interface. This is a kernel
+						   only flag */
+
 struct fiemap_extent_info {
 	unsigned int	fi_flags;		/* Flags as passed from user */
 	u64		fi_start;		/* Logical offset at which
-- 
2.20.1

[PATCH 9/9] 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>
---

Changelog:

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

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

diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
index f16d5f196c6b..097cf52ad09e 100644
--- a/fs/xfs/xfs_aops.c
+++ b/fs/xfs/xfs_aops.c
@@ -1132,29 +1132,6 @@ xfs_vm_releasepage(
 	return iomap_releasepage(page, gfp_mask);
 }
 
-STATIC sector_t
-xfs_vm_bmap(
-	struct address_space	*mapping,
-	sector_t		block)
-{
-	struct xfs_inode	*ip = XFS_I(mapping->host);
-
-	trace_xfs_vm_bmap(ip);
-
-	/*
-	 * The swap code (ab-)uses ->bmap to get a block mapping and then
-	 * bypasses the file system for actual I/O.  We really can't allow
-	 * that on reflinks inodes, so we have to skip out here.  And yes,
-	 * 0 is the magic code for a bmap error.
-	 *
-	 * Since we don't pass back blockdev info, we can't return bmap
-	 * information for rt files either.
-	 */
-	if (xfs_is_cow_inode(ip) || XFS_IS_REALTIME_INODE(ip))
-		return 0;
-	return iomap_bmap(mapping, block, &xfs_iomap_ops);
-}
-
 STATIC int
 xfs_vm_readpage(
 	struct file		*unused,
@@ -1193,7 +1170,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 8094b1920eef..fab17ea7d1eb 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -621,7 +621,6 @@ DEFINE_INODE_EVENT(xfs_readdir);
 #ifdef CONFIG_XFS_POSIX_ACL
 DEFINE_INODE_EVENT(xfs_get_acl);
 #endif
-DEFINE_INODE_EVENT(xfs_vm_bmap);
 DEFINE_INODE_EVENT(xfs_file_ioctl);
 DEFINE_INODE_EVENT(xfs_file_compat_ioctl);
 DEFINE_INODE_EVENT(xfs_ioctl_setattr);
-- 
2.20.1

Re: [PATCH 5/9] fs: Move start and length fiemap fields into fiemap_extent_info

[kbuild test robot]

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

Hi Carlos,

Thank you for the patch! Yet something to improve:

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

url:    https://github.com/0day-ci/linux/commits/Carlos-Maiolino/New-fiemap-infrastructure-and-bmap-removal/20190808-221354
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=sh 

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

All errors (new ones prefixed by >>):

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

vim +1006 fs//cifs/cifsfs.c

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

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

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

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

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

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

[kbuild test robot]

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

Hi Carlos,

Thank you for the patch! Yet something to improve:

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

url:    https://github.com/0day-ci/linux/commits/Carlos-Maiolino/New-fiemap-infrastructure-and-bmap-removal/20190808-221354
config: sh-allnoconfig (attached as .config)
compiler: sh4-linux-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=sh 

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

All errors (new ones prefixed by >>):

   fs/ioctl.c: In function 'ioctl_fibmap':
>> fs/ioctl.c:68:10: error: implicit declaration of function 'bmap'; did you mean 'kmap'? [-Werror=implicit-function-declaration]
     error = bmap(inode, &block);
             ^~~~
             kmap
   cc1: some warnings being treated as errors

vim +68 fs/ioctl.c

    53	
    54	static int ioctl_fibmap(struct file *filp, int __user *p)
    55	{
    56		struct inode *inode = file_inode(filp);
    57		int error, ur_block;
    58		sector_t block;
    59	
    60		if (!capable(CAP_SYS_RAWIO))
    61			return -EPERM;
    62	
    63		error = get_user(ur_block, p);
    64		if (error)
    65			return error;
    66	
    67		block = ur_block;
  > 68		error = bmap(inode, &block);
    69	
    70		if (error)
    71			ur_block = 0;
    72		else
    73			ur_block = block;
    74	
    75		error = put_user(ur_block, p);
    76	
    77		return error;
    78	}
    79	

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

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

Re: [PATCH 1/9] fs: Enable bmap() function to properly return errors

[kbuild test robot]

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

Hi Carlos,

Thank you for the patch! Perhaps something to improve:

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

url:    https://github.com/0day-ci/linux/commits/Carlos-Maiolino/New-fiemap-infrastructure-and-bmap-removal/20190808-221354
reproduce: make htmldocs

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

All warnings (new ones prefixed by >>):

   Warning: The Sphinx 'sphinx_rtd_theme' HTML theme was not found. Make sure you have the theme installed to produce pretty HTML output. Falling back to the default theme.
   WARNING: dot(1) not found, for better output quality install graphviz from http://www.graphviz.org
   WARNING: convert(1) not found, for SVG to PDF conversion install ImageMagick (https://www.imagemagick.org)
   include/linux/spi/spi.h:190: warning: Function parameter or member 'driver_override' not described in 'spi_device'
   include/linux/input/sparse-keymap.h:43: warning: Function parameter or member 'sw' not described in 'key_entry'
   drivers/gpu/drm/mcde/mcde_drv.c:1: warning: 'ST-Ericsson MCDE DRM Driver' not found
   include/linux/i2c.h:337: warning: Function parameter or member 'init_irq' not described in 'i2c_client'
   include/linux/regulator/machine.h:196: warning: Function parameter or member 'max_uV_step' not described in 'regulation_constraints'
   include/linux/regulator/driver.h:223: warning: Function parameter or member 'resume' not described in 'regulator_ops'
   drivers/usb/typec/bus.c:1: warning: 'typec_altmode_unregister_driver' not found
   drivers/usb/typec/bus.c:1: warning: 'typec_altmode_register_driver' not found
   drivers/usb/typec/class.c:1: warning: 'typec_altmode_register_notifier' not found
   drivers/usb/typec/class.c:1: warning: 'typec_altmode_unregister_notifier' not found
   include/linux/w1.h:272: warning: Function parameter or member 'of_match_table' not described in 'w1_family'
>> fs/inode.c:1607: warning: Function parameter or member 'block' not described in 'bmap'
   fs/direct-io.c:258: warning: Excess function parameter 'offset' description in 'dio_complete'
   fs/libfs.c:496: warning: Excess function parameter 'available' description in 'simple_write_end'
   fs/posix_acl.c:647: warning: Function parameter or member 'inode' not described in 'posix_acl_update_mode'
   fs/posix_acl.c:647: warning: Function parameter or member 'mode_p' not described in 'posix_acl_update_mode'
   fs/posix_acl.c:647: warning: Function parameter or member 'acl' not described in 'posix_acl_update_mode'
   lib/genalloc.c:1: warning: 'gen_pool_add_virt' not found
   lib/genalloc.c:1: warning: 'gen_pool_alloc' not found
   lib/genalloc.c:1: warning: 'gen_pool_free' not found
   lib/genalloc.c:1: warning: 'gen_pool_alloc_algo' not found
   mm/util.c:1: warning: 'get_user_pages_fast' not found
   mm/slab.c:4215: warning: Function parameter or member 'objp' not described in '__ksize'
   include/linux/skbuff.h:893: warning: Function parameter or member 'dev_scratch' not described in 'sk_buff'
   include/linux/skbuff.h:893: warning: Function parameter or member 'list' not described in 'sk_buff'
   include/linux/skbuff.h:893: warning: Function parameter or member 'ip_defrag_offset' not described in 'sk_buff'
   include/linux/skbuff.h:893: warning: Function parameter or member 'skb_mstamp_ns' not described in 'sk_buff'
   include/linux/skbuff.h:893: warning: Function parameter or member '__cloned_offset' not described in 'sk_buff'
   include/linux/skbuff.h:893: warning: Function parameter or member 'head_frag' not described in 'sk_buff'
   include/linux/skbuff.h:893: warning: Function parameter or member '__pkt_type_offset' not described in 'sk_buff'
   include/linux/skbuff.h:893: warning: Function parameter or member 'encapsulation' not described in 'sk_buff'
   include/linux/skbuff.h:893: warning: Function parameter or member 'encap_hdr_csum' not described in 'sk_buff'
   include/linux/skbuff.h:893: warning: Function parameter or member 'csum_valid' not described in 'sk_buff'
   include/linux/skbuff.h:893: warning: Function parameter or member '__pkt_vlan_present_offset' not described in 'sk_buff'
   include/linux/skbuff.h:893: warning: Function parameter or member 'vlan_present' not described in 'sk_buff'
   include/linux/skbuff.h:893: warning: Function parameter or member 'csum_complete_sw' not described in 'sk_buff'
   include/linux/skbuff.h:893: warning: Function parameter or member 'csum_level' not described in 'sk_buff'
   include/linux/skbuff.h:893: warning: Function parameter or member 'inner_protocol_type' not described in 'sk_buff'
   include/linux/skbuff.h:893: warning: Function parameter or member 'remcsum_offload' not described in 'sk_buff'
   include/linux/skbuff.h:893: warning: Function parameter or member 'sender_cpu' not described in 'sk_buff'
   include/linux/skbuff.h:893: warning: Function parameter or member 'reserved_tailroom' not described in 'sk_buff'
   include/linux/skbuff.h:893: warning: Function parameter or member 'inner_ipproto' not described in 'sk_buff'
   include/net/sock.h:233: warning: Function parameter or member 'skc_addrpair' not described in 'sock_common'
   include/net/sock.h:233: warning: Function parameter or member 'skc_portpair' not described in 'sock_common'
   include/net/sock.h:233: warning: Function parameter or member 'skc_ipv6only' not described in 'sock_common'
   include/net/sock.h:233: warning: Function parameter or member 'skc_net_refcnt' not described in 'sock_common'
   include/net/sock.h:233: warning: Function parameter or member 'skc_v6_daddr' not described in 'sock_common'
   include/net/sock.h:233: warning: Function parameter or member 'skc_v6_rcv_saddr' not described in 'sock_common'
   include/net/sock.h:233: warning: Function parameter or member 'skc_cookie' not described in 'sock_common'
   include/net/sock.h:233: warning: Function parameter or member 'skc_listener' not described in 'sock_common'
   include/net/sock.h:233: warning: Function parameter or member 'skc_tw_dr' not described in 'sock_common'
   include/net/sock.h:233: warning: Function parameter or member 'skc_rcv_wnd' not described in 'sock_common'
   include/net/sock.h:233: warning: Function parameter or member 'skc_tw_rcv_nxt' not described in 'sock_common'
   include/net/sock.h:515: warning: Function parameter or member 'sk_rx_skb_cache' not described in 'sock'
   include/net/sock.h:515: warning: Function parameter or member 'sk_wq_raw' not described in 'sock'
   include/net/sock.h:515: warning: Function parameter or member 'tcp_rtx_queue' not described in 'sock'
   include/net/sock.h:515: warning: Function parameter or member 'sk_tx_skb_cache' not described in 'sock'
   include/net/sock.h:515: warning: Function parameter or member 'sk_route_forced_caps' not described in 'sock'
   include/net/sock.h:515: warning: Function parameter or member 'sk_txtime_report_errors' not described in 'sock'
   include/net/sock.h:515: warning: Function parameter or member 'sk_validate_xmit_skb' not described in 'sock'
   include/net/sock.h:515: warning: Function parameter or member 'sk_bpf_storage' not described in 'sock'
   include/net/sock.h:2439: warning: Function parameter or member 'tcp_rx_skb_cache_key' not described in 'DECLARE_STATIC_KEY_FALSE'
   include/net/sock.h:2439: warning: Excess function parameter 'sk' description in 'DECLARE_STATIC_KEY_FALSE'
   include/net/sock.h:2439: warning: Excess function parameter 'skb' description in 'DECLARE_STATIC_KEY_FALSE'
   include/linux/netdevice.h:2040: warning: Function parameter or member 'gso_partial_features' not described in 'net_device'
   include/linux/netdevice.h:2040: warning: Function parameter or member 'l3mdev_ops' not described in 'net_device'
   include/linux/netdevice.h:2040: warning: Function parameter or member 'xfrmdev_ops' not described in 'net_device'
   include/linux/netdevice.h:2040: warning: Function parameter or member 'tlsdev_ops' not described in 'net_device'
   include/linux/netdevice.h:2040: warning: Function parameter or member 'name_assign_type' not described in 'net_device'
   include/linux/netdevice.h:2040: warning: Function parameter or member 'ieee802154_ptr' not described in 'net_device'
   include/linux/netdevice.h:2040: warning: Function parameter or member 'mpls_ptr' not described in 'net_device'
   include/linux/netdevice.h:2040: warning: Function parameter or member 'xdp_prog' not described in 'net_device'
   include/linux/netdevice.h:2040: warning: Function parameter or member 'gro_flush_timeout' not described in 'net_device'
   include/linux/netdevice.h:2040: warning: Function parameter or member 'nf_hooks_ingress' not described in 'net_device'
   include/linux/netdevice.h:2040: warning: Function parameter or member '____cacheline_aligned_in_smp' not described in 'net_device'
   include/linux/netdevice.h:2040: warning: Function parameter or member 'qdisc_hash' not described in 'net_device'
   include/linux/netdevice.h:2040: warning: Function parameter or member 'xps_cpus_map' not described in 'net_device'
   include/linux/netdevice.h:2040: warning: Function parameter or member 'xps_rxqs_map' not described in 'net_device'
   include/linux/phylink.h:56: warning: Function parameter or member '__ETHTOOL_DECLARE_LINK_MODE_MASK(advertising' not described in 'phylink_link_state'
   include/linux/phylink.h:56: warning: Function parameter or member '__ETHTOOL_DECLARE_LINK_MODE_MASK(lp_advertising' not described in 'phylink_link_state'
   drivers/net/phy/phylink.c:595: warning: Function parameter or member 'config' not described in 'phylink_create'
   drivers/net/phy/phylink.c:595: warning: Excess function parameter 'ndev' description in 'phylink_create'
   include/linux/lsm_hooks.h:1811: warning: Function parameter or member 'quotactl' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1811: warning: Function parameter or member 'quota_on' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1811: warning: Function parameter or member 'sb_free_mnt_opts' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1811: warning: Function parameter or member 'sb_eat_lsm_opts' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1811: warning: Function parameter or member 'sb_kern_mount' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1811: warning: Function parameter or member 'sb_show_options' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1811: warning: Function parameter or member 'sb_add_mnt_opt' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1811: warning: Function parameter or member 'd_instantiate' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1811: warning: Function parameter or member 'getprocattr' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1811: warning: Function parameter or member 'setprocattr' not described in 'security_list_options'
   drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:142: warning: Function parameter or member 'blockable' not described in 'amdgpu_mn_read_lock'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:347: warning: cannot understand function prototype: 'struct amdgpu_vm_pt_cursor '
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:348: warning: cannot understand function prototype: 'struct amdgpu_vm_pt_cursor '
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:494: warning: Function parameter or member 'start' not described in 'amdgpu_vm_pt_first_dfs'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:546: warning: Function parameter or member 'adev' not described in 'for_each_amdgpu_vm_pt_dfs_safe'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:546: warning: Function parameter or member 'vm' not described in 'for_each_amdgpu_vm_pt_dfs_safe'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:546: warning: Function parameter or member 'start' not described in 'for_each_amdgpu_vm_pt_dfs_safe'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:546: warning: Function parameter or member 'cursor' not described in 'for_each_amdgpu_vm_pt_dfs_safe'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:546: warning: Function parameter or member 'entry' not described in 'for_each_amdgpu_vm_pt_dfs_safe'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:823: warning: Function parameter or member 'level' not described in 'amdgpu_vm_bo_param'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1285: warning: Function parameter or member 'params' not described in 'amdgpu_vm_update_flags'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1285: warning: Function parameter or member 'bo' not described in 'amdgpu_vm_update_flags'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1285: warning: Function parameter or member 'level' not described in 'amdgpu_vm_update_flags'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1285: warning: Function parameter or member 'pe' not described in 'amdgpu_vm_update_flags'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1285: warning: Function parameter or member 'addr' not described in 'amdgpu_vm_update_flags'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1285: warning: Function parameter or member 'count' not described in 'amdgpu_vm_update_flags'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1285: warning: Function parameter or member 'incr' not described in 'amdgpu_vm_update_flags'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1285: warning: Function parameter or member 'flags' not described in 'amdgpu_vm_update_flags'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:2822: warning: Function parameter or member 'pasid' not described in 'amdgpu_vm_make_compute'
   drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c:378: warning: Excess function parameter 'entry' description in 'amdgpu_irq_dispatch'

vim +1607 fs/inode.c

  1590	
  1591	/**
  1592	 *	bmap	- find a block number in a file
  1593	 *	@inode:  inode owning the block number being requested
  1594	 *	@*block: pointer containing the block to find
  1595	 *
  1596	 *	Replaces the value in *block with the block number on the device holding
  1597	 *	corresponding to the requested block number in the file.
  1598	 *	That is, asked for block 4 of inode 1 the function will replace the
  1599	 *	4 in *block, with disk block relative to the disk start that holds that
  1600	 *	block of the file.
  1601	 *
  1602	 *	Returns -EINVAL in case of error, 0 otherwise. If mapping falls into a
  1603	 *	hole, returns 0 and *block is also set to 0.
  1604	 */
  1605	int bmap(struct inode *inode, sector_t *block)
  1606	{
> 1607		if (!inode->i_mapping->a_ops->bmap)
  1608			return -EINVAL;
  1609	
  1610		*block = inode->i_mapping->a_ops->bmap(inode->i_mapping, *block);
  1611		return 0;
  1612	}
  1613	EXPORT_SYMBOL(bmap);
  1614	

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

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

Re: [PATCH 3/9] ecryptfs: drop direct calls to ->bmap

[kbuild test robot]

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

Hi Carlos,

Thank you for the patch! Yet something to improve:

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

url:    https://github.com/0day-ci/linux/commits/Carlos-Maiolino/New-fiemap-infrastructure-and-bmap-removal/20190808-221354
config: x86_64-randconfig-g004-201931 (attached as .config)
compiler: gcc-7 (Debian 7.4.0-10) 7.4.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

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

All errors (new ones prefixed by >>):

   fs/ecryptfs/mmap.c: In function 'ecryptfs_bmap':
>> fs/ecryptfs/mmap.c:528:12: error: implicit declaration of function 'bmap'; did you mean 'kmap'? [-Werror=implicit-function-declaration]
     int ret = bmap(lower_inode, &block);
               ^~~~
               kmap
   cc1: some warnings being treated as errors

vim +528 fs/ecryptfs/mmap.c

   524	
   525	static sector_t ecryptfs_bmap(struct address_space *mapping, sector_t block)
   526	{
   527		struct inode *lower_inode = ecryptfs_inode_to_lower(mapping->host);
 > 528		int ret = bmap(lower_inode, &block);
   529	
   530		if (ret)
   531			return 0;
   532		return block;
   533	}
   534	

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

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

Re: [PATCH 7/9] fiemap: Use a callback to fill fiemap extents

[kbuild test robot]

Hi Carlos,

Thank you for the patch! Perhaps something to improve:

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

url:    https://github.com/0day-ci/linux/commits/Carlos-Maiolino/New-fiemap-infrastructure-and-bmap-removal/20190808-221354
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.1-rc1-7-g2b96cd8-dirty
        make ARCH=x86_64 allmodconfig
        make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'

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


sparse warnings: (new ones prefixed by >>)

>> fs/ioctl.c:87:52: sparse: sparse: incorrect type in initializer (different address spaces) @@    expected struct fiemap_extent [noderef] <asn:1> *dest @@    got n:1> *dest @@
>> fs/ioctl.c:87:52: sparse:    expected struct fiemap_extent [noderef] <asn:1> *dest
>> fs/ioctl.c:87:52: sparse:    got void *fi_cb_data
   fs/ioctl.c:83:5: sparse: sparse: symbol 'fiemap_fill_user_extent' was not declared. Should it be static?
>> fs/ioctl.c:218:28: sparse: sparse: incorrect type in assignment (different address spaces) @@    expected void *[assigned] fi_cb_data @@    got struct fiemap_extent [nvoid *[assigned] fi_cb_data @@
>> fs/ioctl.c:218:28: sparse:    expected void *[assigned] fi_cb_data
>> fs/ioctl.c:218:28: sparse:    got struct fiemap_extent [noderef] <asn:1> *
>> fs/ioctl.c:224:14: sparse: sparse: incorrect type in argument 1 (different address spaces) @@    expected void const volatile [noderef] <asn:1> * @@    got oderef] <asn:1> * @@
>> fs/ioctl.c:224:14: sparse:    expected void const volatile [noderef] <asn:1> *
>> fs/ioctl.c:224:14: sparse:    got void *[assigned] fi_cb_data

vim +87 fs/ioctl.c

    79	
    80	#define SET_UNKNOWN_FLAGS	(FIEMAP_EXTENT_DELALLOC)
    81	#define SET_NO_UNMOUNTED_IO_FLAGS	(FIEMAP_EXTENT_DATA_ENCRYPTED)
    82	#define SET_NOT_ALIGNED_FLAGS	(FIEMAP_EXTENT_DATA_TAIL|FIEMAP_EXTENT_DATA_INLINE)
    83	int fiemap_fill_user_extent(struct fiemap_extent_info *fieinfo, u64 logical,
    84				    u64 phys, u64 len, u32 flags)
    85	{
    86		struct fiemap_extent extent;
  > 87		struct fiemap_extent __user *dest = fieinfo->fi_cb_data;
    88	
    89		/* only count the extents */
    90		if (fieinfo->fi_extents_max == 0) {
    91			fieinfo->fi_extents_mapped++;
    92			return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0;
    93		}
    94	
    95		if (fieinfo->fi_extents_mapped >= fieinfo->fi_extents_max)
    96			return 1;
    97	
    98		if (flags & SET_UNKNOWN_FLAGS)
    99			flags |= FIEMAP_EXTENT_UNKNOWN;
   100		if (flags & SET_NO_UNMOUNTED_IO_FLAGS)
   101			flags |= FIEMAP_EXTENT_ENCODED;
   102		if (flags & SET_NOT_ALIGNED_FLAGS)
   103			flags |= FIEMAP_EXTENT_NOT_ALIGNED;
   104	
   105		memset(&extent, 0, sizeof(extent));
   106		extent.fe_logical = logical;
   107		extent.fe_physical = phys;
   108		extent.fe_length = len;
   109		extent.fe_flags = flags;
   110	
   111		dest += fieinfo->fi_extents_mapped;
   112		if (copy_to_user(dest, &extent, sizeof(extent)))
   113			return -EFAULT;
   114	
   115		fieinfo->fi_extents_mapped++;
   116		if (fieinfo->fi_extents_mapped == fieinfo->fi_extents_max)
   117			return 1;
   118		return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0;
   119	}
   120	
   121	/**
   122	 * fiemap_fill_next_extent - Fiemap helper function
   123	 * @fieinfo:	Fiemap context passed into ->fiemap
   124	 * @logical:	Extent logical start offset, in bytes
   125	 * @phys:	Extent physical start offset, in bytes
   126	 * @len:	Extent length, in bytes
   127	 * @flags:	FIEMAP_EXTENT flags that describe this extent
   128	 *
   129	 * Called from file system ->fiemap callback. Will populate extent
   130	 * info as passed in via arguments and copy to user memory. On
   131	 * success, extent count on fieinfo is incremented.
   132	 *
   133	 * Returns 0 on success, -errno on error, 1 if this was the last
   134	 * extent that will fit in user array.
   135	 */
   136	int fiemap_fill_next_extent(struct fiemap_extent_info *fieinfo, u64 logical,
   137				    u64 phys, u64 len, u32 flags)
   138	{
   139		return fieinfo->fi_cb(fieinfo, logical, phys, len, flags);
   140	}
   141	EXPORT_SYMBOL(fiemap_fill_next_extent);
   142	
   143	/**
   144	 * fiemap_check_flags - check validity of requested flags for fiemap
   145	 * @fieinfo:	Fiemap context passed into ->fiemap
   146	 * @fs_flags:	Set of fiemap flags that the file system understands
   147	 *
   148	 * Called from file system ->fiemap callback. This will compute the
   149	 * intersection of valid fiemap flags and those that the fs supports. That
   150	 * value is then compared against the user supplied flags. In case of bad user
   151	 * flags, the invalid values will be written into the fieinfo structure, and
   152	 * -EBADR is returned, which tells ioctl_fiemap() to return those values to
   153	 * userspace. For this reason, a return code of -EBADR should be preserved.
   154	 *
   155	 * Returns 0 on success, -EBADR on bad flags.
   156	 */
   157	int fiemap_check_flags(struct fiemap_extent_info *fieinfo, u32 fs_flags)
   158	{
   159		u32 incompat_flags;
   160	
   161		incompat_flags = fieinfo->fi_flags & ~(FIEMAP_FLAGS_COMPAT & fs_flags);
   162		if (incompat_flags) {
   163			fieinfo->fi_flags = incompat_flags;
   164			return -EBADR;
   165		}
   166		return 0;
   167	}
   168	EXPORT_SYMBOL(fiemap_check_flags);
   169	
   170	static int fiemap_check_ranges(struct super_block *sb,
   171				       u64 start, u64 len, u64 *new_len)
   172	{
   173		u64 maxbytes = (u64) sb->s_maxbytes;
   174	
   175		*new_len = len;
   176	
   177		if (len == 0)
   178			return -EINVAL;
   179	
   180		if (start > maxbytes)
   181			return -EFBIG;
   182	
   183		/*
   184		 * Shrink request scope to what the fs can actually handle.
   185		 */
   186		if (len > maxbytes || (maxbytes - len) < start)
   187			*new_len = maxbytes - start;
   188	
   189		return 0;
   190	}
   191	
   192	static int ioctl_fiemap(struct file *filp, unsigned long arg)
   193	{
   194		struct fiemap fiemap;
   195		struct fiemap __user *ufiemap = (struct fiemap __user *) arg;
   196		struct fiemap_extent_info fieinfo = { 0, };
   197		struct inode *inode = file_inode(filp);
   198		struct super_block *sb = inode->i_sb;
   199		u64 len;
   200		int error;
   201	
   202		if (!inode->i_op->fiemap)
   203			return -EOPNOTSUPP;
   204	
   205		if (copy_from_user(&fiemap, ufiemap, sizeof(fiemap)))
   206			return -EFAULT;
   207	
   208		if (fiemap.fm_extent_count > FIEMAP_MAX_EXTENTS)
   209			return -EINVAL;
   210	
   211		error = fiemap_check_ranges(sb, fiemap.fm_start, fiemap.fm_length,
   212					    &len);
   213		if (error)
   214			return error;
   215	
   216		fieinfo.fi_flags = fiemap.fm_flags;
   217		fieinfo.fi_extents_max = fiemap.fm_extent_count;
 > 218		fieinfo.fi_cb_data = ufiemap->fm_extents;
   219		fieinfo.fi_start = fiemap.fm_start;
   220		fieinfo.fi_len = len;
   221		fieinfo.fi_cb = fiemap_fill_user_extent;
   222	
   223		if (fiemap.fm_extent_count != 0 &&
 > 224		    !access_ok(fieinfo.fi_cb_data,
   225			       fieinfo.fi_extents_max * sizeof(struct fiemap_extent)))
   226			return -EFAULT;
   227	
   228		if (fieinfo.fi_flags & FIEMAP_FLAG_SYNC)
   229			filemap_write_and_wait(inode->i_mapping);
   230	
   231		error = inode->i_op->fiemap(inode, &fieinfo);
   232		fiemap.fm_flags = fieinfo.fi_flags;
   233		fiemap.fm_mapped_extents = fieinfo.fi_extents_mapped;
   234		if (copy_to_user(ufiemap, &fiemap, sizeof(fiemap)))
   235			error = -EFAULT;
   236	
   237		return error;
   238	}
   239	

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

Re: [PATCH 8/9] Use FIEMAP for FIBMAP calls

[kbuild test robot]

Hi Carlos,

Thank you for the patch! Perhaps something to improve:

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

url:    https://github.com/0day-ci/linux/commits/Carlos-Maiolino/New-fiemap-infrastructure-and-bmap-removal/20190808-221354
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.1-rc1-7-g2b96cd8-dirty
        make ARCH=x86_64 allmodconfig
        make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'

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


sparse warnings: (new ones prefixed by >>)

>> fs/inode.c:1597:36: sparse: sparse: Using plain integer as NULL pointer
   fs/inode.c:721:24: sparse: sparse: context imbalance in 'inode_lru_isolate' - wrong count at exit
   fs/inode.c:810:9: sparse: sparse: context imbalance in 'find_inode' - different lock contexts for basic block
   fs/inode.c:841:9: sparse: sparse: context imbalance in 'find_inode_fast' - different lock contexts for basic block
   fs/inode.c:1447:5: sparse: sparse: context imbalance in 'insert_inode_locked' - wrong count at exit
   include/linux/spinlock.h:378:9: sparse: sparse: context imbalance in 'iput_final' - unexpected unlock
   fs/inode.c:1572:6: sparse: sparse: context imbalance in 'iput' - wrong count at exit
   fs/inode.c:2024:13: sparse: sparse: context imbalance in '__wait_on_freeing_inode' - unexpected unlock

vim +1597 fs/inode.c

  1590	
  1591	static int fiemap_fill_kernel_extent(struct fiemap_extent_info *fieinfo,
  1592				u64 logical, u64 phys, u64 len, u32 flags)
  1593	{
  1594		struct fiemap_extent *extent = fieinfo->fi_cb_data;
  1595	
  1596		/* only count the extents */
> 1597		if (fieinfo->fi_cb_data == 0) {
  1598			fieinfo->fi_extents_mapped++;
  1599			goto out;
  1600		}
  1601	
  1602		if (fieinfo->fi_extents_mapped >= fieinfo->fi_extents_max)
  1603			return 1;
  1604	
  1605		if (flags & FIEMAP_EXTENT_DELALLOC)
  1606			flags |= FIEMAP_EXTENT_UNKNOWN;
  1607		if (flags & FIEMAP_EXTENT_DATA_ENCRYPTED)
  1608			flags |= FIEMAP_EXTENT_ENCODED;
  1609		if (flags & (FIEMAP_EXTENT_DATA_TAIL | FIEMAP_EXTENT_DATA_INLINE))
  1610			flags |= FIEMAP_EXTENT_NOT_ALIGNED;
  1611	
  1612		extent->fe_logical = logical;
  1613		extent->fe_physical = phys;
  1614		extent->fe_length = len;
  1615		extent->fe_flags = flags;
  1616	
  1617		fieinfo->fi_extents_mapped++;
  1618	
  1619		if (fieinfo->fi_extents_mapped == fieinfo->fi_extents_max)
  1620			return 1;
  1621	
  1622	out:
  1623		if (flags & FIEMAP_EXTENT_LAST)
  1624			return 1;
  1625		return 0;
  1626	}
  1627	

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

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

[Carlos Maiolino]

Hey folks,

> All errors (new ones prefixed by >>):
> 
>    fs/ioctl.c: In function 'ioctl_fibmap':
> >> fs/ioctl.c:68:10: error: implicit declaration of function 'bmap'; did you mean 'kmap'? [-Werror=implicit-function-declaration]

Any of you guys may have a better idea on how to fix this?

Essentially, this happens when CONFIG_BLOCK is not set, and although I don't
really see a hard requirement to have bmap() exported only when CONFIG_BLOCK is
set, at the same time, I don't see use for bmap() if CONFIG_BLOCK is not set.

So, I'm in a kind of a chicken-egg problem.

I am considering to just remove the #ifdef CONFIG_BLOCK / #endif from the bmap()
declaration. This will fix the warning, and I don't see any side effects. What
you guys think?


>      error = bmap(inode, &block);
>              ^~~~
>              kmap
>    cc1: some warnings being treated as errors
> 
> vim +68 fs/ioctl.c
> 
>     53	
>     54	static int ioctl_fibmap(struct file *filp, int __user *p)
>     55	{
>     56		struct inode *inode = file_inode(filp);
>     57		int error, ur_block;
>     58		sector_t block;
>     59	
>     60		if (!capable(CAP_SYS_RAWIO))
>     61			return -EPERM;
>     62	
>     63		error = get_user(ur_block, p);
>     64		if (error)
>     65			return error;
>     66	
>     67		block = ur_block;
>   > 68		error = bmap(inode, &block);
>     69	
>     70		if (error)
>     71			ur_block = 0;
>     72		else
>     73			ur_block = block;
>     74	
>     75		error = put_user(ur_block, p);
>     76	
>     77		return error;
>     78	}
>     79	
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation



-- 
Carlos

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

[Christoph Hellwig]

On Wed, Aug 14, 2019 at 01:01:50PM +0200, Carlos Maiolino wrote:
> Hey folks,
> 
> > All errors (new ones prefixed by >>):
> > 
> >    fs/ioctl.c: In function 'ioctl_fibmap':
> > >> fs/ioctl.c:68:10: error: implicit declaration of function 'bmap'; did you mean 'kmap'? [-Werror=implicit-function-declaration]
> 
> Any of you guys may have a better idea on how to fix this?
> 
> Essentially, this happens when CONFIG_BLOCK is not set, and although I don't
> really see a hard requirement to have bmap() exported only when CONFIG_BLOCK is
> set, at the same time, I don't see use for bmap() if CONFIG_BLOCK is not set.
> 
> So, I'm in a kind of a chicken-egg problem.
> 
> I am considering to just remove the #ifdef CONFIG_BLOCK / #endif from the bmap()
> declaration. This will fix the warning, and I don't see any side effects. What
> you guys think?

Just provide an inline !CONFIG_BLOCK stub for bmap() in fs.h that always
returns -EINVAL.

Re: [PATCH 1/9] fs: Enable bmap() function to properly return errors

[Christoph Hellwig]

Just curious from looking this again - shoudn't the 0 block be turned
into an error by the bmap() function?  At least for the legacy ->bmap
case so that we don't have to carry this cruft around.

Re: [PATCH 2/9] cachefiles: drop direct usage of ->bmap method.

[Christoph Hellwig]

On Thu, Aug 08, 2019 at 10:27:37AM +0200, Carlos Maiolino wrote:
> +	block = page->index;
> +	block <<= shift;

Can't this cause overflows?

> +
> +	ret = bmap(inode, &block);
> +	ASSERT(!ret);

I think we want some real error handling here instead of just an
assert..

Re: [PATCH 7/9] fiemap: Use a callback to fill fiemap extents

[Christoph Hellwig]

Looks good,

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

Re: [PATCH 8/9] Use FIEMAP for FIBMAP calls

[Christoph Hellwig]

The whole FIEMAP_KERNEL_FIBMAP thing looks very counter productive.
bmap() should be able to make the right decision based on the passed
in flags, no need to have a fake FIEMAP flag for that.

Re: [PATCH 1/9] fs: Enable bmap() function to properly return errors

[Carlos Maiolino]

On Wed, Aug 14, 2019 at 01:14:35PM +0200, Christoph Hellwig wrote:
> Just curious from looking this again - shoudn't the 0 block be turned
> into an error by the bmap() function?  At least for the legacy ->bmap
> case so that we don't have to carry this cruft around.

I don't think so, this patch does not change the bmap interface on the
filesystems itself, and at this point, we don't have a way to distinguish
between a hole and an error from the filesystem's bmap() interface. The main
idea of enabling bmap() to return errors, was to take advantage of the fiemap
errors (in another patch in this series).

Although, I do agree that this also opens the possibility to change the
interface itself, and make ->bmap calls to really return errors, but this will
require a bigger change in all users of ->bmap(). I can work on that, but I'd
prefer to change this interface in another patchset, after this one is merged.
So, we can work only on those filesystems where ->bmap() will still be a thing.

Cheers.

-- 
Carlos

Re: [PATCH 2/9] cachefiles: drop direct usage of ->bmap method.

[Carlos Maiolino]

On Wed, Aug 14, 2019 at 01:15:35PM +0200, Christoph Hellwig wrote:
> On Thu, Aug 08, 2019 at 10:27:37AM +0200, Carlos Maiolino wrote:
> > +	block = page->index;
> > +	block <<= shift;
> 
> Can't this cause overflows?

Hmm, I honestly don't know. I did look at the code, and I couldn't really spot
anything concrete.

Maybe if the block size is much smaller than PAGE_SIZE, but I am really not
sure.

Bear in mind though, I didn't change the logic here at all. I just reused one
variable instead of juggling both (block0 and block) old variables. So, if this
really can overflow, the code is already buggy even without my patch, I'm CC'ing
dhowells just in case.


> 
> > +
> > +	ret = bmap(inode, &block);
> > +	ASSERT(!ret);
> 
> I think we want some real error handling here instead of just an
> assert..

I left this ASSERT() here, to match the current logic. By now, the only error we
can get is -EINVAL, which basically says ->bmap() method does not exist, which
is basically what does happen today with:

ASSERT(inode->i_mapping->a_ops->bmap);


But I do agree, it will be better to provide some sort of error handling here,
maybe I should do something like:

ASSERT(ret == -EINVAL)

to keep the logic exactly the same and do not blow up in the future if/when we
expand possible error values from bmap()

What you think?

-- 
Carlos

Re: [PATCH 2/9] cachefiles: drop direct usage of ->bmap method.

[David Howells]

Carlos Maiolino <cmaiolino@redhat.com> wrote:

> > > +	block = page->index;
> > > +	block <<= shift;
> > 
> > Can't this cause overflows?
> 
> Hmm, I honestly don't know. I did look at the code, and I couldn't really spot
> anything concrete.

Maybe, though we'd have to support file sizes over 16 Exabytes for that to be
a problem.

Note that bmap() is *only* used to find out if the page is present in the
cache - and even that I'm not actually doing very well, since I really *ought*
to check every block in the page.

I really want to replace the use of bmap entirely with iov_iter doing DIO.
Cachefiles currently does double buffering because it works through the
pagecache of the underlying to do actual read or write - and this appears to
cause memory management problems.

David

Re: [PATCH 8/9] Use FIEMAP for FIBMAP calls

[Carlos Maiolino]

On Wed, Aug 14, 2019 at 01:18:37PM +0200, Christoph Hellwig wrote:
> The whole FIEMAP_KERNEL_FIBMAP thing looks very counter productive.
> bmap() should be able to make the right decision based on the passed
> in flags, no need to have a fake FIEMAP flag for that.

Using the FIEMAP_KERNEL_FIBMAP flag, is a way to tell filesystems from where the
request came from, so filesystems can handle it differently. For example, we
can't allow in XFS a FIBMAP request on a COW/RTIME inode, and we use the FIBMAP
flag in such situations.

We could maybe check for the callback in fieinfo->fi_cb instead of using the
flag, but I don't see how much more productive this could be.

Maybe a helper, something like

#define is_fibmap(f)	((f->fi_cb) == fiemap_fill_kernel_extent)


But again, I don't know how much better this is comparing with a flag

Cheers
-- 
Carlos

Re: [PATCH 2/9] cachefiles: drop direct usage of ->bmap method.

[David Howells]

Christoph Hellwig <hch@lst.de> wrote:

> > +	block = page->index;
> > +	block <<= shift;
> 
> Can't this cause overflows?

No, not unless the netfs allows files >16EiB in size and as long as block
(type sector_t) is a 64-bit integer.

A 16EiB-1 (0xffffffffffffffff) file would have 4P-1 (0xfffffffffffff) pages
assuming a 4K page size.

At a block size of 1 (and a shift therefore of 12), the maximum block number
calculated would be 0xfffffffffffff000.

David

Re: [PATCH 2/9] cachefiles: drop direct usage of ->bmap method.

[Christoph Hellwig]

On Tue, Aug 20, 2019 at 01:50:30PM +0100, David Howells wrote:
> Carlos Maiolino <cmaiolino@redhat.com> wrote:
> 
> > > > +	block = page->index;
> > > > +	block <<= shift;
> > > 
> > > Can't this cause overflows?
> > 
> > Hmm, I honestly don't know. I did look at the code, and I couldn't really spot
> > anything concrete.
> 
> Maybe, though we'd have to support file sizes over 16 Exabytes for that to be
> a problem.

On 32-bit sysems page->index is a 32-bit value, so you'd overflow at
pretty normal sizes of a few TB.

> Note that bmap() is *only* used to find out if the page is present in the
> cache - and even that I'm not actually doing very well, since I really *ought*
> to check every block in the page.
> 
> I really want to replace the use of bmap entirely with iov_iter doing DIO.
> Cachefiles currently does double buffering because it works through the
> pagecache of the underlying to do actual read or write - and this appears to
> cause memory management problems.

Not related to this patch, but using iov_iter with dio is trivial, what
is the blocker therere?

Re: [PATCH 8/9] Use FIEMAP for FIBMAP calls

[Christoph Hellwig]

On Tue, Aug 20, 2019 at 03:01:18PM +0200, Carlos Maiolino wrote:
> On Wed, Aug 14, 2019 at 01:18:37PM +0200, Christoph Hellwig wrote:
> > The whole FIEMAP_KERNEL_FIBMAP thing looks very counter productive.
> > bmap() should be able to make the right decision based on the passed
> > in flags, no need to have a fake FIEMAP flag for that.
> 
> Using the FIEMAP_KERNEL_FIBMAP flag, is a way to tell filesystems from where the
> request came from, so filesystems can handle it differently. For example, we
> can't allow in XFS a FIBMAP request on a COW/RTIME inode, and we use the FIBMAP
> flag in such situations.

But the whole point is that the file system should not have to know
this.  It is not the file systems business in any way to now where the
call came from.  The file system just needs to provide enough information
so that the caller can make informed decisions.

And in this case that means if any of FIEMAP_EXTENT_DELALLOC,
FIEMAP_EXTENT_ENCODED, FIEMAP_EXTENT_DATA_ENCRYPTED,
FIEMAP_EXTENT_NOT_ALIGNED, FIEMAP_EXTENT_DATA_INLINE,
FIEMAP_EXTENT_DATA_TAIL, FIEMAP_EXTENT_UNWRITTEN or
FIEMAP_EXTENT_SHARED is present the caller should fail the
bmap request.

Re: [PATCH 2/9] cachefiles: drop direct usage of ->bmap method.

[David Howells]

Christoph Hellwig <hch@lst.de> wrote:

> Not related to this patch, but using iov_iter with dio is trivial, what
> is the blocker therere?

The usual: time.

The change as a whole is not actually trivial since it will involve completely
overhauling the fscache data API and how the filesystems use it - and then
having cachefiles perform the DIO asynchronously as per Trond's requirements
for using fscache with NFS.

I also need to work out how I'm going to do data/hole detection.  Can I set,
say, O_NOREADHOLE and then expect the DIO to stop early with a short read?  Or
do I need to use SEEK_DATA/SEEK_HOLE in advance to define the occupied
regions?

Maybe a better way would be to take a leaf out of the book of OpenAFS and
suchlike and keep a parallel file that tracks the occupancy of a cache object
(eg. a bitmap with 1 bit per 64k block) - but that the synchronisation and
performance issues.

David

Re: [PATCH 2/9] cachefiles: drop direct usage of ->bmap method.

[Christoph Hellwig]

On Fri, Aug 30, 2019 at 05:17:51PM +0100, David Howells wrote:
> Christoph Hellwig <hch@lst.de> wrote:
> 
> > Not related to this patch, but using iov_iter with dio is trivial, what
> > is the blocker therere?
> 
> The usual: time.
> 
> The change as a whole is not actually trivial since it will involve completely
> overhauling the fscache data API and how the filesystems use it - and then
> having cachefiles perform the DIO asynchronously as per Trond's requirements
> for using fscache with NFS.

Well, doing in-kernel async I/O is actually rather trivial these days.
Take a look at the loop driver for an example.

> I also need to work out how I'm going to do data/hole detection.  Can I set,
> say, O_NOREADHOLE and then expect the DIO to stop early with a short read?  Or
> do I need to use SEEK_DATA/SEEK_HOLE in advance to define the occupied
> regions?

We'll you'd need to implement a IOCB_NOHOLE, but that wouldn't be all
that hard.  Having a READ_PLUS like interface that actually tells you
how large the hole is might be hard.

Re: [PATCH 2/9] cachefiles: drop direct usage of ->bmap method.

[David Howells]

Christoph Hellwig <hch@lst.de> wrote:

> We'll you'd need to implement a IOCB_NOHOLE, but that wouldn't be all
> that hard.  Having a READ_PLUS like interface that actually tells you
> how large the hole is might be hard.

Actually, that raises another couple of questions:

 (1) Is a filesystem allowed to join up two disjoint blocks of data with a
     block of zeros to make a single extent?  If this happens, I'll see the
     inserted block of zeros to be valid data.

 (2) Is a filesystem allowed to punch out a block of valid zero data to make a
     hole?  This would cause me to refetch the data.

David

Re: [PATCH 2/9] cachefiles: drop direct usage of ->bmap method.

[Dave Chinner]

On Sat, Aug 31, 2019 at 01:45:57AM +0100, David Howells wrote:
> Christoph Hellwig <hch@lst.de> wrote:
> 
> > We'll you'd need to implement a IOCB_NOHOLE, but that wouldn't be all
> > that hard.  Having a READ_PLUS like interface that actually tells you
> > how large the hole is might be hard.
> 
> Actually, that raises another couple of questions:
> 
>  (1) Is a filesystem allowed to join up two disjoint blocks of data with a
>      block of zeros to make a single extent?  If this happens, I'll see the
>      inserted block of zeros to be valid data.

Yes.

>  (2) Is a filesystem allowed to punch out a block of valid zero data to make a
>      hole?  This would cause me to refetch the data.

Yes.

Essentially, assumptions that filesystems will not change the file
layout even if the data does not change are invalid. copy-on-write,
deduplication, speculative preallocation, etc mean the file layout
can change even if the file data itself is not directly modified.

If you want to cache physical file layout information and be
notified when they may change, then that's what layout leases are
for....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 8/9] Use FIEMAP for FIBMAP calls

[Carlos Maiolino]

Hey, thanks for the info.

Although..

On Thu, Aug 29, 2019 at 09:15:55AM +0200, Christoph Hellwig wrote:
> On Tue, Aug 20, 2019 at 03:01:18PM +0200, Carlos Maiolino wrote:
> > On Wed, Aug 14, 2019 at 01:18:37PM +0200, Christoph Hellwig wrote:
> > > The whole FIEMAP_KERNEL_FIBMAP thing looks very counter productive.
> > > bmap() should be able to make the right decision based on the passed
> > > in flags, no need to have a fake FIEMAP flag for that.
> > 
> > Using the FIEMAP_KERNEL_FIBMAP flag, is a way to tell filesystems from where the
> > request came from, so filesystems can handle it differently. For example, we
> > can't allow in XFS a FIBMAP request on a COW/RTIME inode, and we use the FIBMAP
> > flag in such situations.
> 
> But the whole point is that the file system should not have to know
> this.  It is not the file systems business in any way to now where the
> call came from.  The file system just needs to provide enough information
> so that the caller can make informed decisions.
> 
> And in this case that means if any of FIEMAP_EXTENT_DELALLOC,
> FIEMAP_EXTENT_ENCODED, FIEMAP_EXTENT_DATA_ENCRYPTED,
> FIEMAP_EXTENT_NOT_ALIGNED, FIEMAP_EXTENT_DATA_INLINE,
> FIEMAP_EXTENT_DATA_TAIL, FIEMAP_EXTENT_UNWRITTEN or
> FIEMAP_EXTENT_SHARED is present the caller should fail the
> bmap request.

This seems doable, yes, but... Doing that essentially will make some
filesystems, like BTRFS, to suddenly start to support fibmap, this was another
reason why we opted in the first place to let filesystems know whom the caller
was.

We could maybe add a new FIEMAP_EXTENT_* flag in the future to, let's say,
specify a specific block may be split between more than one device, but, well.
It's an idea, but it won't change the fact BTRFS for example will suddenly start
to support FIBMAP.

-- 
Carlos

Re: [PATCH 8/9] Use FIEMAP for FIBMAP calls

[Darrick J. Wong]

On Tue, Sep 10, 2019 at 02:28:35PM +0200, Carlos Maiolino wrote:
> Hey, thanks for the info.
> 
> Although..
> 
> On Thu, Aug 29, 2019 at 09:15:55AM +0200, Christoph Hellwig wrote:
> > On Tue, Aug 20, 2019 at 03:01:18PM +0200, Carlos Maiolino wrote:
> > > On Wed, Aug 14, 2019 at 01:18:37PM +0200, Christoph Hellwig wrote:
> > > > The whole FIEMAP_KERNEL_FIBMAP thing looks very counter productive.
> > > > bmap() should be able to make the right decision based on the passed
> > > > in flags, no need to have a fake FIEMAP flag for that.
> > > 
> > > Using the FIEMAP_KERNEL_FIBMAP flag, is a way to tell filesystems from where the
> > > request came from, so filesystems can handle it differently. For example, we
> > > can't allow in XFS a FIBMAP request on a COW/RTIME inode, and we use the FIBMAP
> > > flag in such situations.
> > 
> > But the whole point is that the file system should not have to know
> > this.  It is not the file systems business in any way to now where the
> > call came from.  The file system just needs to provide enough information
> > so that the caller can make informed decisions.
> > 
> > And in this case that means if any of FIEMAP_EXTENT_DELALLOC,
> > FIEMAP_EXTENT_ENCODED, FIEMAP_EXTENT_DATA_ENCRYPTED,
> > FIEMAP_EXTENT_NOT_ALIGNED, FIEMAP_EXTENT_DATA_INLINE,
> > FIEMAP_EXTENT_DATA_TAIL, FIEMAP_EXTENT_UNWRITTEN or
> > FIEMAP_EXTENT_SHARED is present the caller should fail the
> > bmap request.
> 
> This seems doable, yes, but... Doing that essentially will make some
> filesystems, like BTRFS, to suddenly start to support fibmap, this was another
> reason why we opted in the first place to let filesystems know whom the caller
> was.
> 
> We could maybe add a new FIEMAP_EXTENT_* flag in the future to, let's say,
> specify a specific block may be split between more than one device, but, well.
> It's an idea, but it won't change the fact BTRFS for example will suddenly start
> to support FIBMAP.

...or burn another superblock sb_flag on "this fs supports FIBMAP";
have the in-kernel bmap() function bail out if it isn't set; and only
set it for the filesystems that used to supply ->bmap?

--D

> -- 
> Carlos

[PATCH 2/9] cachefiles: drop direct usage of ->bmap method.

[Carlos Maiolino]

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

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

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


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

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

[PATCH 2/9] 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 8a577409d030..1a74aa978056 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) {
@@ -715,7 +716,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 */
@@ -732,7 +732,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
@@ -740,13 +740,14 @@ int cachefiles_read_or_alloc_pages(struct fscache_retrieval *op,
 		 *   good enough for this as it doesn't indicate errors, but
 		 *   it's all we've got for the moment
 		 */
-		block0 = page->index;
-		block0 <<= shift;
+		block = page->index;
+		block <<= shift;
+
+		ret = bmap(inode, &block);
+		ASSERT(!ret);
 
-		block = inode->i_mapping->a_ops->bmap(inode->i_mapping,
-						      block0);
 		_debug("%llx -> %llx",
-		       (unsigned long long) block0,
+		       (unsigned long long) (page->index << shift),
 		       (unsigned long long) block);
 
 		if (block) {
-- 
2.20.1

[PATCH 2/9] 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 8a577409d030..1a74aa978056 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) {
@@ -715,7 +716,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 */
@@ -732,7 +732,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
@@ -740,13 +740,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.2