在 2025/7/12 08:21, Qu Wenruo 写道:
在 2025/7/12 04:45, David Sterba 写道:
On Wed, Jul 09, 2025 at 07:33:56AM +0930, Qu Wenruo wrote:
在 2025/7/8 22:55, David Sterba 写道:
Implement sb->freeze_super that can instruct our threads to pause
themselves. In case of (read-write) scrub this means to undo
mnt_want_write, implemented as sb_start_write()/sb_end_write().
The freeze_super callback is necessary otherwise the call
sb_want_write() inside the generic implementation hangs.
I don't this this is really going to work.
The main problem is out of btrfs, it's all about the s_writer.rw_sem.
If we have a running scrub, it holds the mnt_want_write_file(), which is
a read lock on the rw_sem.
Then we start freezing, which will call sb_wait_write(), which will do a
write lock on the rw_sem, waiting for the scrub to finish.
However the ->freeze() callback is only called when freeze_super() got
the write lock on the rw_sem.
Note there are 2 callbacks for freeze, sb::freeze_super and
sb::freeze_fs.
ioctl_fsfreeze
if fs->freeze_super
call fs_freeze_super()
Oh I forgot you implemented a new callback, ->freeze_super() to do the
extra btrfs specific handling.
But this means, we're exposing ourselves to all the extra VFS handling,
I'm not sure if this is the preferred way, nor if the extra btrfs
handling is good enough to cover what's done inside freeze_super()
function (I guess not though).
My bad, it's calling back into freeze_super() function, so it's fine.
The remaining concern is just should we do this by using
->freeze_super() callback, or do the f2fs' way by poking into
s_writer.frozen.
I'd prefer the later one, as that's already exposed to filesystems and
we do not need extra callback functions especially when it's not widely
used.
Thanks,
Qu
The only user of ->freeze_super() callback is GFS2, and I didn't think
we have enough locking/protection compared to either GFS2 nor VFS.
In that case, I still don't believe it's the correct way to go.
Add fsdevel list to see if it's even recommended to use ->freeze_super()
callback.
Thanks,
Qu
else
freeze_super()
sb_wait_write()
if (sb->freeze_fs)
call sb->freeze_fs()
What you describe matches the 'else' branch as this unconditionally
blocks on the sb_wait_write and never gets to our callback. This is what
I observed too.
To fix that I've added the sb::freeze_super so it can do something
to avoid the lockup in sb_wait_write(), because we must at some point
call freeze_super().
Schematically it goes like this:
Freeze thread Scrub
btrfs_ioctl_scrub
mnt_want_write_file (effectively sb_want_write())
ioctl_fsfreeze
fs->freeze_super
set fs->flags FREEZING
freeze_super
sb_wait_freeze()
(BLOCKS)
(chunk loop)
scrub_simple_mirror()
if fs->flags
sb_end_write()
wait_on_bit fs->flags FREEZING
(BLOCKS)
(UNBLOCKS)
...
Unfreezing is basically the same in reverse:
ioctl_fsthaw
fs->thaw_super
thaw_super()
unset fs->flags FREEZING
(UNBLOCKS)
sb_want_write()
...
So if you see a problem in that please let me know, I've tested the
patch and it worked but I might me missing something.
