Re: [PATCH 15/20] fiemap: Start using new callback from fiemap_ctx

[Carlos Maiolino <cmaiolino@redhat.com>]

On Mon, Nov 05, 2018 at 03:14:02PM -0700, Andreas Dilger wrote:
> On Oct 30, 2018, at 7:18 AM, Carlos Maiolino <cmaiolino@redhat.com> wrote:
> > 
> > Now that fiemap methods are updated to work based on the new fiemap_ctx
> > structure, update the code to use the new callback.
> > The current fiemap_fill_next_extent will be used for user calls of
> > ->fiemap methods, as its done today, but passed in as a callback to
> > fiemap_ctx. So, rename it to ifemap_fill_usr_extent().
> 
> Minor typo here s/ifemap/fiemap/
> 
> Couldn't this series have (mostly?) been achieved by just adding the
> callback to fiemap_extent_info and using that everywhere?

Not really, because fiemap_extent_info has (had now) a userspace pointer, which
isn't used in FIBMAP calls in later patches, so the idea of using callbacks
started as a need to use different code workflow, depending on the user of (now
deprecated) fiemap_extent_info

> 
> Cheers, Andreas
> 
> > The 'new' fiemap_fill_next_extent() will now be just a helper to call
> > fiemap_ctx callback.
> > 
> > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> > ---
> > fs/ioctl.c         | 42 +++++++++++++++++++++++++-----------------
> > include/linux/fs.h |  2 +-
> > 2 files changed, 26 insertions(+), 18 deletions(-)
> > 
> > diff --git a/fs/ioctl.c b/fs/ioctl.c
> > index 27f79b29cb07..85a7aec40e3d 100644
> > --- a/fs/ioctl.c
> > +++ b/fs/ioctl.c
> > @@ -68,25 +68,10 @@ static int ioctl_fibmap(struct file *filp, int __user *p)
> > 	return put_user(res, p);
> > }
> > 
> > -/**
> > - * 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_ctx *f_ctx, u64 logical,
> > +int fiemap_fill_usr_extent(struct fiemap_ctx *f_ctx, u64 logical,
> > 			    u64 phys, u64 len, u32 flags)
> > {
> > 	struct fiemap_extent_info *fieinfo = f_ctx->fc_data;
> > @@ -124,8 +109,29 @@ int fiemap_fill_next_extent(struct fiemap_ctx *f_ctx, u64 logical,
> > 		return 1;
> > 	return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0;
> > }
> > -EXPORT_SYMBOL(fiemap_fill_next_extent);
> > 
> > +/**
> > + * 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_ctx *f_ctx, u64 logical,
> > +			    u64 phys, u64 len, u32 flags)
> > +{
> > +	return f_ctx->fc_cb(f_ctx, logical, phys, len, flags);
> > +}
> > +EXPORT_SYMBOL(fiemap_fill_next_extent);
> > /**
> >  * fiemap_check_flags - check validity of requested flags for fiemap
> >  * @fieinfo:	Fiemap context passed into ->fiemap
> > @@ -208,6 +214,8 @@ static int ioctl_fiemap(struct file *filp, unsigned long arg)
> > 	f_ctx.fc_start = fiemap.fm_start;
> > 	f_ctx.fc_len = len;
> > 
> > +	f_ctx.fc_cb = fiemap_fill_usr_extent;
> > +
> > 	if (fiemap.fm_extent_count != 0 &&
> > 	    !access_ok(VERIFY_WRITE, fieinfo.fi_extents_start,
> > 		       fieinfo.fi_extents_max * sizeof(struct fiemap_extent)))
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index 945cfb3e06e4..9a538bc0dac2 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -1704,7 +1704,7 @@ typedef int (*fiemap_fill_cb)(struct fiemap_ctx *f_ctx, u64 logical,
> > struct fiemap_ctx {
> > 	unsigned int fc_flags;	/* Flags as passed from user */
> > 	void *fc_data;
> > -	fiemap_fill_cb fc_cb; /* Unused by now */
> > +	fiemap_fill_cb fc_cb;
> > 	u64 fc_start;
> > 	u64 fc_len;
> > };
> > --
> > 2.17.1
> > 
> 
> 
> Cheers, Andreas
> 
> 
> 
> 
> 



-- 
Carlos