On Fri, Nov 03, 2023 at 04:43:52PM +0100, Jan Kara wrote: > On Fri 03-11-23 16:10:10, Christian Brauner wrote: > > On Fri, Nov 03, 2023 at 03:19:40PM +0100, Christoph Hellwig wrote: > > > On Fri, Nov 03, 2023 at 02:52:27PM +0100, Christian Brauner wrote: > > > > Fix this by counting the number of block devices that requested the > > > > filesystem to be frozen in @bdev_count in struct sb_writers and only > > > > unfreeze once the @bdev_count hits zero. Survives fstests and blktests > > > > and makes the reproducer succeed. > > > > > > Is there a good reason to not just refcount the freezes in general? > > > > If we start counting freezes in general we break userspace as > > freeze_super() is called from ioctl_fsfreeze() and that expects to > > return EBUSY on an already frozen filesystem. xfs scrub might be another > > user that might break if we change that. > > I guess Christoph meant that we'd count all the sb freezes into the > refcount (what is now bdev_count) but without HOLDER_BDEV flag we will Ah, sorry I didn't get that from the message. > return EBUSY if the refcount is > 0 instead of incrementing it. Yeah, that should work. I'm playing with this rn. > There would be a subtle behavioral difference that now if you freeze the fs > with ioctl_fsfreeze() and then try to freeze through the blockdev, you'll > get EBUSY while with the new method the bdev freeze will succeed but I > don't think that can do any harm. It even kind of makes more sense. Yeah, though afaict that change is independent of whether we count all freeze_super() calls or only such with HOLDER_BDEV. I've had the same reaction as you.