Skip to content

Commit a84d5d4

Browse files
boryaskdave
authored andcommitted
btrfs: detect nocow for swap after snapshot delete
can_nocow_extent and btrfs_cross_ref_exist both rely on a heuristic for detecting a must cow condition which is not exactly accurate, but saves unnecessary tree traversal. The incorrect assumption is that if the extent was created in a generation smaller than the last snapshot generation, it must be referenced by that snapshot. That is true, except the snapshot could have since been deleted, without affecting the last snapshot generation. The original patch claimed a performance win from this check, but it also leads to a bug where you are unable to use a swapfile if you ever snapshotted the subvolume it's in. Make the check slower and more strict for the swapon case, without modifying the general cow checks as a compromise. Turning swap on does not seem to be a particularly performance sensitive operation, so incurring a possibly unnecessary btrfs_search_slot seems worthwhile for the added usability. Note: Until the snapshot is competely cleaned after deletion, check_committed_refs will still cause the logic to think that cow is necessary, so the user must until 'btrfs subvolu sync' finished before activating the swapfile swapon. CC: stable@vger.kernel.org # 5.4+ Suggested-by: Omar Sandoval <osandov@osandov.com> Signed-off-by: Boris Burkov <boris@bur.io> Signed-off-by: David Sterba <dsterba@suse.com>
1 parent fb2fecb commit a84d5d4

4 files changed

Lines changed: 25 additions & 16 deletions

File tree

fs/btrfs/ctree.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2518,7 +2518,7 @@ int btrfs_pin_extent_for_log_replay(struct btrfs_trans_handle *trans,
25182518
u64 bytenr, u64 num_bytes);
25192519
int btrfs_exclude_logged_extents(struct extent_buffer *eb);
25202520
int btrfs_cross_ref_exist(struct btrfs_root *root,
2521-
u64 objectid, u64 offset, u64 bytenr);
2521+
u64 objectid, u64 offset, u64 bytenr, bool strict);
25222522
struct extent_buffer *btrfs_alloc_tree_block(struct btrfs_trans_handle *trans,
25232523
struct btrfs_root *root,
25242524
u64 parent, u64 root_objectid,
@@ -2934,7 +2934,7 @@ struct extent_map *btrfs_get_extent_fiemap(struct btrfs_inode *inode,
29342934
u64 start, u64 len);
29352935
noinline int can_nocow_extent(struct inode *inode, u64 offset, u64 *len,
29362936
u64 *orig_start, u64 *orig_block_len,
2937-
u64 *ram_bytes);
2937+
u64 *ram_bytes, bool strict);
29382938

29392939
void __btrfs_del_delalloc_inode(struct btrfs_root *root,
29402940
struct btrfs_inode *inode);

fs/btrfs/extent-tree.c

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2306,7 +2306,8 @@ static noinline int check_delayed_ref(struct btrfs_root *root,
23062306

23072307
static noinline int check_committed_ref(struct btrfs_root *root,
23082308
struct btrfs_path *path,
2309-
u64 objectid, u64 offset, u64 bytenr)
2309+
u64 objectid, u64 offset, u64 bytenr,
2310+
bool strict)
23102311
{
23112312
struct btrfs_fs_info *fs_info = root->fs_info;
23122313
struct btrfs_root *extent_root = fs_info->extent_root;
@@ -2348,9 +2349,13 @@ static noinline int check_committed_ref(struct btrfs_root *root,
23482349
btrfs_extent_inline_ref_size(BTRFS_EXTENT_DATA_REF_KEY))
23492350
goto out;
23502351

2351-
/* If extent created before last snapshot => it's definitely shared */
2352-
if (btrfs_extent_generation(leaf, ei) <=
2353-
btrfs_root_last_snapshot(&root->root_item))
2352+
/*
2353+
* If extent created before last snapshot => it's shared unless the
2354+
* snapshot has been deleted. Use the heuristic if strict is false.
2355+
*/
2356+
if (!strict &&
2357+
(btrfs_extent_generation(leaf, ei) <=
2358+
btrfs_root_last_snapshot(&root->root_item)))
23542359
goto out;
23552360

23562361
iref = (struct btrfs_extent_inline_ref *)(ei + 1);
@@ -2375,7 +2380,7 @@ static noinline int check_committed_ref(struct btrfs_root *root,
23752380
}
23762381

23772382
int btrfs_cross_ref_exist(struct btrfs_root *root, u64 objectid, u64 offset,
2378-
u64 bytenr)
2383+
u64 bytenr, bool strict)
23792384
{
23802385
struct btrfs_path *path;
23812386
int ret;
@@ -2386,7 +2391,7 @@ int btrfs_cross_ref_exist(struct btrfs_root *root, u64 objectid, u64 offset,
23862391

23872392
do {
23882393
ret = check_committed_ref(root, path, objectid,
2389-
offset, bytenr);
2394+
offset, bytenr, strict);
23902395
if (ret && ret != -ENOENT)
23912396
goto out;
23922397

fs/btrfs/file.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1571,7 +1571,7 @@ static int check_can_nocow(struct btrfs_inode *inode, loff_t pos,
15711571
}
15721572

15731573
ret = can_nocow_extent(&inode->vfs_inode, lockstart, &num_bytes,
1574-
NULL, NULL, NULL);
1574+
NULL, NULL, NULL, false);
15751575
if (ret <= 0) {
15761576
ret = 0;
15771577
if (!nowait)

fs/btrfs/inode.c

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1609,7 +1609,7 @@ static noinline int run_delalloc_nocow(struct btrfs_inode *inode,
16091609
goto out_check;
16101610
ret = btrfs_cross_ref_exist(root, ino,
16111611
found_key.offset -
1612-
extent_offset, disk_bytenr);
1612+
extent_offset, disk_bytenr, false);
16131613
if (ret) {
16141614
/*
16151615
* ret could be -EIO if the above fails to read
@@ -6949,6 +6949,8 @@ static struct extent_map *btrfs_new_extent_direct(struct btrfs_inode *inode,
69496949
* @orig_start: (optional) Return the original file offset of the file extent
69506950
* @orig_len: (optional) Return the original on-disk length of the file extent
69516951
* @ram_bytes: (optional) Return the ram_bytes of the file extent
6952+
* @strict: if true, omit optimizations that might force us into unnecessary
6953+
* cow. e.g., don't trust generation number.
69526954
*
69536955
* This function will flush ordered extents in the range to ensure proper
69546956
* nocow checks for (nowait == false) case.
@@ -6963,7 +6965,7 @@ static struct extent_map *btrfs_new_extent_direct(struct btrfs_inode *inode,
69636965
*/
69646966
noinline int can_nocow_extent(struct inode *inode, u64 offset, u64 *len,
69656967
u64 *orig_start, u64 *orig_block_len,
6966-
u64 *ram_bytes)
6968+
u64 *ram_bytes, bool strict)
69676969
{
69686970
struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
69696971
struct btrfs_path *path;
@@ -7041,8 +7043,9 @@ noinline int can_nocow_extent(struct inode *inode, u64 offset, u64 *len,
70417043
* Do the same check as in btrfs_cross_ref_exist but without the
70427044
* unnecessary search.
70437045
*/
7044-
if (btrfs_file_extent_generation(leaf, fi) <=
7045-
btrfs_root_last_snapshot(&root->root_item))
7046+
if (!strict &&
7047+
(btrfs_file_extent_generation(leaf, fi) <=
7048+
btrfs_root_last_snapshot(&root->root_item)))
70467049
goto out;
70477050

70487051
backref_offset = btrfs_file_extent_offset(leaf, fi);
@@ -7078,7 +7081,8 @@ noinline int can_nocow_extent(struct inode *inode, u64 offset, u64 *len,
70787081
*/
70797082

70807083
ret = btrfs_cross_ref_exist(root, btrfs_ino(BTRFS_I(inode)),
7081-
key.offset - backref_offset, disk_bytenr);
7084+
key.offset - backref_offset, disk_bytenr,
7085+
strict);
70827086
if (ret) {
70837087
ret = 0;
70847088
goto out;
@@ -7299,7 +7303,7 @@ static int btrfs_get_blocks_direct_write(struct extent_map **map,
72997303
block_start = em->block_start + (start - em->start);
73007304

73017305
if (can_nocow_extent(inode, start, &len, &orig_start,
7302-
&orig_block_len, &ram_bytes) == 1 &&
7306+
&orig_block_len, &ram_bytes, false) == 1 &&
73037307
btrfs_inc_nocow_writers(fs_info, block_start)) {
73047308
struct extent_map *em2;
73057309

@@ -10130,7 +10134,7 @@ static int btrfs_swap_activate(struct swap_info_struct *sis, struct file *file,
1013010134
free_extent_map(em);
1013110135
em = NULL;
1013210136

10133-
ret = can_nocow_extent(inode, start, &len, NULL, NULL, NULL);
10137+
ret = can_nocow_extent(inode, start, &len, NULL, NULL, NULL, true);
1013410138
if (ret < 0) {
1013510139
goto out;
1013610140
} else if (ret) {

0 commit comments

Comments
 (0)