bcachefs: Kill trans->flags

Recursive transaction commits are occasionally necessary - in
particular, for the upcoming btree write buffer's flush path.

This avoids bugs due to trans->flags being accidentally mutated
mid-commit, which can cause c->writes refcount leaks.

Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
This commit is contained in:
Kent Overstreet 2023-02-09 13:22:12 -05:00
parent 60b5538877
commit 30ca6ece88
5 changed files with 50 additions and 51 deletions

View file

@ -769,6 +769,7 @@ int bch2_btree_key_cache_flush(struct btree_trans *trans,
} }
bool bch2_btree_insert_key_cached(struct btree_trans *trans, bool bch2_btree_insert_key_cached(struct btree_trans *trans,
unsigned flags,
struct btree_path *path, struct btree_path *path,
struct bkey_i *insert) struct bkey_i *insert)
{ {
@ -778,7 +779,7 @@ bool bch2_btree_insert_key_cached(struct btree_trans *trans,
BUG_ON(insert->u64s > ck->u64s); BUG_ON(insert->u64s > ck->u64s);
if (likely(!(trans->flags & BTREE_INSERT_JOURNAL_REPLAY))) { if (likely(!(flags & BTREE_INSERT_JOURNAL_REPLAY))) {
int difference; int difference;
BUG_ON(jset_u64s(insert->u64s) > trans->journal_preres.u64s); BUG_ON(jset_u64s(insert->u64s) > trans->journal_preres.u64s);

View file

@ -29,7 +29,7 @@ bch2_btree_key_cache_find(struct bch_fs *, enum btree_id, struct bpos);
int bch2_btree_path_traverse_cached(struct btree_trans *, struct btree_path *, int bch2_btree_path_traverse_cached(struct btree_trans *, struct btree_path *,
unsigned); unsigned);
bool bch2_btree_insert_key_cached(struct btree_trans *, bool bch2_btree_insert_key_cached(struct btree_trans *, unsigned,
struct btree_path *, struct bkey_i *); struct btree_path *, struct bkey_i *);
int bch2_btree_key_cache_flush(struct btree_trans *, int bch2_btree_key_cache_flush(struct btree_trans *,
enum btree_id, struct bpos); enum btree_id, struct bpos);

View file

@ -458,7 +458,6 @@ struct btree_trans {
struct journal_preres journal_preres; struct journal_preres journal_preres;
u64 *journal_seq; u64 *journal_seq;
struct disk_reservation *disk_res; struct disk_reservation *disk_res;
unsigned flags;
unsigned journal_u64s; unsigned journal_u64s;
unsigned journal_preres_u64s; unsigned journal_preres_u64s;
struct replicas_delta_list *fs_usage_deltas; struct replicas_delta_list *fs_usage_deltas;

View file

@ -80,7 +80,7 @@ int __must_check bch2_trans_update(struct btree_trans *, struct btree_iter *,
void bch2_trans_commit_hook(struct btree_trans *, void bch2_trans_commit_hook(struct btree_trans *,
struct btree_trans_commit_hook *); struct btree_trans_commit_hook *);
int __bch2_trans_commit(struct btree_trans *); int __bch2_trans_commit(struct btree_trans *, unsigned);
int bch2_trans_log_msg(struct btree_trans *, const char *, ...); int bch2_trans_log_msg(struct btree_trans *, const char *, ...);
int bch2_fs_log_msg(struct bch_fs *, const char *, ...); int bch2_fs_log_msg(struct bch_fs *, const char *, ...);
@ -101,9 +101,8 @@ static inline int bch2_trans_commit(struct btree_trans *trans,
{ {
trans->disk_res = disk_res; trans->disk_res = disk_res;
trans->journal_seq = journal_seq; trans->journal_seq = journal_seq;
trans->flags = flags;
return __bch2_trans_commit(trans); return __bch2_trans_commit(trans, flags);
} }
#define commit_do(_trans, _disk_res, _journal_seq, _flags, _do) \ #define commit_do(_trans, _disk_res, _journal_seq, _flags, _do) \

View file

@ -307,7 +307,7 @@ static inline void btree_insert_entry_checks(struct btree_trans *trans,
} }
static noinline int static noinline int
bch2_trans_journal_preres_get_cold(struct btree_trans *trans, unsigned u64s, bch2_trans_journal_preres_get_cold(struct btree_trans *trans, unsigned flags,
unsigned long trace_ip) unsigned long trace_ip)
{ {
struct bch_fs *c = trans->c; struct bch_fs *c = trans->c;
@ -316,7 +316,9 @@ bch2_trans_journal_preres_get_cold(struct btree_trans *trans, unsigned u64s,
bch2_trans_unlock(trans); bch2_trans_unlock(trans);
ret = bch2_journal_preres_get(&c->journal, ret = bch2_journal_preres_get(&c->journal,
&trans->journal_preres, u64s, 0); &trans->journal_preres,
trans->journal_preres_u64s,
(flags & JOURNAL_WATERMARK_MASK));
if (ret) if (ret)
return ret; return ret;
@ -330,12 +332,10 @@ bch2_trans_journal_preres_get_cold(struct btree_trans *trans, unsigned u64s,
} }
static __always_inline int bch2_trans_journal_res_get(struct btree_trans *trans, static __always_inline int bch2_trans_journal_res_get(struct btree_trans *trans,
unsigned flags) unsigned flags)
{ {
return bch2_journal_res_get(&trans->c->journal, &trans->journal_res, return bch2_journal_res_get(&trans->c->journal, &trans->journal_res,
trans->journal_u64s, trans->journal_u64s, flags);
flags|
(trans->flags & JOURNAL_WATERMARK_MASK));
} }
#define JSET_ENTRY_LOG_U64s 4 #define JSET_ENTRY_LOG_U64s 4
@ -365,9 +365,8 @@ static inline int btree_key_can_insert(struct btree_trans *trans,
return 0; return 0;
} }
static int btree_key_can_insert_cached(struct btree_trans *trans, static int btree_key_can_insert_cached(struct btree_trans *trans, unsigned flags,
struct btree_path *path, struct btree_path *path, unsigned u64s)
unsigned u64s)
{ {
struct bch_fs *c = trans->c; struct bch_fs *c = trans->c;
struct bkey_cached *ck = (void *) path->l[0].b; struct bkey_cached *ck = (void *) path->l[0].b;
@ -379,7 +378,7 @@ static int btree_key_can_insert_cached(struct btree_trans *trans,
if (!test_bit(BKEY_CACHED_DIRTY, &ck->flags) && if (!test_bit(BKEY_CACHED_DIRTY, &ck->flags) &&
bch2_btree_key_cache_must_wait(c) && bch2_btree_key_cache_must_wait(c) &&
!(trans->flags & BTREE_INSERT_JOURNAL_RECLAIM)) !(flags & BTREE_INSERT_JOURNAL_RECLAIM))
return -BCH_ERR_btree_insert_need_journal_reclaim; return -BCH_ERR_btree_insert_need_journal_reclaim;
/* /*
@ -589,7 +588,7 @@ static noinline int bch2_trans_commit_run_gc_triggers(struct btree_trans *trans)
} }
static inline int static inline int
bch2_trans_commit_write_locked(struct btree_trans *trans, bch2_trans_commit_write_locked(struct btree_trans *trans, unsigned flags,
struct btree_insert_entry **stopped_at, struct btree_insert_entry **stopped_at,
unsigned long trace_ip) unsigned long trace_ip)
{ {
@ -629,7 +628,7 @@ bch2_trans_commit_write_locked(struct btree_trans *trans,
u64s += i->k->k.u64s; u64s += i->k->k.u64s;
ret = !i->cached ret = !i->cached
? btree_key_can_insert(trans, insert_l(i)->b, u64s) ? btree_key_can_insert(trans, insert_l(i)->b, u64s)
: btree_key_can_insert_cached(trans, i->path, u64s); : btree_key_can_insert_cached(trans, flags, i->path, u64s);
if (ret) { if (ret) {
*stopped_at = i; *stopped_at = i;
return ret; return ret;
@ -643,8 +642,9 @@ bch2_trans_commit_write_locked(struct btree_trans *trans,
* Don't get journal reservation until after we know insert will * Don't get journal reservation until after we know insert will
* succeed: * succeed:
*/ */
if (likely(!(trans->flags & BTREE_INSERT_JOURNAL_REPLAY))) { if (likely(!(flags & BTREE_INSERT_JOURNAL_REPLAY))) {
ret = bch2_trans_journal_res_get(trans, ret = bch2_trans_journal_res_get(trans,
(flags & JOURNAL_WATERMARK_MASK)|
JOURNAL_RES_GET_NONBLOCK); JOURNAL_RES_GET_NONBLOCK);
if (ret) if (ret)
return ret; return ret;
@ -661,7 +661,7 @@ bch2_trans_commit_write_locked(struct btree_trans *trans,
*/ */
if (IS_ENABLED(CONFIG_BCACHEFS_DEBUG) && if (IS_ENABLED(CONFIG_BCACHEFS_DEBUG) &&
!(trans->flags & BTREE_INSERT_JOURNAL_REPLAY)) { !(flags & BTREE_INSERT_JOURNAL_REPLAY)) {
if (bch2_journal_seq_verify) if (bch2_journal_seq_verify)
trans_for_each_update(trans, i) trans_for_each_update(trans, i)
i->k->k.version.lo = trans->journal_res.seq; i->k->k.version.lo = trans->journal_res.seq;
@ -696,7 +696,7 @@ bch2_trans_commit_write_locked(struct btree_trans *trans,
trans->journal_res.u64s -= trans->extra_journal_entries.nr; trans->journal_res.u64s -= trans->extra_journal_entries.nr;
} }
if (likely(!(trans->flags & BTREE_INSERT_JOURNAL_REPLAY))) { if (likely(!(flags & BTREE_INSERT_JOURNAL_REPLAY))) {
trans_for_each_update(trans, i) { trans_for_each_update(trans, i) {
struct journal *j = &c->journal; struct journal *j = &c->journal;
struct jset_entry *entry; struct jset_entry *entry;
@ -735,7 +735,7 @@ bch2_trans_commit_write_locked(struct btree_trans *trans,
if (!i->cached) if (!i->cached)
btree_insert_key_leaf(trans, i); btree_insert_key_leaf(trans, i);
else if (!i->key_cache_already_flushed) else if (!i->key_cache_already_flushed)
bch2_btree_insert_key_cached(trans, i->path, i->k); bch2_btree_insert_key_cached(trans, flags, i->path, i->k);
else { else {
bch2_btree_key_cache_drop(trans, i->path); bch2_btree_key_cache_drop(trans, i->path);
btree_path_set_dirty(i->path, BTREE_ITER_NEED_TRAVERSE); btree_path_set_dirty(i->path, BTREE_ITER_NEED_TRAVERSE);
@ -784,12 +784,12 @@ static noinline void bch2_drop_overwrites_from_journal(struct btree_trans *trans
} }
#ifdef CONFIG_BCACHEFS_DEBUG #ifdef CONFIG_BCACHEFS_DEBUG
static noinline int bch2_trans_commit_bkey_invalid(struct btree_trans *trans, static noinline int bch2_trans_commit_bkey_invalid(struct btree_trans *trans, unsigned flags,
struct btree_insert_entry *i, struct btree_insert_entry *i,
struct printbuf *err) struct printbuf *err)
{ {
struct bch_fs *c = trans->c; struct bch_fs *c = trans->c;
int rw = (trans->flags & BTREE_INSERT_JOURNAL_REPLAY) ? READ : WRITE; int rw = (flags & BTREE_INSERT_JOURNAL_REPLAY) ? READ : WRITE;
printbuf_reset(err); printbuf_reset(err);
prt_printf(err, "invalid bkey on insert from %s -> %ps", prt_printf(err, "invalid bkey on insert from %s -> %ps",
@ -815,7 +815,7 @@ static noinline int bch2_trans_commit_bkey_invalid(struct btree_trans *trans,
/* /*
* Get journal reservation, take write locks, and attempt to do btree update(s): * Get journal reservation, take write locks, and attempt to do btree update(s):
*/ */
static inline int do_bch2_trans_commit(struct btree_trans *trans, static inline int do_bch2_trans_commit(struct btree_trans *trans, unsigned flags,
struct btree_insert_entry **stopped_at, struct btree_insert_entry **stopped_at,
unsigned long trace_ip) unsigned long trace_ip)
{ {
@ -826,11 +826,11 @@ static inline int do_bch2_trans_commit(struct btree_trans *trans,
#ifdef CONFIG_BCACHEFS_DEBUG #ifdef CONFIG_BCACHEFS_DEBUG
trans_for_each_update(trans, i) { trans_for_each_update(trans, i) {
int rw = (trans->flags & BTREE_INSERT_JOURNAL_REPLAY) ? READ : WRITE; int rw = (flags & BTREE_INSERT_JOURNAL_REPLAY) ? READ : WRITE;
if (unlikely(bch2_bkey_invalid(c, bkey_i_to_s_c(i->k), if (unlikely(bch2_bkey_invalid(c, bkey_i_to_s_c(i->k),
i->bkey_type, rw, &buf))) i->bkey_type, rw, &buf)))
return bch2_trans_commit_bkey_invalid(trans, i, &buf); return bch2_trans_commit_bkey_invalid(trans, flags, i, &buf);
btree_insert_entry_checks(trans, i); btree_insert_entry_checks(trans, i);
} }
#endif #endif
@ -846,7 +846,7 @@ static inline int do_bch2_trans_commit(struct btree_trans *trans,
if (!same_leaf_as_next(trans, i)) { if (!same_leaf_as_next(trans, i)) {
if (u64s_delta <= 0) { if (u64s_delta <= 0) {
ret = bch2_foreground_maybe_merge(trans, i->path, ret = bch2_foreground_maybe_merge(trans, i->path,
i->level, trans->flags); i->level, flags);
if (unlikely(ret)) if (unlikely(ret))
return ret; return ret;
} }
@ -857,11 +857,9 @@ static inline int do_bch2_trans_commit(struct btree_trans *trans,
ret = bch2_journal_preres_get(&c->journal, ret = bch2_journal_preres_get(&c->journal,
&trans->journal_preres, trans->journal_preres_u64s, &trans->journal_preres, trans->journal_preres_u64s,
JOURNAL_RES_GET_NONBLOCK| (flags & JOURNAL_WATERMARK_MASK)|JOURNAL_RES_GET_NONBLOCK);
(trans->flags & JOURNAL_WATERMARK_MASK));
if (unlikely(ret == -BCH_ERR_journal_preres_get_blocked)) if (unlikely(ret == -BCH_ERR_journal_preres_get_blocked))
ret = bch2_trans_journal_preres_get_cold(trans, ret = bch2_trans_journal_preres_get_cold(trans, flags, trace_ip);
trans->journal_preres_u64s, trace_ip);
if (unlikely(ret)) if (unlikely(ret))
return ret; return ret;
@ -869,7 +867,7 @@ static inline int do_bch2_trans_commit(struct btree_trans *trans,
if (unlikely(ret)) if (unlikely(ret))
return ret; return ret;
ret = bch2_trans_commit_write_locked(trans, stopped_at, trace_ip); ret = bch2_trans_commit_write_locked(trans, flags, stopped_at, trace_ip);
if (!ret && unlikely(trans->journal_replay_not_finished)) if (!ret && unlikely(trans->journal_replay_not_finished))
bch2_drop_overwrites_from_journal(trans); bch2_drop_overwrites_from_journal(trans);
@ -908,7 +906,7 @@ static int journal_reclaim_wait_done(struct bch_fs *c)
} }
static noinline static noinline
int bch2_trans_commit_error(struct btree_trans *trans, int bch2_trans_commit_error(struct btree_trans *trans, unsigned flags,
struct btree_insert_entry *i, struct btree_insert_entry *i,
int ret, unsigned long trace_ip) int ret, unsigned long trace_ip)
{ {
@ -916,7 +914,7 @@ int bch2_trans_commit_error(struct btree_trans *trans,
switch (ret) { switch (ret) {
case -BCH_ERR_btree_insert_btree_node_full: case -BCH_ERR_btree_insert_btree_node_full:
ret = bch2_btree_split_leaf(trans, i->path, trans->flags); ret = bch2_btree_split_leaf(trans, i->path, flags);
if (bch2_err_matches(ret, BCH_ERR_transaction_restart)) if (bch2_err_matches(ret, BCH_ERR_transaction_restart))
trace_and_count(c, trans_restart_btree_node_split, trans, trace_ip, i->path); trace_and_count(c, trans_restart_btree_node_split, trans, trace_ip, i->path);
break; break;
@ -934,13 +932,15 @@ int bch2_trans_commit_error(struct btree_trans *trans,
case -BCH_ERR_journal_res_get_blocked: case -BCH_ERR_journal_res_get_blocked:
bch2_trans_unlock(trans); bch2_trans_unlock(trans);
if ((trans->flags & BTREE_INSERT_JOURNAL_RECLAIM) && if ((flags & BTREE_INSERT_JOURNAL_RECLAIM) &&
!(trans->flags & JOURNAL_WATERMARK_reserved)) { !(flags & JOURNAL_WATERMARK_reserved)) {
ret = -BCH_ERR_journal_reclaim_would_deadlock; ret = -BCH_ERR_journal_reclaim_would_deadlock;
break; break;
} }
ret = bch2_trans_journal_res_get(trans, JOURNAL_RES_GET_CHECK); ret = bch2_trans_journal_res_get(trans,
(flags & JOURNAL_WATERMARK_MASK)|
JOURNAL_RES_GET_CHECK);
if (ret) if (ret)
break; break;
@ -970,20 +970,20 @@ int bch2_trans_commit_error(struct btree_trans *trans,
BUG_ON(bch2_err_matches(ret, BCH_ERR_transaction_restart) != !!trans->restarted); BUG_ON(bch2_err_matches(ret, BCH_ERR_transaction_restart) != !!trans->restarted);
bch2_fs_inconsistent_on(bch2_err_matches(ret, ENOSPC) && bch2_fs_inconsistent_on(bch2_err_matches(ret, ENOSPC) &&
!(trans->flags & BTREE_INSERT_NOWAIT) && !(flags & BTREE_INSERT_NOWAIT) &&
(trans->flags & BTREE_INSERT_NOFAIL), c, (flags & BTREE_INSERT_NOFAIL), c,
"%s: incorrectly got %s\n", __func__, bch2_err_str(ret)); "%s: incorrectly got %s\n", __func__, bch2_err_str(ret));
return ret; return ret;
} }
static noinline int static noinline int
bch2_trans_commit_get_rw_cold(struct btree_trans *trans) bch2_trans_commit_get_rw_cold(struct btree_trans *trans, unsigned flags)
{ {
struct bch_fs *c = trans->c; struct bch_fs *c = trans->c;
int ret; int ret;
if (likely(!(trans->flags & BTREE_INSERT_LAZY_RW)) || if (likely(!(flags & BTREE_INSERT_LAZY_RW)) ||
test_bit(BCH_FS_STARTED, &c->flags)) test_bit(BCH_FS_STARTED, &c->flags))
return -BCH_ERR_erofs_trans_commit; return -BCH_ERR_erofs_trans_commit;
@ -1019,7 +1019,7 @@ do_bch2_trans_commit_to_journal_replay(struct btree_trans *trans)
return ret; return ret;
} }
int __bch2_trans_commit(struct btree_trans *trans) int __bch2_trans_commit(struct btree_trans *trans, unsigned flags)
{ {
struct bch_fs *c = trans->c; struct bch_fs *c = trans->c;
struct btree_insert_entry *i = NULL; struct btree_insert_entry *i = NULL;
@ -1030,7 +1030,7 @@ int __bch2_trans_commit(struct btree_trans *trans)
!trans->extra_journal_entries.nr) !trans->extra_journal_entries.nr)
goto out_reset; goto out_reset;
if (trans->flags & BTREE_INSERT_GC_LOCK_HELD) if (flags & BTREE_INSERT_GC_LOCK_HELD)
lockdep_assert_held(&c->gc_lock); lockdep_assert_held(&c->gc_lock);
ret = bch2_trans_commit_run_triggers(trans); ret = bch2_trans_commit_run_triggers(trans);
@ -1042,9 +1042,9 @@ int __bch2_trans_commit(struct btree_trans *trans)
goto out_reset; goto out_reset;
} }
if (!(trans->flags & BTREE_INSERT_NOCHECK_RW) && if (!(flags & BTREE_INSERT_NOCHECK_RW) &&
unlikely(!bch2_write_ref_tryget(c, BCH_WRITE_REF_trans))) { unlikely(!bch2_write_ref_tryget(c, BCH_WRITE_REF_trans))) {
ret = bch2_trans_commit_get_rw_cold(trans); ret = bch2_trans_commit_get_rw_cold(trans, flags);
if (ret) if (ret)
goto out_reset; goto out_reset;
} }
@ -1076,7 +1076,7 @@ int __bch2_trans_commit(struct btree_trans *trans)
/* we're going to journal the key being updated: */ /* we're going to journal the key being updated: */
u64s = jset_u64s(i->k->k.u64s); u64s = jset_u64s(i->k->k.u64s);
if (i->cached && if (i->cached &&
likely(!(trans->flags & BTREE_INSERT_JOURNAL_REPLAY))) likely(!(flags & BTREE_INSERT_JOURNAL_REPLAY)))
trans->journal_preres_u64s += u64s; trans->journal_preres_u64s += u64s;
if (i->flags & BTREE_UPDATE_NOJOURNAL) if (i->flags & BTREE_UPDATE_NOJOURNAL)
@ -1092,7 +1092,7 @@ int __bch2_trans_commit(struct btree_trans *trans)
if (trans->extra_journal_res) { if (trans->extra_journal_res) {
ret = bch2_disk_reservation_add(c, trans->disk_res, ret = bch2_disk_reservation_add(c, trans->disk_res,
trans->extra_journal_res, trans->extra_journal_res,
(trans->flags & BTREE_INSERT_NOFAIL) (flags & BTREE_INSERT_NOFAIL)
? BCH_DISK_RESERVATION_NOFAIL : 0); ? BCH_DISK_RESERVATION_NOFAIL : 0);
if (ret) if (ret)
goto err; goto err;
@ -1101,7 +1101,7 @@ int __bch2_trans_commit(struct btree_trans *trans)
bch2_trans_verify_not_in_restart(trans); bch2_trans_verify_not_in_restart(trans);
memset(&trans->journal_res, 0, sizeof(trans->journal_res)); memset(&trans->journal_res, 0, sizeof(trans->journal_res));
ret = do_bch2_trans_commit(trans, &i, _RET_IP_); ret = do_bch2_trans_commit(trans, flags, &i, _RET_IP_);
/* make sure we didn't drop or screw up locks: */ /* make sure we didn't drop or screw up locks: */
bch2_trans_verify_locks(trans); bch2_trans_verify_locks(trans);
@ -1113,14 +1113,14 @@ int __bch2_trans_commit(struct btree_trans *trans)
out: out:
bch2_journal_preres_put(&c->journal, &trans->journal_preres); bch2_journal_preres_put(&c->journal, &trans->journal_preres);
if (likely(!(trans->flags & BTREE_INSERT_NOCHECK_RW))) if (likely(!(flags & BTREE_INSERT_NOCHECK_RW)))
bch2_write_ref_put(c, BCH_WRITE_REF_trans); bch2_write_ref_put(c, BCH_WRITE_REF_trans);
out_reset: out_reset:
bch2_trans_reset_updates(trans); bch2_trans_reset_updates(trans);
return ret; return ret;
err: err:
ret = bch2_trans_commit_error(trans, i, ret, _RET_IP_); ret = bch2_trans_commit_error(trans, flags, i, ret, _RET_IP_);
if (ret) if (ret)
goto out; goto out;