On Tue, Aug 27, 2024 at 12:23:22PM -0400, Kent Overstreet wrote:
> 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.
>
What tripped the static checker up is that bch2_trans_unlock_long() calls
bch2_trans_unlock().
fs/bcachefs/btree_locking.c
815 void bch2_trans_unlock_long(struct btree_trans *trans)
816 {
817 bch2_trans_unlock(trans);
818 bch2_trans_srcu_unlock(trans);
819 }
But looking at it now, I guess if we call bch2_trans_unlock() twice the second
unlock is a no-op. Thanks!
regards,
dan carpenter
