btrfs: fix race between RO remount and the cleaner task

author Filipe Manana <fdmanana@suse.com>

Mon, 14 Dec 2020 10:10:47 +0000 (10:10 +0000)

committer David Sterba <dsterba@suse.com>

Fri, 18 Dec 2020 14:00:02 +0000 (15:00 +0100)
author Filipe Manana <fdmanana@suse.com>
Mon, 14 Dec 2020 10:10:47 +0000 (10:10 +0000)
committer David Sterba <dsterba@suse.com>
Fri, 18 Dec 2020 14:00:02 +0000 (15:00 +0100)
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h

index 3935d29..0225c52 100644 (file)
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -132,6 +132,8 @@ enum {
          * defrag
          */
         BTRFS_FS_STATE_REMOUNTING,
+       /* Filesystem in RO mode */
+       BTRFS_FS_STATE_RO,
         /* Track if a transaction abort has been reported on this filesystem */
         BTRFS_FS_STATE_TRANS_ABORTED,
         /*
@@ -2892,10 +2894,26 @@ static inline int btrfs_fs_closing(struct btrfs_fs_info *fs_info)
   * If we remount the fs to be R/O or umount the fs, the cleaner needn't do
   * anything except sleeping. This function is used to check the status of
   * the fs.
+ * We check for BTRFS_FS_STATE_RO to avoid races with a concurrent remount,
+ * since setting and checking for SB_RDONLY in the superblock's flags is not
+ * atomic.
   */
  static inline int btrfs_need_cleaner_sleep(struct btrfs_fs_info *fs_info)
  {
-       return fs_info->sb->s_flags & SB_RDONLY || btrfs_fs_closing(fs_info);
+       return test_bit(BTRFS_FS_STATE_RO, &fs_info->fs_state) ||
+               btrfs_fs_closing(fs_info);
+}
+
+static inline void btrfs_set_sb_rdonly(struct super_block *sb)
+{
+       sb->s_flags |= SB_RDONLY;
+       set_bit(BTRFS_FS_STATE_RO, &btrfs_sb(sb)->fs_state);
+}
+
+static inline void btrfs_clear_sb_rdonly(struct super_block *sb)
+{
+       sb->s_flags &= ~SB_RDONLY;
+       clear_bit(BTRFS_FS_STATE_RO, &btrfs_sb(sb)->fs_state);
  }
  
  /* tree mod log functions from ctree.c */
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c

index e941cba..e7bcbd0 100644 (file)
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1729,7 +1729,7 @@ static int cleaner_kthread(void *arg)
                  */
                 btrfs_delete_unused_bgs(fs_info);
  sleep:
-               clear_bit(BTRFS_FS_CLEANER_RUNNING, &fs_info->flags);
+               clear_and_wake_up_bit(BTRFS_FS_CLEANER_RUNNING, &fs_info->flags);
                 if (kthread_should_park())
                         kthread_parkme();
                 if (kthread_should_stop())
@@ -2830,6 +2830,9 @@ static int init_mount_fs_info(struct btrfs_fs_info *fs_info, struct super_block
                 return -ENOMEM;
         btrfs_init_delayed_root(fs_info->delayed_root);
  
+       if (sb_rdonly(sb))
+               set_bit(BTRFS_FS_STATE_RO, &fs_info->fs_state);
+
         return btrfs_alloc_stripe_hash_table(fs_info);
  }
  
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c

index b24fa62..38740cc 100644 (file)
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -175,7 +175,7 @@ void __btrfs_handle_fs_error(struct btrfs_fs_info *fs_info, const char *function
         btrfs_discard_stop(fs_info);
  
         /* btrfs handle error by forcing the filesystem readonly */
-       sb->s_flags |= SB_RDONLY;
+       btrfs_set_sb_rdonly(sb);
         btrfs_info(fs_info, "forced readonly");
         /*
          * Note that a running device replace operation is not canceled here
@@ -1953,7 +1953,7 @@ static int btrfs_remount(struct super_block *sb, int *flags, char *data)
                 /* avoid complains from lockdep et al. */
                 up(&fs_info->uuid_tree_rescan_sem);
  
-               sb->s_flags |= SB_RDONLY;
+               btrfs_set_sb_rdonly(sb);
  
                 /*
                  * Setting SB_RDONLY will put the cleaner thread to
@@ -1964,6 +1964,20 @@ static int btrfs_remount(struct super_block *sb, int *flags, char *data)
                  */
                 btrfs_delete_unused_bgs(fs_info);
  
+               /*
+                * The cleaner task could be already running before we set the
+                * flag BTRFS_FS_STATE_RO (and SB_RDONLY in the superblock).
+                * We must make sure that after we finish the remount, i.e. after
+                * we call btrfs_commit_super(), the cleaner can no longer start
+                * a transaction - either because it was dropping a dead root,
+                * running delayed iputs or deleting an unused block group (the
+                * cleaner picked a block group from the list of unused block
+                * groups before we were able to in the previous call to
+                * btrfs_delete_unused_bgs()).
+                */
+               wait_on_bit(&fs_info->flags, BTRFS_FS_CLEANER_RUNNING,
+                           TASK_UNINTERRUPTIBLE);
+
                 btrfs_dev_replace_suspend_for_unmount(fs_info);
                 btrfs_scrub_cancel(fs_info);
                 btrfs_pause_balance(fs_info);
@@ -2014,7 +2028,7 @@ static int btrfs_remount(struct super_block *sb, int *flags, char *data)
                 if (ret)
                         goto restore;
  
-               sb->s_flags &= ~SB_RDONLY;
+               btrfs_clear_sb_rdonly(sb);
  
                 set_bit(BTRFS_FS_OPEN, &fs_info->flags);
         }
@@ -2036,6 +2050,8 @@ restore:
         /* We've hit an error - don't reset SB_RDONLY */
         if (sb_rdonly(sb))
                 old_flags |= SB_RDONLY;
+       if (!(old_flags & SB_RDONLY))
+               clear_bit(BTRFS_FS_STATE_RO, &fs_info->fs_state);
         sb->s_flags = old_flags;
         fs_info->mount_opt = old_opts;
         fs_info->compress_type = old_compress_type;
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c

index 7930e1c..2c0aa03 100644 (file)
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -2593,7 +2593,7 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
         set_blocksize(device->bdev, BTRFS_BDEV_BLOCKSIZE);
  
         if (seeding_dev) {
-               sb->s_flags &= ~SB_RDONLY;
+               btrfs_clear_sb_rdonly(sb);
                 ret = btrfs_prepare_sprout(fs_info);
                 if (ret) {
                         btrfs_abort_transaction(trans, ret);
@@ -2729,7 +2729,7 @@ error_sysfs:
         mutex_unlock(&fs_info->fs_devices->device_list_mutex);
  error_trans:
         if (seeding_dev)
-               sb->s_flags |= SB_RDONLY;
+               btrfs_set_sb_rdonly(sb);
         if (trans)
                 btrfs_end_transaction(trans);
  error_free_zone:
author	Filipe Manana <fdmanana@suse.com>
	Mon, 14 Dec 2020 10:10:47 +0000 (10:10 +0000)
committer	David Sterba <dsterba@suse.com>
	Fri, 18 Dec 2020 14:00:02 +0000 (15:00 +0100)
fs/btrfs/ctree.h		patch \| blob \| history
fs/btrfs/disk-io.c		patch \| blob \| history
fs/btrfs/super.c		patch \| blob \| history
fs/btrfs/volumes.c		patch \| blob \| history