Skip to content

Commit dd0734f

Browse files
fdmananakdave
authored andcommitted
btrfs: fix race between swap file activation and snapshot creation
When creating a snapshot we check if the current number of swap files, in the root, is non-zero, and if it is, we error out and warn that we can not create the snapshot because there are active swap files. However this is racy because when a task started activation of a swap file, another task might have started already snapshot creation and might have seen the counter for the number of swap files as zero. This means that after the swap file is activated we may end up with a snapshot of the same root successfully created, and therefore when the first write to the swap file happens it has to fall back into COW mode, which should never happen for active swap files. Basically what can happen is: 1) Task A starts snapshot creation and enters ioctl.c:create_snapshot(). There it sees that root->nr_swapfiles has a value of 0 so it continues; 2) Task B enters btrfs_swap_activate(). It is not aware that another task started snapshot creation but it did not finish yet. It increments root->nr_swapfiles from 0 to 1; 3) Task B checks that the file meets all requirements to be an active swap file - it has NOCOW set, there are no snapshots for the inode's root at the moment, no file holes, no reflinked extents, etc; 4) Task B returns success and now the file is an active swap file; 5) Task A commits the transaction to create the snapshot and finishes. The swap file's extents are now shared between the original root and the snapshot; 6) A write into an extent of the swap file is attempted - there is a snapshot of the file's root, so we fall back to COW mode and therefore the physical location of the extent changes on disk. So fix this by taking the snapshot lock during swap file activation before locking the extent range, as that is the order in which we lock these during buffered writes. Fixes: ed46ff3 ("Btrfs: support swap files") CC: stable@vger.kernel.org # 5.4+ Reviewed-by: Anand Jain <anand.jain@oracle.com> Reviewed-by: Josef Bacik <josef@toxicpanda.com> Signed-off-by: Filipe Manana <fdmanana@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
1 parent 195a49e commit dd0734f

1 file changed

Lines changed: 19 additions & 2 deletions

File tree

fs/btrfs/inode.c

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10298,7 +10298,8 @@ static int btrfs_swap_activate(struct swap_info_struct *sis, struct file *file,
1029810298
sector_t *span)
1029910299
{
1030010300
struct inode *inode = file_inode(file);
10301-
struct btrfs_fs_info *fs_info = BTRFS_I(inode)->root->fs_info;
10301+
struct btrfs_root *root = BTRFS_I(inode)->root;
10302+
struct btrfs_fs_info *fs_info = root->fs_info;
1030210303
struct extent_io_tree *io_tree = &BTRFS_I(inode)->io_tree;
1030310304
struct extent_state *cached_state = NULL;
1030410305
struct extent_map *em = NULL;
@@ -10349,13 +10350,27 @@ static int btrfs_swap_activate(struct swap_info_struct *sis, struct file *file,
1034910350
"cannot activate swapfile while exclusive operation is running");
1035010351
return -EBUSY;
1035110352
}
10353+
10354+
/*
10355+
* Prevent snapshot creation while we are activating the swap file.
10356+
* We do not want to race with snapshot creation. If snapshot creation
10357+
* already started before we bumped nr_swapfiles from 0 to 1 and
10358+
* completes before the first write into the swap file after it is
10359+
* activated, than that write would fallback to COW.
10360+
*/
10361+
if (!btrfs_drew_try_write_lock(&root->snapshot_lock)) {
10362+
btrfs_exclop_finish(fs_info);
10363+
btrfs_warn(fs_info,
10364+
"cannot activate swapfile because snapshot creation is in progress");
10365+
return -EINVAL;
10366+
}
1035210367
/*
1035310368
* Snapshots can create extents which require COW even if NODATACOW is
1035410369
* set. We use this counter to prevent snapshots. We must increment it
1035510370
* before walking the extents because we don't want a concurrent
1035610371
* snapshot to run after we've already checked the extents.
1035710372
*/
10358-
atomic_inc(&BTRFS_I(inode)->root->nr_swapfiles);
10373+
atomic_inc(&root->nr_swapfiles);
1035910374

1036010375
isize = ALIGN_DOWN(inode->i_size, fs_info->sectorsize);
1036110376

@@ -10501,6 +10516,8 @@ static int btrfs_swap_activate(struct swap_info_struct *sis, struct file *file,
1050110516
if (ret)
1050210517
btrfs_swap_deactivate(file);
1050310518

10519+
btrfs_drew_write_unlock(&root->snapshot_lock);
10520+
1050410521
btrfs_exclop_finish(fs_info);
1050510522

1050610523
if (ret)

0 commit comments

Comments
 (0)