[PATCH RFC 8/9] Use FIEMAP for FIBMAP calls

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

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

Signed-off-by: Carlos Maiolino <cmaiolino@xxxxxxxxxx>
---

I'm flagging this patch as RFC, because here lies the big discussion about how
to prevent filesystems without FIBMAP support, to keep not supporting it once
this patchset is integrated.

Christoph suggested the use of a flag, so, the main difference between this RFC
V3 and the previous one, is the inclusion of the flag FIEMAP_KERNEL_FIBMAP,
which let the underlying filesystem know if the call is coming from which ioctl
the call is coming from (fibmap or fiemap).

Note that I didn't bother to look too deep if some filesystems like ocfs2, f2fs,
etc, (filesystems I don't use to work on) does require extra checks than those I
added to this patch. The goal of this patch is to discuss the generic
implementation of FIEMAP_KERNEL_FIBMAP flag. If this implementation is ok, I'll
work on the filesystems specifically.

Note the bmap call is able to handle errors now, so, invalid fibmap requests
can now return -EINVAL instead of 0.

I added modifications to specific filesystems in this patch, otherwise there
would be a gap between the implementation of the feature (fibmap via fiemap)
where filesystems without FIBMAP support, would suddenly start to support it

The remaining of the patches in this patchset are either a copy from the
previous series (with the respective Reviewed-by tags), and/or a new updated
version containing the changes suggested previously.

Comments are much appreciated.

Cheers

 fs/ext4/extents.c     |  7 +++-
 fs/f2fs/data.c        |  5 ++-
 fs/inode.c            | 80 +++++++++++++++++++++++++++++++++++++++++--
 fs/ioctl.c            | 40 +++++++++++++++-------
 fs/iomap.c            |  2 +-
 fs/ocfs2/extent_map.c |  8 ++++-
 fs/xfs/xfs_iops.c     |  5 +++
 include/linux/fs.h    |  4 +++
 8 files changed, 133 insertions(+), 18 deletions(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index c1fed5b286df..cbfbd9d85648 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -5008,7 +5008,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)
@@ -5055,6 +5057,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 450f2b260c46..28e4f28d3d39 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1405,6 +1405,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;
@@ -1422,7 +1425,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;
 
diff --git a/fs/inode.c b/fs/inode.c
index db681d310465..323dfe3d26fd 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1578,6 +1578,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
@@ -1594,10 +1666,14 @@ 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);
+	else 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 acbb14a89f16..cbd083896518 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -75,11 +75,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;
@@ -87,17 +84,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));
@@ -113,7 +110,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;
 }
 
 /**
@@ -149,13 +150,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;
@@ -206,6 +217,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)
@@ -316,7 +331,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.c b/fs/iomap.c
index df1ea0a21563..f57c50974fa3 100644
--- a/fs/iomap.c
+++ b/fs/iomap.c
@@ -1194,7 +1194,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 e01fd38ea935..2884395f3972 100644
--- a/fs/ocfs2/extent_map.c
+++ b/fs/ocfs2/extent_map.c
@@ -747,7 +747,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)
 {
@@ -756,6 +756,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;
@@ -765,6 +766,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 2b04f69c2c58..7e2d88b4a25e 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -1096,6 +1096,11 @@ xfs_vn_fiemap(
 	struct fiemap_extent_info *fieinfo)
 {
 	int	error;
+	struct	xfs_inode	*ip = XFS_I(inode);
+
+	if (fieinfo->fi_flags & FIEMAP_KERNEL_FIBMAP)
+		if (xfs_is_reflink_inode(ip) || XFS_IS_REALTIME_INODE(ip))
+			return -EINVAL;
 
 	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 bed301c3c1a2..447f992391d4 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1698,6 +1698,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;
-- 
2.17.2




[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux