The functions start_transaction() and btrfs_update_delayed_refs_rsv()
are concurrently executed at runtime in the following call contexts:

Thread 1:
  btrfs_sync_file()
    btrfs_start_transaction()
      start_transaction()

Thread 2:
  finish_ordered_fn()
    btrfs_finish_ordered_io()
      insert_reserved_file_extent()
        __btrfs_drop_extents()
          btrfs_free_extent()
            btrfs_add_delayed_data_ref()
              btrfs_update_delayed_refs_rsv()

In start_transaction():
  if (delayed_refs_rsv->full == 0)
  ...
  else if (... && !delayed_refs_rsv->full)

In btrfs_update_delayed_refs_rsv():
  spin_lock(&delayed_rsv->lock);
  delayed_rsv->size += num_bytes;
  delayed_rsv->full = 0;
  spin_unlock(&delayed_rsv->lock);

The values delayed_refs_rsv->full and delayed_rsv->full access the same
memory, and these data races can occur.
These data races were found and actually reproduced by our conccurency
fuzzer.

To fix these races, the spinlock delayed_refs_rsv->lock is used to
protect the access to delayed_refs_rsv->full in start_transaction().

Signed-off-by: Jia-Ju Bai <baijiaju1...@gmail.com>
---
 fs/btrfs/transaction.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index 8cede6eb9843..ca38d7cf665d 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -524,6 +524,7 @@ start_transaction(struct btrfs_root *root, unsigned int 
num_items,
        u64 qgroup_reserved = 0;
        bool reloc_reserved = false;
        int ret;
+       unsigned short full = 0;
 
        /* Send isn't supposed to start transactions. */
        ASSERT(current->journal_info != BTRFS_SEND_TRANS_STUB);
@@ -541,6 +542,10 @@ start_transaction(struct btrfs_root *root, unsigned int 
num_items,
                goto got_it;
        }
 
+       spin_lock(&delayed_refs_rsv->lock);
+       full = delayed_refs_rsv->full;
+       spin_unlock(&delayed_refs_rsv->lock);
+
        /*
         * Do the reservation before we join the transaction so we can do all
         * the appropriate flushing if need be.
@@ -563,7 +568,7 @@ start_transaction(struct btrfs_root *root, unsigned int 
num_items,
                 * refill that amount for whatever is missing in the reserve.
                 */
                num_bytes = btrfs_calc_insert_metadata_size(fs_info, num_items);
-               if (delayed_refs_rsv->full == 0) {
+               if (full == 0) {
                        delayed_refs_bytes = num_bytes;
                        num_bytes <<= 1;
                }
@@ -585,7 +590,7 @@ start_transaction(struct btrfs_root *root, unsigned int 
num_items,
                        num_bytes -= delayed_refs_bytes;
                }
        } else if (num_items == 0 && flush == BTRFS_RESERVE_FLUSH_ALL &&
-                  !delayed_refs_rsv->full) {
+                  !full) {
                /*
                 * Some people call with btrfs_start_transaction(root, 0)
                 * because they can be throttled, but have some other mechanism
-- 
2.17.1

Reply via email to