[PATCH] super: remember whether freeze holds writer rwsems

[PATCH] super: remember whether freeze holds writer rwsems

[Karl Mehltretter]

freeze_super() does not acquire the writer rwsems when the superblock is
read-only. thaw_super_locked() currently decides whether to release them
from the current SB_RDONLY flag. Filesystem error paths can change that
flag between freeze and thaw without s_umount serialization.

If a writable freeze is followed by a forced read-only transition, thaw
reports success but skips ->unfreeze_fs() and sb_freeze_unlock(). The
superblock is marked unfrozen while all writer rwsems remain write-locked.
Conversely, if a filesystem is read-only when frozen and SB_RDONLY is
cleared before thaw, thaw can release rwsems that were never acquired.

Record whether a successful freeze acquired the writer rwsems and use that
state during thaw instead of re-sampling SB_RDONLY.

Fixes: 8129ed29644b ("change sb_writers to use percpu_rw_semaphore")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---
Tested with a test-only KUnit case in x86_64 QEMU on linux-next
32b6ef9a5d0e (next-20260902). It freezes a writable ramfs, sets SB_RDONLY,
thaws it, and calls sb_start_write_trylock(). The call fails without this
patch and succeeds with it.

Backport note: before e0b62a4dee24 ("fs: add fs/super_types.h header"),
struct sb_writers is in include/linux/fs.h.

 fs/super.c                     | 4 +++-
 include/linux/fs/super_types.h | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/super.c b/fs/super.c
index 9d40252135212..e8d75cef74677 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -2329,6 +2329,7 @@ int freeze_super(struct super_block *sb, enum freeze_holder who, const void *fre
 	 */
 	WARN_ON_ONCE(freeze_inc(sb, who) > 1);
 	sb->s_writers.freeze_owner = freeze_owner;
+	sb->s_writers.freeze_rwsems_locked = true;
 	sb->s_writers.frozen = SB_FREEZE_COMPLETE;
 	wake_up_var(&sb->s_writers.frozen);
 	lockdep_sb_freeze_release(sb);
@@ -2364,7 +2365,7 @@ static int thaw_super_locked(struct super_block *sb, enum freeze_holder who,
 		goto out_unlock;
 	}
 
-	if (sb_rdonly(sb)) {
+	if (!sb->s_writers.freeze_rwsems_locked) {
 		sb->s_writers.frozen = SB_UNFROZEN;
 		sb->s_writers.freeze_owner = NULL;
 		wake_up_var(&sb->s_writers.frozen);
@@ -2387,6 +2388,7 @@ static int thaw_super_locked(struct super_block *sb, enum freeze_holder who,
 	sb->s_writers.freeze_owner = NULL;
 	wake_up_var(&sb->s_writers.frozen);
 	sb_freeze_unlock(sb, SB_FREEZE_FS);
+	sb->s_writers.freeze_rwsems_locked = false;
 out_deactivate:
 	deactivate_locked_super(sb);
 	return 0;
diff --git a/include/linux/fs/super_types.h b/include/linux/fs/super_types.h
index ecd96aeb1cee7..3e15efab65329 100644
--- a/include/linux/fs/super_types.h
+++ b/include/linux/fs/super_types.h
@@ -53,6 +53,7 @@ enum {
 
 struct sb_writers {
 	unsigned short			frozen;		/* Is sb frozen? */
+	bool				freeze_rwsems_locked; /* Freeze holds writer rwsems */
 	int				freeze_kcount;	/* How many kernel freeze requests? */
 	int				freeze_ucount;	/* How many userspace freeze requests? */
 	const void			*freeze_owner;	/* Owner of the freeze */
-- 
2.53.0

Re: [PATCH] super: remember whether freeze holds writer rwsems

[Jan Kara]

On Thu 03-09-26 08:08:55, Karl Mehltretter wrote:
> freeze_super() does not acquire the writer rwsems when the superblock is
> read-only. thaw_super_locked() currently decides whether to release them
> from the current SB_RDONLY flag. Filesystem error paths can change that
> flag between freeze and thaw without s_umount serialization.
> 
> If a writable freeze is followed by a forced read-only transition, thaw
> reports success but skips ->unfreeze_fs() and sb_freeze_unlock(). The
> superblock is marked unfrozen while all writer rwsems remain write-locked.
> Conversely, if a filesystem is read-only when frozen and SB_RDONLY is
> cleared before thaw, thaw can release rwsems that were never acquired.
> 
> Record whether a successful freeze acquired the writer rwsems and use that
> state during thaw instead of re-sampling SB_RDONLY.
> 
> Fixes: 8129ed29644b ("change sb_writers to use percpu_rw_semaphore")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>

This looks like one of these theoretical issues LLMs come up with. Frankly,
I just wouldn't bother. If the fs gets forcibly remounted RO due to
metadata errors, a frozen fs is likely the least of your worries.

								Honza

> ---
> Tested with a test-only KUnit case in x86_64 QEMU on linux-next
> 32b6ef9a5d0e (next-20260902). It freezes a writable ramfs, sets SB_RDONLY,
> thaws it, and calls sb_start_write_trylock(). The call fails without this
> patch and succeeds with it.
> 
> Backport note: before e0b62a4dee24 ("fs: add fs/super_types.h header"),
> struct sb_writers is in include/linux/fs.h.
> 
>  fs/super.c                     | 4 +++-
>  include/linux/fs/super_types.h | 1 +
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/super.c b/fs/super.c
> index 9d40252135212..e8d75cef74677 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -2329,6 +2329,7 @@ int freeze_super(struct super_block *sb, enum freeze_holder who, const void *fre
>  	 */
>  	WARN_ON_ONCE(freeze_inc(sb, who) > 1);
>  	sb->s_writers.freeze_owner = freeze_owner;
> +	sb->s_writers.freeze_rwsems_locked = true;
>  	sb->s_writers.frozen = SB_FREEZE_COMPLETE;
>  	wake_up_var(&sb->s_writers.frozen);
>  	lockdep_sb_freeze_release(sb);
> @@ -2364,7 +2365,7 @@ static int thaw_super_locked(struct super_block *sb, enum freeze_holder who,
>  		goto out_unlock;
>  	}
>  
> -	if (sb_rdonly(sb)) {
> +	if (!sb->s_writers.freeze_rwsems_locked) {
>  		sb->s_writers.frozen = SB_UNFROZEN;
>  		sb->s_writers.freeze_owner = NULL;
>  		wake_up_var(&sb->s_writers.frozen);
> @@ -2387,6 +2388,7 @@ static int thaw_super_locked(struct super_block *sb, enum freeze_holder who,
>  	sb->s_writers.freeze_owner = NULL;
>  	wake_up_var(&sb->s_writers.frozen);
>  	sb_freeze_unlock(sb, SB_FREEZE_FS);
> +	sb->s_writers.freeze_rwsems_locked = false;
>  out_deactivate:
>  	deactivate_locked_super(sb);
>  	return 0;
> diff --git a/include/linux/fs/super_types.h b/include/linux/fs/super_types.h
> index ecd96aeb1cee7..3e15efab65329 100644
> --- a/include/linux/fs/super_types.h
> +++ b/include/linux/fs/super_types.h
> @@ -53,6 +53,7 @@ enum {
>  
>  struct sb_writers {
>  	unsigned short			frozen;		/* Is sb frozen? */
> +	bool				freeze_rwsems_locked; /* Freeze holds writer rwsems */
>  	int				freeze_kcount;	/* How many kernel freeze requests? */
>  	int				freeze_ucount;	/* How many userspace freeze requests? */
>  	const void			*freeze_owner;	/* Owner of the freeze */
> -- 
> 2.53.0
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH] super: remember whether freeze holds writer rwsems

[Oleg Nesterov]

On 09/03, Karl Mehltretter wrote:
>
> Record whether a successful freeze acquired the writer rwsems and use that
> state during thaw instead of re-sampling SB_RDONLY.

Looks correct at first glance, but I leave this to other reviewers,
I don't understand fs/ enough.

> Tested with a test-only KUnit case in x86_64 QEMU on linux-next
> 32b6ef9a5d0e (next-20260902). It freezes a writable ramfs, sets SB_RDONLY,
> thaws it, and calls sb_start_write_trylock(). The call fails without this
> patch and succeeds with it.

But I am curious, how exactly did you test this?

ramfs_ops doesn't have ->freeze_fs or ->freeze_super, so
ioctl_fsfreeze() should fail?

And how it is possible to remount with -ro, reconfigure_super() should
see sb->s_writers.frozen != SB_UNFROZEN ?

Oleg.

Re: [PATCH] super: remember whether freeze holds writer rwsems

[Karl Mehltretter]

On Thu, Sep 03, 2026 at 02:44:19PM +0100, Oleg Nesterov wrote:
> > Tested with a test-only KUnit case in x86_64 QEMU on linux-next
> > 32b6ef9a5d0e (next-20260902). It freezes a writable ramfs, sets SB_RDONLY,
> > thaws it, and calls sb_start_write_trylock(). The call fails without this
> > patch and succeeds with it.
> 
> But I am curious, how exactly did you test this?
> 
> ramfs_ops doesn't have ->freeze_fs or ->freeze_super, so
> ioctl_fsfreeze() should fail?
> 
> And how it is possible to remount with -ro, reconfigure_super() should
> see sb->s_writers.frozen != SB_UNFROZEN ?
> 
Hi Oleg,

Thank you for the review!

The original test called freeze_super() directly, changed SB_RDONLY,
and called thaw_super(). 
Ramfs only supplied an initialized superblock. 
It did not use FIFREEZE or a normal remount.

I have now tested real filesystem error paths in x86_64 QEMU:

Filesystem  Baseline                         this VFS patch
----------  -------------------------------  -------------------------------
ext2        write hangs after thaw,          write succeeds unmount is clean
            unmount gives three
            rcu_sync_dtor() warnings
NILFS2      unmount gives three              unmount is clean
            rcu_sync_dtor() warnings
JFS         write hangs in                   VFS rwsem is released then the
            percpu_rwsem_wait()              write hangs in JFS txBegin()

The ext2 and NILFS2 tests use real FIFREEZE and FITHAW and corrupted disk
metadata. 

The JFS result also shows the scope of the patch. It balances the VFS rwsems,
but JFS skips txResume() after seeing SB_RDONLY. Needs JFS fix.

Karl

Re: [PATCH] super: remember whether freeze holds writer rwsems

[Karl Mehltretter]

On Thu, Sep 03, 2026 at 12:40:42PM +0100, Jan Kara wrote:
> This looks like one of these theoretical issues LLMs come up with. Frankly,
> I just wouldn't bother. If the fs gets forcibly remounted RO due to
> metadata errors, a frozen fs is likely the least of your worries.
> 

You may be right that this is not worth fixing in generic VFS. I tried the
real filesystem paths anyway:

          Without patch                 With patch
ext2      next write hangs, unmount     write and unmount complete
          gives three RCU warnings
NILFS2    unmount gives three           unmount completes cleanly
          RCU warnings

These tests use FIFREEZE, FITHAW, and corrupted filesystem images.
Other filesystems have similar error paths.

The f2fs report fixed by 930c6ab93492 exercised the same sequence. The ext4
change d3476f3dad4a removed the analogous SB_RDONLY assignment because it
confused filesystem freezing.

The VFS patch only makes thaw release what freeze acquired. It is needed
because these filesystems change SB_RDONLY outside the remount path.

Should I send a v2 with the changelog limited to the reproduced failure,
look at filesystem-specific fixes, or leave this alone?

Thanks,
Karl

Re: [PATCH] super: remember whether freeze holds writer rwsems

[Oleg Nesterov]

On 09/03, Karl Mehltretter wrote:
>
> On Thu, Sep 03, 2026 at 02:44:19PM +0100, Oleg Nesterov wrote:
> > > Tested with a test-only KUnit case in x86_64 QEMU on linux-next
> > > 32b6ef9a5d0e (next-20260902). It freezes a writable ramfs, sets SB_RDONLY,
> > > thaws it, and calls sb_start_write_trylock(). The call fails without this
> > > patch and succeeds with it.
> >
> > But I am curious, how exactly did you test this?
> >
> > ramfs_ops doesn't have ->freeze_fs or ->freeze_super, so
> > ioctl_fsfreeze() should fail?
> >
> > And how it is possible to remount with -ro, reconfigure_super() should
> > see sb->s_writers.frozen != SB_UNFROZEN ?
> >
> Hi Oleg,
>
> Thank you for the review!
>
> The original test called freeze_super() directly, changed SB_RDONLY,
> and called thaw_super().

Thanks, but this doesn't answer my questions...

How exactly this original test "called freeze_super() directly" ?
Could you show this test-case ?

IOW, how can we actually hit the problems described in the changelog?

Let me repeat, you can safely ignore me, I don't understand fs/,
I am just curious.

Oleg.

Re: [PATCH] super: remember whether freeze holds writer rwsems

[Jan Kara]

On Thu 03-09-26 21:50:45, Karl Mehltretter wrote:
> On Thu, Sep 03, 2026 at 12:40:42PM +0100, Jan Kara wrote:
> > This looks like one of these theoretical issues LLMs come up with. Frankly,
> > I just wouldn't bother. If the fs gets forcibly remounted RO due to
> > metadata errors, a frozen fs is likely the least of your worries.
> > 
> 
> You may be right that this is not worth fixing in generic VFS. I tried the
> real filesystem paths anyway:
> 
>           Without patch                 With patch
> ext2      next write hangs, unmount     write and unmount complete
>           gives three RCU warnings
> NILFS2    unmount gives three           unmount completes cleanly
>           RCU warnings
> 
> These tests use FIFREEZE, FITHAW, and corrupted filesystem images.
> Other filesystems have similar error paths.
> 
> The f2fs report fixed by 930c6ab93492 exercised the same sequence. The ext4
> change d3476f3dad4a removed the analogous SB_RDONLY assignment because it
> confused filesystem freezing.
> 
> The VFS patch only makes thaw release what freeze acquired. It is needed
> because these filesystems change SB_RDONLY outside the remount path.

Yes, generally just flipping SB_RDONLY bit is problematic and can lead
to more surprises than just fs freezing issues. So yes, I think improving
filesystems so that they just don't set SB_RDONLY on error is a better way
of handling these problems. I'd certainly accept a patch for ext2 for this.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR