Hi Christoph,
On Wed, Sep 05, 2018 at 08:56:49PM +0200, Christoph Hellwig wrote:
> On Wed, Sep 05, 2018 at 07:31:47AM -0700, Matthew Wilcox wrote:
> > On Wed, Sep 05, 2018 at 03:57:48PM +0200, Carlos Maiolino wrote:
> > > + if (inode->i_op->fiemap) {
> > > + fextent.fe_logical = 0;
> > > + fextent.fe_physical = 0;
> > > + fieinfo.fi_flags = FIEMAP_KERNEL_FIBMAP;
> > > + fieinfo.fi_extents_max = 1;
> > > + fieinfo.fi_extents_start = (__force struct fiemap_extent __user *) &fextent;
> > > +
> > > + error = inode->i_op->fiemap(inode, &fieinfo, start, 1);
> >
> > You'd have to play games with set_fs() and friends if you want to do this.
> > The fiemap implementation is going to access fi_extents_start with a call
> > to copy_to_user() and for machines with a 4G/4G split, you need that
> > address to be interpreted as kernel space, not user space.
> >
> > See fiemap_fill_next_extent():
>
> Yeah. I think we need to pass fiemap_fill_next_extent() as a function
> pointer to fiemap in a prep patch, and then pass a different pointer
> for the in-kernel usage. Which is good API design anyway, so we should
> have done this from the beginning.
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?
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
Thanks a lot for the review
--
Carlos
