From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-f66.google.com ([209.85.221.66]:40691 "EHLO mail-wr1-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725981AbeIFNF2 (ORCPT ); Thu, 6 Sep 2018 09:05:28 -0400 Received: by mail-wr1-f66.google.com with SMTP id n2-v6so10405095wrw.7 for ; Thu, 06 Sep 2018 01:31:08 -0700 (PDT) Date: Thu, 6 Sep 2018 10:31:05 +0200 From: Carlos Maiolino To: Christoph Hellwig Cc: Matthew Wilcox , linux-fsdevel@vger.kernel.org, sandeen@redhat.com, david@fromorbit.com Subject: Re: [PATCH 2/2] Use fiemap internal infra-structure to handle FIBMAP Message-ID: <20180906083105.tsqbpypdalekpaom@odin.usersys.redhat.com> References: <20180905135748.30098-1-cmaiolino@redhat.com> <20180905135748.30098-3-cmaiolino@redhat.com> <20180905143147.GC3729@bombadil.infradead.org> <20180905185649.GB423@lst.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180905185649.GB423@lst.de> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: 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