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

[Andreas Dilger <adilger@dilger.ca>]

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

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

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

Cheers, Andreas

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


Cheers, Andreas






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