Skip to content

Commit 72350ee

Browse files
author
Kent Overstreet
committed
bcachefs: Kill snapshot arg to fsck_write_inode()
It was initially believed that it would be better to be explicit about the snapshot we're updating when writing inodes in fsck; however, it turns out that passing around the snapshot separately is more error prone and we're usually updating the inode in the same snapshow we read it from. This is different from normal filesystem paths, where we do the update in the snapshot of the subvolume we're in. Signed-off-by: Kent Overstreet <[email protected]>
1 parent c9306a9 commit 72350ee

File tree

4 files changed

+51
-55
lines changed

4 files changed

+51
-55
lines changed

‎fs/bcachefs/fsck.c

Lines changed: 43 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -137,25 +137,22 @@ static int lookup_first_inode(struct btree_trans *trans, u64 inode_nr,
137137
return ret;
138138
}
139139

140-
static int lookup_inode(struct btree_trans *trans, u64 inode_nr,
141-
struct bch_inode_unpacked *inode,
142-
u32 *snapshot)
140+
static int lookup_inode(struct btree_trans *trans, u64 inode_nr, u32 snapshot,
141+
struct bch_inode_unpacked *inode)
143142
{
144143
struct btree_iter iter;
145144
struct bkey_s_c k;
146145
int ret;
147146

148147
k = bch2_bkey_get_iter(trans, &iter, BTREE_ID_inodes,
149-
SPOS(0, inode_nr, *snapshot), 0);
148+
SPOS(0, inode_nr, snapshot), 0);
150149
ret = bkey_err(k);
151150
if (ret)
152151
goto err;
153152

154153
ret = bkey_is_inode(k.k)
155154
? bch2_inode_unpack(k, inode)
156155
: -BCH_ERR_ENOENT_inode;
157-
if (!ret)
158-
*snapshot = iter.pos.snapshot;
159156
err:
160157
bch2_trans_iter_exit(trans, &iter);
161158
return ret;
@@ -250,8 +247,7 @@ static int lookup_lostfound(struct btree_trans *trans, u32 snapshot,
250247

251248
struct bch_inode_unpacked root_inode;
252249
struct bch_hash_info root_hash_info;
253-
u32 root_inode_snapshot = snapshot;
254-
ret = lookup_inode(trans, root_inum.inum, &root_inode, &root_inode_snapshot);
250+
ret = lookup_inode(trans, root_inum.inum, snapshot, &root_inode);
255251
bch_err_msg(c, ret, "looking up root inode %llu for subvol %u",
256252
root_inum.inum, le32_to_cpu(st.master_subvol));
257253
if (ret)
@@ -277,7 +273,7 @@ static int lookup_lostfound(struct btree_trans *trans, u32 snapshot,
277273
* The bch2_check_dirents pass has already run, dangling dirents
278274
* shouldn't exist here:
279275
*/
280-
ret = lookup_inode(trans, inum, lostfound, &snapshot);
276+
ret = lookup_inode(trans, inum, snapshot, lostfound);
281277
bch_err_msg(c, ret, "looking up lost+found %llu:%u in (root inode %llu, snapshot root %u)",
282278
inum, snapshot, root_inum.inum, bch2_snapshot_root(c, snapshot));
283279
return ret;
@@ -302,6 +298,7 @@ static int lookup_lostfound(struct btree_trans *trans, u32 snapshot,
302298
bch2_inode_init_early(c, lostfound);
303299
bch2_inode_init_late(lostfound, now, 0, 0, S_IFDIR|0700, 0, &root_inode);
304300
lostfound->bi_dir = root_inode.bi_inum;
301+
lostfound->bi_snapshot = le32_to_cpu(st.root_snapshot);
305302

306303
root_inode.bi_nlink++;
307304

@@ -329,17 +326,15 @@ static int lookup_lostfound(struct btree_trans *trans, u32 snapshot,
329326
return ret;
330327
}
331328

332-
static int reattach_inode(struct btree_trans *trans,
333-
struct bch_inode_unpacked *inode,
334-
u32 inode_snapshot)
329+
static int reattach_inode(struct btree_trans *trans, struct bch_inode_unpacked *inode)
335330
{
336331
struct bch_fs *c = trans->c;
337332
struct bch_hash_info dir_hash;
338333
struct bch_inode_unpacked lostfound;
339334
char name_buf[20];
340335
struct qstr name;
341336
u64 dir_offset = 0;
342-
u32 dirent_snapshot = inode_snapshot;
337+
u32 dirent_snapshot = inode->bi_snapshot;
343338
int ret;
344339

345340
if (inode->bi_subvol) {
@@ -363,7 +358,12 @@ static int reattach_inode(struct btree_trans *trans,
363358
lostfound.bi_nlink += S_ISDIR(inode->bi_mode);
364359

365360
/* ensure lost+found inode is also present in inode snapshot */
366-
ret = __bch2_fsck_write_inode(trans, &lostfound, inode_snapshot);
361+
if (!inode->bi_subvol) {
362+
BUG_ON(!bch2_snapshot_is_ancestor(c, inode->bi_snapshot, lostfound.bi_snapshot));
363+
lostfound.bi_snapshot = inode->bi_snapshot;
364+
}
365+
366+
ret = __bch2_fsck_write_inode(trans, &lostfound);
367367
if (ret)
368368
return ret;
369369

@@ -388,7 +388,7 @@ static int reattach_inode(struct btree_trans *trans,
388388
inode->bi_dir = lostfound.bi_inum;
389389
inode->bi_dir_offset = dir_offset;
390390

391-
return __bch2_fsck_write_inode(trans, inode, inode_snapshot);
391+
return __bch2_fsck_write_inode(trans, inode);
392392
}
393393

394394
static int remove_backpointer(struct btree_trans *trans,
@@ -427,7 +427,7 @@ static int reattach_subvol(struct btree_trans *trans, struct bkey_s_c_subvolume
427427
if (ret)
428428
return ret;
429429

430-
ret = reattach_inode(trans, &inode, le32_to_cpu(s.v->snapshot));
430+
ret = reattach_inode(trans, &inode);
431431
bch_err_msg(c, ret, "reattaching inode %llu", inode.bi_inum);
432432
return ret;
433433
}
@@ -545,8 +545,9 @@ static int reconstruct_inode(struct btree_trans *trans, enum btree_id btree, u32
545545
bch2_inode_init_late(&new_inode, bch2_current_time(c), 0, 0, i_mode|0600, 0, NULL);
546546
new_inode.bi_size = i_size;
547547
new_inode.bi_inum = inum;
548+
new_inode.bi_snapshot = snapshot;
548549

549-
return __bch2_fsck_write_inode(trans, &new_inode, snapshot);
550+
return __bch2_fsck_write_inode(trans, &new_inode);
550551
}
551552

552553
struct snapshots_seen {
@@ -1110,7 +1111,7 @@ static int check_inode(struct btree_trans *trans,
11101111

11111112
u.bi_flags &= ~BCH_INODE_i_size_dirty|BCH_INODE_unlinked;
11121113

1113-
ret = __bch2_fsck_write_inode(trans, &u, iter->pos.snapshot);
1114+
ret = __bch2_fsck_write_inode(trans, &u);
11141115

11151116
bch_err_msg(c, ret, "in fsck updating inode");
11161117
if (ret)
@@ -1258,7 +1259,7 @@ static int check_inode(struct btree_trans *trans,
12581259
}
12591260
do_update:
12601261
if (do_update) {
1261-
ret = __bch2_fsck_write_inode(trans, &u, iter->pos.snapshot);
1262+
ret = __bch2_fsck_write_inode(trans, &u);
12621263
bch_err_msg(c, ret, "in fsck updating inode");
12631264
if (ret)
12641265
goto err_noprint;
@@ -1383,7 +1384,7 @@ static int check_i_sectors_notnested(struct btree_trans *trans, struct inode_wal
13831384
w->last_pos.inode, i->snapshot,
13841385
i->inode.bi_sectors, i->count)) {
13851386
i->inode.bi_sectors = i->count;
1386-
ret = bch2_fsck_write_inode(trans, &i->inode, i->snapshot);
1387+
ret = bch2_fsck_write_inode(trans, &i->inode);
13871388
if (ret)
13881389
break;
13891390
}
@@ -1825,7 +1826,7 @@ static int check_subdir_count_notnested(struct btree_trans *trans, struct inode_
18251826
"directory %llu:%u with wrong i_nlink: got %u, should be %llu",
18261827
w->last_pos.inode, i->snapshot, i->inode.bi_nlink, i->count)) {
18271828
i->inode.bi_nlink = i->count;
1828-
ret = bch2_fsck_write_inode(trans, &i->inode, i->snapshot);
1829+
ret = bch2_fsck_write_inode(trans, &i->inode);
18291830
if (ret)
18301831
break;
18311832
}
@@ -1846,8 +1847,7 @@ noinline_for_stack
18461847
static int check_dirent_inode_dirent(struct btree_trans *trans,
18471848
struct btree_iter *iter,
18481849
struct bkey_s_c_dirent d,
1849-
struct bch_inode_unpacked *target,
1850-
u32 target_snapshot)
1850+
struct bch_inode_unpacked *target)
18511851
{
18521852
struct bch_fs *c = trans->c;
18531853
struct printbuf buf = PRINTBUF;
@@ -1880,7 +1880,7 @@ static int check_dirent_inode_dirent(struct btree_trans *trans,
18801880
target->bi_flags &= ~BCH_INODE_unlinked;
18811881
target->bi_dir = d.k->p.inode;
18821882
target->bi_dir_offset = d.k->p.offset;
1883-
return __bch2_fsck_write_inode(trans, target, target_snapshot);
1883+
return __bch2_fsck_write_inode(trans, target);
18841884
}
18851885

18861886
if (bch2_inode_should_have_bp(target) &&
@@ -1893,7 +1893,7 @@ static int check_dirent_inode_dirent(struct btree_trans *trans,
18931893
goto err;
18941894

18951895
struct bkey_s_c_dirent bp_dirent = dirent_get_by_pos(trans, &bp_iter,
1896-
SPOS(target->bi_dir, target->bi_dir_offset, target_snapshot));
1896+
SPOS(target->bi_dir, target->bi_dir_offset, target->bi_snapshot));
18971897
ret = bkey_err(bp_dirent);
18981898
if (ret && !bch2_err_matches(ret, ENOENT))
18991899
goto err;
@@ -1906,14 +1906,14 @@ static int check_dirent_inode_dirent(struct btree_trans *trans,
19061906
"inode %llu:%u has wrong backpointer:\n"
19071907
"got %llu:%llu\n"
19081908
"should be %llu:%llu",
1909-
target->bi_inum, target_snapshot,
1909+
target->bi_inum, target->bi_snapshot,
19101910
target->bi_dir,
19111911
target->bi_dir_offset,
19121912
d.k->p.inode,
19131913
d.k->p.offset)) {
19141914
target->bi_dir = d.k->p.inode;
19151915
target->bi_dir_offset = d.k->p.offset;
1916-
ret = __bch2_fsck_write_inode(trans, target, target_snapshot);
1916+
ret = __bch2_fsck_write_inode(trans, target);
19171917
goto out;
19181918
}
19191919

@@ -1928,7 +1928,7 @@ static int check_dirent_inode_dirent(struct btree_trans *trans,
19281928
trans, inode_dir_multiple_links,
19291929
"%s %llu:%u with multiple links\n%s",
19301930
S_ISDIR(target->bi_mode) ? "directory" : "subvolume",
1931-
target->bi_inum, target_snapshot, buf.buf)) {
1931+
target->bi_inum, target->bi_snapshot, buf.buf)) {
19321932
ret = __remove_dirent(trans, d.k->p);
19331933
goto out;
19341934
}
@@ -1941,10 +1941,10 @@ static int check_dirent_inode_dirent(struct btree_trans *trans,
19411941
if (fsck_err_on(backpointer_exists && !target->bi_nlink,
19421942
trans, inode_multiple_links_but_nlink_0,
19431943
"inode %llu:%u type %s has multiple links but i_nlink 0\n%s",
1944-
target->bi_inum, target_snapshot, bch2_d_types[d.v->d_type], buf.buf)) {
1944+
target->bi_inum, target->bi_snapshot, bch2_d_types[d.v->d_type], buf.buf)) {
19451945
target->bi_nlink++;
19461946
target->bi_flags &= ~BCH_INODE_unlinked;
1947-
ret = __bch2_fsck_write_inode(trans, target, target_snapshot);
1947+
ret = __bch2_fsck_write_inode(trans, target);
19481948
if (ret)
19491949
goto err;
19501950
}
@@ -1961,15 +1961,14 @@ noinline_for_stack
19611961
static int check_dirent_target(struct btree_trans *trans,
19621962
struct btree_iter *iter,
19631963
struct bkey_s_c_dirent d,
1964-
struct bch_inode_unpacked *target,
1965-
u32 target_snapshot)
1964+
struct bch_inode_unpacked *target)
19661965
{
19671966
struct bch_fs *c = trans->c;
19681967
struct bkey_i_dirent *n;
19691968
struct printbuf buf = PRINTBUF;
19701969
int ret = 0;
19711970

1972-
ret = check_dirent_inode_dirent(trans, iter, d, target, target_snapshot);
1971+
ret = check_dirent_inode_dirent(trans, iter, d, target);
19731972
if (ret)
19741973
goto err;
19751974

@@ -2128,7 +2127,7 @@ static int check_dirent_to_subvol(struct btree_trans *trans, struct btree_iter *
21282127
u64 target_inum = le64_to_cpu(s.v->inode);
21292128
u32 target_snapshot = le32_to_cpu(s.v->snapshot);
21302129

2131-
ret = lookup_inode(trans, target_inum, &subvol_root, &target_snapshot);
2130+
ret = lookup_inode(trans, target_inum, target_snapshot, &subvol_root);
21322131
if (ret && !bch2_err_matches(ret, ENOENT))
21332132
goto err;
21342133

@@ -2144,13 +2143,13 @@ static int check_dirent_to_subvol(struct btree_trans *trans, struct btree_iter *
21442143
target_inum,
21452144
subvol_root.bi_parent_subvol, parent_subvol)) {
21462145
subvol_root.bi_parent_subvol = parent_subvol;
2147-
ret = __bch2_fsck_write_inode(trans, &subvol_root, target_snapshot);
2146+
subvol_root.bi_snapshot = le32_to_cpu(s.v->snapshot);
2147+
ret = __bch2_fsck_write_inode(trans, &subvol_root);
21482148
if (ret)
21492149
goto err;
21502150
}
21512151

2152-
ret = check_dirent_target(trans, iter, d, &subvol_root,
2153-
target_snapshot);
2152+
ret = check_dirent_target(trans, iter, d, &subvol_root);
21542153
if (ret)
21552154
goto err;
21562155
out:
@@ -2243,8 +2242,7 @@ static int check_dirent(struct btree_trans *trans, struct btree_iter *iter,
22432242
}
22442243

22452244
darray_for_each(target->inodes, i) {
2246-
ret = check_dirent_target(trans, iter, d,
2247-
&i->inode, i->snapshot);
2245+
ret = check_dirent_target(trans, iter, d, &i->inode);
22482246
if (ret)
22492247
goto err;
22502248
}
@@ -2385,7 +2383,7 @@ static int check_root_trans(struct btree_trans *trans)
23852383
goto err;
23862384
}
23872385

2388-
ret = lookup_inode(trans, BCACHEFS_ROOT_INO, &root_inode, &snapshot);
2386+
ret = lookup_inode(trans, BCACHEFS_ROOT_INO, snapshot, &root_inode);
23892387
if (ret && !bch2_err_matches(ret, ENOENT))
23902388
return ret;
23912389

@@ -2398,8 +2396,9 @@ static int check_root_trans(struct btree_trans *trans)
23982396
bch2_inode_init(c, &root_inode, 0, 0, S_IFDIR|0755,
23992397
0, NULL);
24002398
root_inode.bi_inum = inum;
2399+
root_inode.bi_snapshot = snapshot;
24012400

2402-
ret = __bch2_fsck_write_inode(trans, &root_inode, snapshot);
2401+
ret = __bch2_fsck_write_inode(trans, &root_inode);
24032402
bch_err_msg(c, ret, "writing root inode");
24042403
}
24052404
err:
@@ -2566,7 +2565,7 @@ static int check_path(struct btree_trans *trans, pathbuf *p, struct bkey_s_c ino
25662565
(printbuf_reset(&buf),
25672566
bch2_bkey_val_to_text(&buf, c, inode_k),
25682567
buf.buf)))
2569-
ret = reattach_inode(trans, &inode, snapshot);
2568+
ret = reattach_inode(trans, &inode);
25702569
goto out;
25712570
}
25722571

@@ -2612,7 +2611,7 @@ static int check_path(struct btree_trans *trans, pathbuf *p, struct bkey_s_c ino
26122611
if (ret)
26132612
break;
26142613

2615-
ret = reattach_inode(trans, &inode, snapshot);
2614+
ret = reattach_inode(trans, &inode);
26162615
bch_err_msg(c, ret, "reattaching inode %llu", inode.bi_inum);
26172616
}
26182617
break;
@@ -2842,7 +2841,7 @@ static int check_nlinks_update_inode(struct btree_trans *trans, struct btree_ite
28422841
u.bi_inum, bch2_d_types[mode_to_type(u.bi_mode)],
28432842
bch2_inode_nlink_get(&u), link->count)) {
28442843
bch2_inode_nlink_set(&u, link->count);
2845-
ret = __bch2_fsck_write_inode(trans, &u, k.k->p.snapshot);
2844+
ret = __bch2_fsck_write_inode(trans, &u);
28462845
}
28472846
fsck_err:
28482847
return ret;

‎fs/bcachefs/inode.c

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -387,9 +387,7 @@ int bch2_inode_write_flags(struct btree_trans *trans,
387387
return bch2_trans_update(trans, iter, &inode_p->inode.k_i, flags);
388388
}
389389

390-
int __bch2_fsck_write_inode(struct btree_trans *trans,
391-
struct bch_inode_unpacked *inode,
392-
u32 snapshot)
390+
int __bch2_fsck_write_inode(struct btree_trans *trans, struct bch_inode_unpacked *inode)
393391
{
394392
struct bkey_inode_buf *inode_p =
395393
bch2_trans_kmalloc(trans, sizeof(*inode_p));
@@ -398,19 +396,17 @@ int __bch2_fsck_write_inode(struct btree_trans *trans,
398396
return PTR_ERR(inode_p);
399397

400398
bch2_inode_pack(inode_p, inode);
401-
inode_p->inode.k.p.snapshot = snapshot;
399+
inode_p->inode.k.p.snapshot = inode->bi_snapshot;
402400

403401
return bch2_btree_insert_nonextent(trans, BTREE_ID_inodes,
404402
&inode_p->inode.k_i,
405403
BTREE_UPDATE_internal_snapshot_node);
406404
}
407405

408-
int bch2_fsck_write_inode(struct btree_trans *trans,
409-
struct bch_inode_unpacked *inode,
410-
u32 snapshot)
406+
int bch2_fsck_write_inode(struct btree_trans *trans, struct bch_inode_unpacked *inode)
411407
{
412408
int ret = commit_do(trans, NULL, NULL, BCH_TRANS_COMMIT_no_enospc,
413-
__bch2_fsck_write_inode(trans, inode, snapshot));
409+
__bch2_fsck_write_inode(trans, inode));
414410
bch_err_fn(trans->c, ret);
415411
return ret;
416412
}

‎fs/bcachefs/inode.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,8 @@ static inline int bch2_inode_write(struct btree_trans *trans,
112112
return bch2_inode_write_flags(trans, iter, inode, 0);
113113
}
114114

115-
int __bch2_fsck_write_inode(struct btree_trans *, struct bch_inode_unpacked *, u32);
116-
int bch2_fsck_write_inode(struct btree_trans *, struct bch_inode_unpacked *, u32);
115+
int __bch2_fsck_write_inode(struct btree_trans *, struct bch_inode_unpacked *);
116+
int bch2_fsck_write_inode(struct btree_trans *, struct bch_inode_unpacked *);
117117

118118
void bch2_inode_init_early(struct bch_fs *,
119119
struct bch_inode_unpacked *);

‎fs/bcachefs/subvolume.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,8 @@ static int check_subvol(struct btree_trans *trans,
102102
inode.bi_inum, inode.bi_snapshot,
103103
inode.bi_subvol, subvol.k->p.offset)) {
104104
inode.bi_subvol = subvol.k->p.offset;
105-
ret = __bch2_fsck_write_inode(trans, &inode, le32_to_cpu(subvol.v->snapshot));
105+
inode.bi_snapshot = le32_to_cpu(subvol.v->snapshot);
106+
ret = __bch2_fsck_write_inode(trans, &inode);
106107
if (ret)
107108
goto err;
108109
}

0 commit comments

Comments
 (0)