On Tue, Aug 27, 2024 at 01:53:55PM GMT, Dan Carpenter wrote:
> On Thu, Jul 19, 2024 at 06:36:50PM -0400, Kent Overstreet wrote:
> > bcachefs: Unlock trans when waiting for user input in fsck
>
> Hello Kent Overstreet,
>
> ommit 889fb3dc5d6f ("bcachefs: Unlock trans when waiting for user
> input in fsck") from May 29, 2024 (linux-next), leads to the
> following (UNPUBLISHED) Smatch static checker warning:
>
> fs/bcachefs/error.c:129 bch2_fsck_ask_yn() error: double unlocked 'trans' (orig line 113)
>
> fs/bcachefs/error.c
> 102 static enum ask_yn bch2_fsck_ask_yn(struct bch_fs *c, struct btree_trans *trans)
> 103 {
> 104 struct stdio_redirect *stdio = c->stdio;
> 105
> 106 if (c->stdio_filter && c->stdio_filter != current)
> 107 stdio = NULL;
> 108
> 109 if (!stdio)
> 110 return YN_NO;
> 111
> 112 if (trans)
> 113 bch2_trans_unlock(trans);
> ^^^^^^^^^^^^^^^^^^^^^^^^^
> Unlock
>
> 114
> 115 unsigned long unlock_long_at = trans ? jiffies + HZ * 2 : 0;
> 116 darray_char line = {};
> 117 int ret;
> 118
> 119 do {
> 120 unsigned long t;
> 121 bch2_print(c, " (y,n, or Y,N for all errors of this type) ");
> 122 rewait:
> 123 t = unlock_long_at
> 124 ? max_t(long, unlock_long_at - jiffies, 0)
> 125 : MAX_SCHEDULE_TIMEOUT;
> 126
> 127 int r = bch2_stdio_redirect_readline_timeout(stdio, &line, t);
> 128 if (r == -ETIME) {
> 129 bch2_trans_unlock_long(trans);
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Double unlock
Those are different types of unlocks.
The first unlock drops btree locks, but we still have pointers and lock
sequence numbers to those nodes so that we can do a bch2_trans_relock()
later, and continue the same transaction.
But we still have an SRCU read lock held which prevents those nodes
from being reclaimed, and we can't hold that for too long either.
So if we're blocked here too long we have to also do an unlock_long(),
which forces a transaction restart.
