Skip to content

Commit 37fad94

Browse files
author
Kent Overstreet
committed
bcachefs: snapshot_create_lock
Add a new lock for snapshot creation - this addresses a few races with logged operations and snapshot deletion. Signed-off-by: Kent Overstreet <[email protected]>
1 parent 1e2d399 commit 37fad94

File tree

5 files changed

+44
-7
lines changed

5 files changed

+44
-7
lines changed

fs/bcachefs/bcachefs.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -746,6 +746,7 @@ struct bch_fs {
746746
struct snapshot_table __rcu *snapshots;
747747
size_t snapshot_table_size;
748748
struct mutex snapshot_table_lock;
749+
struct rw_semaphore snapshot_create_lock;
749750

750751
struct work_struct snapshot_delete_work;
751752
struct work_struct snapshot_wait_for_pagecache_and_delete_work;

fs/bcachefs/fs-ioctl.c

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -318,8 +318,8 @@ static int bch2_ioc_goingdown(struct bch_fs *c, u32 __user *arg)
318318
return ret;
319319
}
320320

321-
static long bch2_ioctl_subvolume_create(struct bch_fs *c, struct file *filp,
322-
struct bch_ioctl_subvolume arg)
321+
static long __bch2_ioctl_subvolume_create(struct bch_fs *c, struct file *filp,
322+
struct bch_ioctl_subvolume arg)
323323
{
324324
struct inode *dir;
325325
struct bch_inode_info *inode;
@@ -440,6 +440,16 @@ static long bch2_ioctl_subvolume_create(struct bch_fs *c, struct file *filp,
440440
return error;
441441
}
442442

443+
static long bch2_ioctl_subvolume_create(struct bch_fs *c, struct file *filp,
444+
struct bch_ioctl_subvolume arg)
445+
{
446+
down_write(&c->snapshot_create_lock);
447+
long ret = __bch2_ioctl_subvolume_create(c, filp, arg);
448+
up_write(&c->snapshot_create_lock);
449+
450+
return ret;
451+
}
452+
443453
static long bch2_ioctl_subvolume_destroy(struct bch_fs *c, struct file *filp,
444454
struct bch_ioctl_subvolume arg)
445455
{

fs/bcachefs/io_misc.c

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -287,9 +287,18 @@ int bch2_truncate(struct bch_fs *c, subvol_inum inum, u64 new_i_size, u64 *i_sec
287287
op.v.inum = cpu_to_le64(inum.inum);
288288
op.v.new_i_size = cpu_to_le64(new_i_size);
289289

290-
return bch2_trans_run(c,
290+
/*
291+
* Logged ops aren't atomic w.r.t. snapshot creation: creating a
292+
* snapshot while they're in progress, then crashing, will result in the
293+
* resume only proceeding in one of the snapshots
294+
*/
295+
down_read(&c->snapshot_create_lock);
296+
int ret = bch2_trans_run(c,
291297
bch2_logged_op_start(trans, &op.k_i) ?:
292298
__bch2_resume_logged_op_truncate(trans, &op.k_i, i_sectors_delta));
299+
up_read(&c->snapshot_create_lock);
300+
301+
return ret;
293302
}
294303

295304
/* finsert/fcollapse: */
@@ -491,7 +500,16 @@ int bch2_fcollapse_finsert(struct bch_fs *c, subvol_inum inum,
491500
op.v.src_offset = cpu_to_le64(offset);
492501
op.v.pos = cpu_to_le64(insert ? U64_MAX : offset);
493502

494-
return bch2_trans_run(c,
503+
/*
504+
* Logged ops aren't atomic w.r.t. snapshot creation: creating a
505+
* snapshot while they're in progress, then crashing, will result in the
506+
* resume only proceeding in one of the snapshots
507+
*/
508+
down_read(&c->snapshot_create_lock);
509+
int ret = bch2_trans_run(c,
495510
bch2_logged_op_start(trans, &op.k_i) ?:
496511
__bch2_resume_logged_op_finsert(trans, &op.k_i, i_sectors_delta));
512+
up_read(&c->snapshot_create_lock);
513+
514+
return ret;
497515
}

fs/bcachefs/snapshot.c

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1447,6 +1447,8 @@ int bch2_delete_dead_snapshots(struct bch_fs *c)
14471447
}
14481448
}
14491449

1450+
down_write(&c->snapshot_create_lock);
1451+
14501452
for_each_btree_key(trans, iter, BTREE_ID_snapshots,
14511453
POS_MIN, 0, k, ret) {
14521454
u32 snapshot = k.k->p.offset;
@@ -1457,6 +1459,9 @@ int bch2_delete_dead_snapshots(struct bch_fs *c)
14571459
}
14581460
bch2_trans_iter_exit(trans, &iter);
14591461

1462+
if (ret)
1463+
goto err_create_lock;
1464+
14601465
/*
14611466
* Fixing children of deleted snapshots can't be done completely
14621467
* atomically, if we crash between here and when we delete the interior
@@ -1467,14 +1472,14 @@ int bch2_delete_dead_snapshots(struct bch_fs *c)
14671472
NULL, NULL, BTREE_INSERT_NOFAIL,
14681473
bch2_fix_child_of_deleted_snapshot(trans, &iter, k, &deleted_interior));
14691474
if (ret)
1470-
goto err;
1475+
goto err_create_lock;
14711476

14721477
darray_for_each(deleted, i) {
14731478
ret = commit_do(trans, NULL, NULL, 0,
14741479
bch2_snapshot_node_delete(trans, *i));
14751480
if (ret) {
14761481
bch_err_msg(c, ret, "deleting snapshot %u", *i);
1477-
goto err;
1482+
goto err_create_lock;
14781483
}
14791484
}
14801485

@@ -1483,11 +1488,13 @@ int bch2_delete_dead_snapshots(struct bch_fs *c)
14831488
bch2_snapshot_node_delete(trans, *i));
14841489
if (ret) {
14851490
bch_err_msg(c, ret, "deleting snapshot %u", *i);
1486-
goto err;
1491+
goto err_create_lock;
14871492
}
14881493
}
14891494

14901495
clear_bit(BCH_FS_HAVE_DELETED_SNAPSHOTS, &c->flags);
1496+
err_create_lock:
1497+
up_write(&c->snapshot_create_lock);
14911498
err:
14921499
darray_exit(&deleted_interior);
14931500
darray_exit(&deleted);

fs/bcachefs/super.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -720,6 +720,7 @@ static struct bch_fs *bch2_fs_alloc(struct bch_sb *sb, struct bch_opts opts)
720720

721721
mutex_init(&c->bio_bounce_pages_lock);
722722
mutex_init(&c->snapshot_table_lock);
723+
init_rwsem(&c->snapshot_create_lock);
723724

724725
spin_lock_init(&c->btree_write_error_lock);
725726

0 commit comments

Comments
 (0)