On 9/6/18 3:12 AM, Carlos Maiolino wrote: >>> + >>> + /* >>> + * Fiemap implementation of some filesystems will return the >>> + * extent map beginning at the requested offset >>> + * (fextent.fi_logical == start), while other FSes will return >>> + * the beginning of the extent at fextent.fe_logical, even if >>> + * the requested offset is somehwere in the middle of the >>> + * extent. We must be sure to return the correct block block >>> + * here in both cases. >>> + */ >>> + if (fextent.fe_logical != start) >>> + res = (fextent.fe_physical + start) >> inode->i_blkbits; >> I don't think this is correct, is it? You can't add the entire requested >> logical file offset (start) to the resulting physical extent start (fe_physical). >> >> You'd need to add the delta, (start - fextent.fe_logical) to the physical extent >> start to get the offset into the mapping, right? >> >> res = (fextent.fe_physical + >> (start - fextent.fe_logical)) >> inode->i_blkbits; > I believe you're right, yes, when I wrote the calculation above I was thinking > about "fe_logical is either 0 or equal start", and I tested it against > multi-extents files. I really have no idea how it survived my tests and xfstests > =/ Yeay... given the recent undetected FIBMAP breakage thanks to iomap conversion in xfs, it appears that we could use better test coverage of this crappy old interface. ;) -Eric
