On 24 Feb 2021 at 02:00, Dave Chinner wrote: > On Tue, Feb 23, 2021 at 01:56:29PM +0530, Chandan Babu R wrote: >> This commit now makes it possible for scrub to check if an inode's extents are >> maximally sized i.e. it checks if an inode's extent is contiguous (in terms of >> both file offset and disk offset) with neighbouring extents and the total >> length of both the extents is less than the maximum allowed extent >> length (i.e. MAXEXTLEN). > > It took me a while to understand that what this is actually doing > (had to read the code because I couldn't work out what this meant). > Essentially, it is determining if two extents that are physically > and logically adjacent were not merged together into a single extent > when the combined size of the two extents would fit into a single > extent record. > > I'm not sure this is an issue - it most definitely isn't corruption > as nothing will have any problems looking up either extent, nor > modifying or removing either extent. It's not ideal, but it isn't > corruption. > > I can see how it would come about, too, because extent removal > doesn't merge remaining partial extents. > > That is, create a long written extent in a file, then use fallocate > to allocate an adjacent extent that puts the two extents over > MAXEXTLEN. Now we have two phsyically and logically adjacent extents > that only differ by state. Now do a single write that converts the > entire unwritten extent to written so no merging occurs during the > state conversion. Now punch out the far end of the second extent. > > This ends up in xfs_bmap_del_extent_real(), which simply removes the > end of the second extent. It does not look up the previous extent > and attempt to merge the remaining part of the second extent into > the previous adjacent extent. > > Hence, at this point, we have two logically and physically adjacent > extents whose combined length is less than MAXLEN. This patch will > now signal that as corruption, which is wrong. Ah ok. Thanks for explaining the scenario which could lead to a false positive report because of the code changes in this patch. -- chandan
