From patchwork Mon Feb 18 13:03:30 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Carlos Maiolino X-Patchwork-Id: 10817953 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 3CA9B13BF for ; Mon, 18 Feb 2019 13:03:59 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 2A0A1283B1 for ; Mon, 18 Feb 2019 13:03:59 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 1E57128786; Mon, 18 Feb 2019 13:03:59 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E6C97283B1 for ; Mon, 18 Feb 2019 13:03:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730713AbfBRND5 (ORCPT ); Mon, 18 Feb 2019 08:03:57 -0500 Received: from mx1.redhat.com ([209.132.183.28]:38654 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730711AbfBRND4 (ORCPT ); Mon, 18 Feb 2019 08:03:56 -0500 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 658F7C04FFFC; Mon, 18 Feb 2019 13:03:56 +0000 (UTC) Received: from hades.usersys.redhat.com (unknown [10.40.205.169]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1BFA06013C; Mon, 18 Feb 2019 13:03:54 +0000 (UTC) From: Carlos Maiolino To: linux-fsdevel@vger.kernel.org Cc: hch@lst.de, darrick.wong@oracle.com, adilger@dilger.ca Subject: [PATCH RFC 8/9] Use FIEMAP for FIBMAP calls Date: Mon, 18 Feb 2019 14:03:30 +0100 Message-Id: <20190218130331.15882-9-cmaiolino@redhat.com> In-Reply-To: <20190218130331.15882-1-cmaiolino@redhat.com> References: <20190218130331.15882-1-cmaiolino@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Mon, 18 Feb 2019 13:03:56 +0000 (UTC) Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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 --- 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;