linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Carlos Maiolino <cmaiolino@redhat.com>
To: linux-fsdevel@vger.kernel.org
Cc: hch@lst.de, adilger@dilger.ca, darrick.wong@oracle.com,
	linux-xfs@vger.kernel.org
Subject: [PATCH 8/9] Use FIEMAP for FIBMAP calls
Date: Wed, 11 Sep 2019 15:43:14 +0200	[thread overview]
Message-ID: <20190911134315.27380-9-cmaiolino@redhat.com> (raw)
In-Reply-To: <20190911134315.27380-1-cmaiolino@redhat.com>

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

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

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

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

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

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

Changelog:

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

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

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

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

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


  parent reply	other threads:[~2019-09-11 13:43 UTC|newest]

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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190911134315.27380-9-cmaiolino@redhat.com \
    --to=cmaiolino@redhat.com \
    --cc=adilger@dilger.ca \
    --cc=darrick.wong@oracle.com \
    --cc=hch@lst.de \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).