• Filipe Manana's avatar
    Btrfs: fix data loss after inode eviction, renaming it, and fsync it · d1d832a0
    Filipe Manana authored
    When we log an inode, regardless of logging it completely or only that it
    exists, we always update it as logged (logged_trans and last_log_commit
    fields of the inode are updated). This is generally fine and avoids future
    attempts to log it from having to do repeated work that brings no value.
    However, if we write data to a file, then evict its inode after all the
    dealloc was flushed (and ordered extents completed), rename the file and
    fsync it, we end up not logging the new extents, since the rename may
    result in logging that the inode exists in case the parent directory was
    logged before. The following reproducer shows and explains how this can
      $ mkfs.btrfs -f /dev/sdb
      $ mount /dev/sdb /mnt
      $ mkdir /mnt/dir
      $ touch /mnt/dir/foo
      $ touch /mnt/dir/bar
      # Do a direct IO write instead of a buffered write because with a
      # buffered write we would need to make sure dealloc gets flushed and
      # complete before we do the inode eviction later, and we can not do that
      # from user space with call to things such as sync(2) since that results
      # in a transaction commit as well.
      $ xfs_io -d -c "pwrite -S 0xd3 0 4K" /mnt/dir/bar
      # Keep the directory dir in use while we evict inodes. We want our file
      # bar's inode to be evicted but we don't want our directory's inode to
      # be evicted (if it were evicted too, we would not be able to reproduce
      # the issue since the first fsync below, of file foo, would result in a
      # transaction commit.
      $ ( cd /mnt/dir; while true; do :; done ) &
      $ pid=$!
      # Wait a bit to give time for the background process to chdir.
      $ sleep 0.1
      # Evict all inodes, except the inode for the directory dir because it is
      # currently in use by our background process.
      $ echo 2 > /proc/sys/vm/drop_caches
      # fsync file foo, which ends up persisting information about the parent
      # directory because it is a new inode.
      $ xfs_io -c fsync /mnt/dir/foo
      # Rename bar, this results in logging that this inode exists (inode item,
      # names, xattrs) because the parent directory is in the log.
      $ mv /mnt/dir/bar /mnt/dir/baz
      # Now fsync baz, which ends up doing absolutely nothing because of the
      # rename operation which logged that the inode exists only.
      $ xfs_io -c fsync /mnt/dir/baz
      <power failure>
      $ mount /dev/sdb /mnt
      $ od -t x1 -A d /mnt/dir/baz
        --> Empty file, data we wrote is missing.
    Fix this by not updating last_sub_trans of an inode when we are logging
    only that it exists and the inode was not yet logged since it was loaded
    from disk (full_sync bit set), this is enough to make btrfs_inode_in_log()
    return false for this scenario and make us log the inode. The logged_trans
    of the inode is still always setsince that alone is used to track if names
    need to be deleted as part of unlink operations.
    Fixes: 257c62e1
     ("Btrfs: avoid tree log commit when there are no changes")
    CC: stable@vger.kernel.org # 4.4+
    Signed-off-by: default avatarFilipe Manana <fdmanana@suse.com>
    Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>