• Filipe Manana's avatar
    btrfs: fix race that causes unnecessary logging of ancestor inodes · 4d6221d7
    Filipe Manana authored
    When logging an inode and we are checking if we need to log ancestors that
    are new, if the previous transaction is still committing we have a time
    window where we can unnecessarily log ancestor inodes that were created in
    the previous transaction.
    
    The race is described by the following steps:
    
    1) We are at transaction 1000;
    
    2) Directory inode X is created, its generation is set to 1000;
    
    3) The commit for transaction 1000 is started by task A;
    
    4) The task committing transaction 1000 sets the transaction state to
       unblocked, writes the dirty extent buffers and the super blocks, then
       unlocks tree_log_mutex;
    
    5) Inode Y, a regular file, is created under directory inode X, this
       results in starting a new transaction with a generation of 1001;
    
    6) The transaction 1000 commit is unpinning extents. At this point
       fs_info->last_trans_committed still has a value of 999;
    
    7) Task B calls fsync on inode Y and gets a handle for transaction 1001;
    
    8) Task B ends up at log_all_new_ancestors() and then because inode Y has
       only one hard link, ends up at log_new_ancestors_fast(). There it reads
       a value of 999 from fs_info->last_trans_committed, and sees that the
       parent inode X has a generation of 1000, so we end up logging inode X:
    
         if (inode->generation > fs_info->last_trans_committed) {
             ret = btrfs_log_inode(trans, root, inode,
                                   LOG_INODE_EXISTS, ctx);
             (...)
    
       which is not necessary since it was created in the past transaction,
       with a generation of 1000, and that transaction has already committed
       its super blocks - it's still unpinning extents so it has not yet
       updated fs_info->last_trans_committed from 999 to 1000.
    
       So this just causes us to spend more time logging and allocating and
       writing more tree blocks for the log tree.
    
    So fix this by comparing an inode's generation with the generation of the
    transaction our transaction handle refers to - if the inode's generation
    matches the generation of the current transaction than we know it is a
    new inode we need to log, otherwise don't log it.
    
    This case is often hit when running dbench for a long enough duration.
    
    This patch belongs to a patch set that is comprised of the following
    patches:
    
      btrfs: fix race causing unnecessary inode logging during link and rename
      btrfs: fix race that results in logging old extents during a fast fsync
      btrfs: fix race that causes unnecessary logging of ancestor inodes
      btrfs: fix race that makes inode logging fallback to transaction commit
      btrfs: fix race leading to unnecessary transaction commit when logging inode
      btrfs: do not block inode logging for so long during transaction commit
    
    Performance results are mentioned in the change log of the last patch.
    Signed-off-by: default avatarFilipe Manana <fdmanana@suse.com>
    Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
    4d6221d7
tree-log.c 173 KB