On Sat, Apr 12, 2025 at 02:39:33PM -0400, Gabriel Shahrouzi wrote: > Fix a shutdown WARNING in bch2_dev_free caused by active write I/O > references (ca->io_ref[WRITE]) on a device being freed. > > The problem occurs when: > - The filesystem is marked read-only (BCH_FS_rw clear in c->flags). > - A subsequent operation (e.g., error handling for device removal) > incorrectly tries to grant write references back to a device. > - During final shutdown, the read-only flag causes the system to skip > stopping write I/O references (bch2_dev_io_ref_stop(ca, WRITE)). > - The leftover active write reference triggers the WARN_ON in > bch2_dev_free. > > Prevent this by checking if the filesystem is read-only before > attempting to grant write references to a device in the problematic > code path. Ensure consistency between the filesystem state flag > and the device I/O reference state during shutdown. > > --- > Not sure what to put for the fixes tag so I omitted it. The bisection > that Syzkaller found technically is correct but only because additional > warn_on checks were added recently. The git blame shows code from 8 > years ago for the specific lines being modified. > > Also not sure if devices should have read and write permissions > (ca->mi.state = BCH_MEMBER_STATE_rw) when filesystem is in read-only > mode. If that is what intended, then I believe this solution works.There > could potentially be other places where a similar scenario occurs.
Yes, that is intended. BCH_MEMBER_STATE is persistent state, stored in the superblock and controlled by the user or when we notice a device is going bad. > > Reported-by: [email protected] > Closes: > https://lore.kernel.org/all/[email protected]/T/ > Signed-off-by: Gabriel Shahrouzi <[email protected]> Nice find - applied. > --- > fs/bcachefs/super.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/fs/bcachefs/super.c b/fs/bcachefs/super.c > index b79e80a435e09..788e870bfef6a 100644 > --- a/fs/bcachefs/super.c > +++ b/fs/bcachefs/super.c > @@ -1757,7 +1757,8 @@ int bch2_dev_remove(struct bch_fs *c, struct bch_dev > *ca, int flags) > up_write(&c->state_lock); > return 0; > err: > - if (ca->mi.state == BCH_MEMBER_STATE_rw && > + if (test_bit(BCH_FS_rw, &c->flags) && > + ca->mi.state == BCH_MEMBER_STATE_rw && > !percpu_ref_is_zero(&ca->io_ref[READ])) > __bch2_dev_read_write(c, ca); > up_write(&c->state_lock); > -- > 2.43.0 >
