Re: [PATCH 2/2] Use fiemap internal infra-structure to handle FIBMAP

[Carlos Maiolino <cmaiolino@redhat.com>]

On Mon, Sep 10, 2018 at 09:50:39AM +0200, Christoph Hellwig wrote:
> On Thu, Sep 06, 2018 at 10:31:05AM +0200, Carlos Maiolino wrote:
> > 
> > Sorry, I'm not 100% sure I followed your point here, do you mind to detail it a
> > bit?
> > Passing fiemap_fill_next_extent() as a pointer to what? ->fiemap() interface?
> 
> Yes.
> 
> > Sounds interesting, but doing this looks like I'll need to do what I was trying
> > to avoid from the beginning, which is the creating of a second struct
> > fiemap_extent_info, to be used in-lernel only, so, the last field doesn't need
> > to be tagged as __user. I'm ok with that though, I was just trying a way to
> > avoid adding unneeded data structures if possible, but looks like it ended up
> > not being a good approach :P
> 
> fieinfo is mostly used as an opaqueue cookie, so I think it should
> be possible to just pass a void pointer to fiemap and only let the 'filler'
> callback (that is fiemap_fill_next_extent or whatever a kernel caller
> passed) interpret it.
> 
> The only thing breaking this right now seems to be fi_flags, so maybe
> we just need to pass that explicitly as another argument to ->fiemap.
> 
> Something like:
> 
> typedef int (fiemap_fill_cb)(struct inode *inode, void *data, u64 logical,
> 		u64 phys, u64 len, u32 flags);
> 
> struct file_operations {
> 	...
> 	int (*fiemap)(struct inode *, unsigned int, fiemap_fill_cb,
> 			void *, u64 start, u64 len);
> 	...

Thanks, this gives me some extra information to keep working on it. And I agree
this is a good way to refactor this infra-structure. I'll work on something
based on this and send it out to the list.

> };

-- 
Carlos