On Thu, Jun 07, 2018 at 09:25:04AM -0700, Darrick J. Wong wrote: > On Thu, Jun 07, 2018 at 10:31:01AM +0200, Carlos Maiolino wrote: > > On Wed, Jun 06, 2018 at 07:40:18AM -0700, Darrick J. Wong wrote: > > > On Wed, Jun 06, 2018 at 03:13:39PM +0200, Carlos Maiolino wrote: > > > > Sigh.. > > > > > > > > On Mon, Jun 04, 2018 at 09:43:09AM -0700, Darrick J. Wong wrote: > > > > > On Sat, Jun 02, 2018 at 11:28:53PM -0400, Theodore Y. Ts'o wrote: > > > > > > On Fri, Jun 01, 2018 at 10:01:54AM -0500, Eric Sandeen wrote: > > > > > > > > Ted, is there any restriction why ext4_fiemap isn't using iomap_fiemap()? Or any > > > > > > > > reason why ext4 fiemap always returns the offset from the beginning of the > > > > > > > > extent? Would you oppose to have it updated to return the offset initially > > > > > > > > requested? Or maybe, change ext4_fiemap() to use iomap_fiemap()? > > > > > > > > > > > > ext4_fiemap() predates iomap_fiemap(). In fact, it used to be that > > > > > > all of the file systems had their own fiemap() implementation. > > > > > > > > > > > > > > I read the fiemap documentation, but I didn't get a clear understanding if > > > > > > > > fiemap should be returning the beginning of the extent, the offset initially > > > > > > > > requested, or if it depends on FS implementation. > > > > > > > > > > > > > > I think the fiemap docs[1] explicitly state that ext4's behavior is valid: > > > > > > > > > > > > > > > Extents returned mirror > > > > > > > > those on disk - that is, the logical offset of the 1st returned extent > > > > > > > > may start before fm_start, and the range covered by the last returned > > > > > > > > extent may end after fm_length. > > > > > > > > > > > > Actually, I read, "Extents returned mirror those on disk" as meaning > > > > > > that the ext4 behavior is *mandated* by the docs. It would be > > > > > > interesting to see what XFS did before the iomap_fiemap() conversion. > > > > > > Or it could have been that the docs were inconsistent with what XFS > > > > > > was doing and then when when ext4_fiemap() was implemented, we > > > > > > followed the docs. Some software archeology would be required to know > > > > > > for sure. > > > > > > > > > > IIRC the pre-iomap xfs_vn_fiemap implementation only returned extent > > > > > data for the block range requested. As far as I can tell, the current > > > > > xfs iomap implementation retains that behavior. > > > > > > > > > > The fiemap spec says that "it is valid for an extents [sic] logical > > > > > offset to start before the request or its logical length to extend past > > > > > the request". To my eyes, that means either behavior is acceptable. > > > > > > > > > > > > > Ok, thanks for the input everyone. I believe Eric's idea then is the one which > > > > makes more sense. If both behaviors are valid, to make fiemap() usage for > > > > fibmap, I think I'll need to get the extent returned by the filesystem and look > > > > for the block into the extent. Thanks a lot for the ideas. > > > > > > Just to throw another monkey wrench into the machine, have you > > > considered using iomap_bmap() instead? It's new for 4.18... > > > > No, but thanks, I'll look into it. > > You might also look at converting ext4 to use iomap_swapfile_activate > since that's new too. iomap_bmap won't report blocks for unwritten > extents (because we shouldn't be pointing userspace at stale disk > blocks), which breaks swapfiles with unwritten extents. I believe the biggest problem in converting ext4 to iomap now, is the behavior change in ext4's FIEMAP, which goes back to my main concern, converting ext4 to use iomap infrastructure, will make ext4 start to report the start block as the one first requested, while current behavior will report the first block on the extent. Unless Ted is ok with this change, I don't think we can do it, which will probably affect users of ext4's current behavior. > > --D > > > > > > > > > --D > > > > > > > > > > > > > > > -- > > > > Carlos > > > > -- > > Carlos -- Carlos
