----- Original Message ----- > Can someone pick this up? Maybe through Jens' block tree as that is > where my commit this is fixing up came from. > > For reference: > > > Reviewed-by: Christoph Hellwig <hch@xxxxxx> > > On Thu, Dec 24, 2020 at 04:49:54AM +0000, Satya Tangirala wrote: > > freeze/thaw_bdev() currently use bdev->bd_fsfreeze_count to infer > > whether or not bdev->bd_fsfreeze_sb is valid (it's valid iff > > bd_fsfreeze_count is non-zero). thaw_bdev() doesn't nullify > > bd_fsfreeze_sb. > > > > But this means a freeze_bdev() call followed by a thaw_bdev() call can > > leave bd_fsfreeze_sb with a non-null value, while bd_fsfreeze_count is > > zero. If freeze_bdev() is called again, and this time > > get_active_super() returns NULL (e.g. because the FS is unmounted), > > we'll end up with bd_fsfreeze_count > 0, but bd_fsfreeze_sb is > > *untouched* - it stays the same (now garbage) value. A subsequent > > thaw_bdev() will decide that the bd_fsfreeze_sb value is legitimate > > (since bd_fsfreeze_count > 0), and attempt to use it. > > > > Fix this by always setting bd_fsfreeze_sb to NULL when > > bd_fsfreeze_count is successfully decremented to 0 in thaw_sb(). > > Alternatively, we could set bd_fsfreeze_sb to whatever > > get_active_super() returns in freeze_bdev() whenever bd_fsfreeze_count > > is successfully incremented to 1 from 0 (which can be achieved cleanly > > by moving the line currently setting bd_fsfreeze_sb to immediately > > after the "sync:" label, but it might be a little too subtle/easily > > overlooked in future). > > > > This fixes the currently panicking xfstests generic/085. > > > > Fixes: 040f04bd2e82 ("fs: simplify freeze_bdev/thaw_bdev") > > Signed-off-by: Satya Tangirala <satyat@xxxxxxxxxx> > > --- > > fs/block_dev.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/fs/block_dev.c b/fs/block_dev.c > > index 9e56ee1f2652..12a811a9ae4b 100644 > > --- a/fs/block_dev.c > > +++ b/fs/block_dev.c > > @@ -606,6 +606,8 @@ int thaw_bdev(struct block_device *bdev) > > error = thaw_super(sb); > > if (error) > > bdev->bd_fsfreeze_count++; > > + else > > + bdev->bd_fsfreeze_sb = NULL; > > out: > > mutex_unlock(&bdev->bd_fsfreeze_mutex); > > return error; > > -- > > 2.29.2.729.g45daf8777d-goog > ---end quoted text--- > > Funny you should ask. I came across this bug in my testing of gfs2 and my patch is slightly different. I was wondering who to send it to. Perhaps Viro? Regards, Bob Peterson