• Dave Chinner's avatar
    xfs: collapse range is delalloc challenged · d39a2ced
    Dave Chinner authored
    FSX has been detecting data corruption after to collapse range
    calls. The key observation is that the offset of the last extent in
    the file was not being shifted, and hence when the file size was
    adjusted it was truncating away data because the extents handled
    been correctly shifted.
    
    Tracing indicated that before the collapse, the extent list looked
    like:
    
    ....
    ino 0x5788 state  idx 6 offset 26 block 195904 count 10 flag 0
    ino 0x5788 state  idx 7 offset 39 block 195917 count 35 flag 0
    ino 0x5788 state  idx 8 offset 86 block 195964 count 32 flag 0
    
    and after the shift of 2 blocks:
    
    ino 0x5788 state  idx 6 offset 24 block 195904 count 10 flag 0
    ino 0x5788 state  idx 7 offset 37 block 195917 count 35 flag 0
    ino 0x5788 state  idx 8 offset 86 block 195964 count 32 flag 0
    
    Note that the last extent did not change offset. After the changing
    of the file size:
    
    ino 0x5788 state  idx 6 offset 24 block 195904 count 10 flag 0
    ino 0x5788 state  idx 7 offset 37 block 195917 count 35 flag 0
    ino 0x5788 state  idx 8 offset 86 block 195964 count 30 flag 0
    
    You can see that the last extent had it's length truncated,
    indicating that we've lost data.
    
    The reason for this is that the xfs_bmap_shift_extents() loop uses
    XFS_IFORK_NEXTENTS() to determine how many extents are in the inode.
    This, unfortunately, doesn't take into account delayed allocation
    extents - it's a count of physically allocated extents - and hence
    when the file being collapsed has a delalloc extent like this one
    does prior to the range being collapsed:
    
    ....
    ino 0x5788 state  idx 4 offset 11 block 4503599627239429 count 1 flag 0
    ....
    
    it gets the count wrong and terminates the shift loop early.
    
    Fix it by using the in-memory extent array size that includes
    delayed allocation extents to determine the number of extents on the
    inode.
    Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
    Tested-by: default avatarBrian Foster <bfoster@redhat.com>
    Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
    Signed-off-by: default avatarDave Chinner <david@fromorbit.com>
    d39a2ced
xfs_bmap.c 160 KB