On Fri, Nov 16, 2018 at 04:57:21PM +0100, Christoph Hellwig wrote:
> > +int fiemap_fill_usr_extent(struct fiemap_ctx *f_ctx, u64 logical,
>
> Please spell out user.
Sure, will do.
>
> > +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);
>
> Do we really need this wrapper? Which reminds me that we usually
> pass the callbacks as direct function arguments. Any good reason it
> is in the context here?
The reason I left the wrapper is to avoid replacing all
fiemap_fill_next_extent() calls by the callback, may not be the best idea
though.
About using the callback in the context, I thought it would be the easiest way
to pass the callbacks around, without adding a function pointer to many
functions.
The callback is set in ioctl_fiemap() (for user context) or bmap(), if we pass
the callback via function argument, many filesystems would need to juggle around
with the function pointer, once, some of them does not call the
fiemap_fill_next_extent directly from the fiemap method, but into other
sub-functions, for example, in ext4, we would need the function pointer in
ext4_fiemap, and other functions like ext4_fill_fiemap_extents() and
ext4_xattr_fiemap(), etc.
We could even call fiemap_fill_{usr,kernel}_extent() directly from
fiemap_fill_next_extent, and use a flag to differentiate both use cases, but
this will kill the transparent implementation IMHO.
--
Carlos
