[PATCH RFC 0/8] fs: introduce super write guard

[PATCH RFC 0/8] fs: introduce super write guard

[Christian Brauner]

Hey,

I'm in the process of adding a few more guards for vfs constructs.
I've chosen the easy case of super_start_write() and super_end_write()
and converted eligible callers. I think long-term we can move a lot of
the manual placement to completely rely on guards - where sensible.

Christian

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
Christian Brauner (8):
      fs: add super_write_guard
      btrfs: use super write guard in btrfs_reclaim_bgs_work()
      btrfs: use super write guard btrfs_run_defrag_inode()
      btrfs: use super write guard in sb_start_write()
      ext4: use super write guard in write_mmp_block()
      btrfs: use super write guard in relocating_repair_kthread()
      open: use super write guard in do_ftruncate()
      xfs: use super write guard in xfs_file_ioctl()

 fs/btrfs/block-group.c | 3 +--
 fs/btrfs/defrag.c      | 7 +++----
 fs/btrfs/volumes.c     | 7 ++++---
 fs/ext4/mmp.c          | 8 ++------
 fs/open.c              | 9 +++------
 fs/xfs/xfs_ioctl.c     | 6 ++----
 include/linux/fs.h     | 5 +++++
 7 files changed, 20 insertions(+), 25 deletions(-)
---
base-commit: dcb6fa37fd7bc9c3d2b066329b0d27dedf8becaa
change-id: 20251104-work-guards-fe3d7a09e258

[PATCH RFC 1/8] fs: add super_write_guard

[Christian Brauner]

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 include/linux/fs.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/linux/fs.h b/include/linux/fs.h
index c895146c1444..9d403c00fbf3 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2052,6 +2052,11 @@ static inline void sb_start_write(struct super_block *sb)
 	__sb_start_write(sb, SB_FREEZE_WRITE);
 }
 
+DEFINE_GUARD(super_write,
+	     struct super_block *,
+	     sb_start_write(_T),
+	     sb_end_write(_T))
+
 static inline bool sb_start_write_trylock(struct super_block *sb)
 {
 	return __sb_start_write_trylock(sb, SB_FREEZE_WRITE);

-- 
2.47.3

Re: [PATCH RFC 1/8] fs: add super_write_guard

[Jan Kara]

On Tue 04-11-25 13:12:30, Christian Brauner wrote:
> Signed-off-by: Christian Brauner <brauner@kernel.org>

Yeah, I think this can be a win. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza
> ---
>  include/linux/fs.h | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index c895146c1444..9d403c00fbf3 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2052,6 +2052,11 @@ static inline void sb_start_write(struct super_block *sb)
>  	__sb_start_write(sb, SB_FREEZE_WRITE);
>  }
>  
> +DEFINE_GUARD(super_write,
> +	     struct super_block *,
> +	     sb_start_write(_T),
> +	     sb_end_write(_T))
> +
>  static inline bool sb_start_write_trylock(struct super_block *sb)
>  {
>  	return __sb_start_write_trylock(sb, SB_FREEZE_WRITE);
> 
> -- 
> 2.47.3
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[PATCH RFC 2/8] btrfs: use super write guard in btrfs_reclaim_bgs_work()

[Christian Brauner]

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 fs/btrfs/block-group.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index 5322ef2ae015..8284b9435758 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -1850,7 +1850,7 @@ void btrfs_reclaim_bgs_work(struct work_struct *work)
 	if (!btrfs_should_reclaim(fs_info))
 		return;
 
-	sb_start_write(fs_info->sb);
+	guard(super_write)(fs_info->sb);
 
 	if (!btrfs_exclop_start(fs_info, BTRFS_EXCLOP_BALANCE)) {
 		sb_end_write(fs_info->sb);
@@ -2030,7 +2030,6 @@ void btrfs_reclaim_bgs_work(struct work_struct *work)
 	list_splice_tail(&retry_list, &fs_info->reclaim_bgs);
 	spin_unlock(&fs_info->unused_bgs_lock);
 	btrfs_exclop_finish(fs_info);
-	sb_end_write(fs_info->sb);
 }
 
 void btrfs_reclaim_bgs(struct btrfs_fs_info *fs_info)

-- 
2.47.3

Re: [PATCH RFC 2/8] btrfs: use super write guard in btrfs_reclaim_bgs_work()

[Qu Wenruo]

在 2025/11/4 22:42, Christian Brauner 写道:
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
>   fs/btrfs/block-group.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
> index 5322ef2ae015..8284b9435758 100644
> --- a/fs/btrfs/block-group.c
> +++ b/fs/btrfs/block-group.c
> @@ -1850,7 +1850,7 @@ void btrfs_reclaim_bgs_work(struct work_struct *work)
>   	if (!btrfs_should_reclaim(fs_info))
>   		return;
>   
> -	sb_start_write(fs_info->sb);
> +	guard(super_write)(fs_info->sb);
>   
>   	if (!btrfs_exclop_start(fs_info, BTRFS_EXCLOP_BALANCE)) {
>   		sb_end_write(fs_info->sb);

This one is still left using the old scheme, and there is another one in 
the mutex_trylock() branch.

I'm wondering how safe is the new scope based auto freeing.

Like when the freeing function is called? Will it break the existing 
freeing/locking sequence in other locations?

For this call site, sb_end_write() is always called last so it's fine.

Thanks,
Qu

> @@ -2030,7 +2030,6 @@ void btrfs_reclaim_bgs_work(struct work_struct *work)
>   	list_splice_tail(&retry_list, &fs_info->reclaim_bgs);
>   	spin_unlock(&fs_info->unused_bgs_lock);
>   	btrfs_exclop_finish(fs_info);
> -	sb_end_write(fs_info->sb);
>   }
>   
>   void btrfs_reclaim_bgs(struct btrfs_fs_info *fs_info)
> 

Re: [PATCH RFC 2/8] btrfs: use super write guard in btrfs_reclaim_bgs_work()

[Daniel Vacek]

On Tue, 4 Nov 2025 at 21:43, Qu Wenruo <wqu@suse.com> wrote:
> 在 2025/11/4 22:42, Christian Brauner 写道:
> > Signed-off-by: Christian Brauner <brauner@kernel.org>
> > ---
> >   fs/btrfs/block-group.c | 3 +--
> >   1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
> > index 5322ef2ae015..8284b9435758 100644
> > --- a/fs/btrfs/block-group.c
> > +++ b/fs/btrfs/block-group.c
> > @@ -1850,7 +1850,7 @@ void btrfs_reclaim_bgs_work(struct work_struct *work)
> >       if (!btrfs_should_reclaim(fs_info))
> >               return;
> >
> > -     sb_start_write(fs_info->sb);
> > +     guard(super_write)(fs_info->sb);
> >
> >       if (!btrfs_exclop_start(fs_info, BTRFS_EXCLOP_BALANCE)) {
> >               sb_end_write(fs_info->sb);
>
> This one is still left using the old scheme, and there is another one in
> the mutex_trylock() branch.
>
> I'm wondering how safe is the new scope based auto freeing.
>
> Like when the freeing function is called? Will it break the existing
> freeing/locking sequence in other locations?
>
> For this call site, sb_end_write() is always called last so it's fine.

It needs to be used sensibly. In this case it matches the original semantics.
Well, up to the part where a guard just consumes additional
stack/register storing the sb pointer. That is the price which needs
to be accounted for.

--nX

> Thanks,
> Qu
>
> > @@ -2030,7 +2030,6 @@ void btrfs_reclaim_bgs_work(struct work_struct *work)
> >       list_splice_tail(&retry_list, &fs_info->reclaim_bgs);
> >       spin_unlock(&fs_info->unused_bgs_lock);
> >       btrfs_exclop_finish(fs_info);
> > -     sb_end_write(fs_info->sb);
> >   }
> >
> >   void btrfs_reclaim_bgs(struct btrfs_fs_info *fs_info)
> >
>
>

[PATCH RFC 3/8] btrfs: use super write guard btrfs_run_defrag_inode()

[Christian Brauner]

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 fs/btrfs/defrag.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/defrag.c b/fs/btrfs/defrag.c
index 7b277934f66f..35fb8ee164dc 100644
--- a/fs/btrfs/defrag.c
+++ b/fs/btrfs/defrag.c
@@ -254,10 +254,9 @@ static int btrfs_run_defrag_inode(struct btrfs_fs_info *fs_info,
 	range.extent_thresh = defrag->extent_thresh;
 	file_ra_state_init(ra, inode->vfs_inode.i_mapping);
 
-	sb_start_write(fs_info->sb);
-	ret = btrfs_defrag_file(inode, ra, &range, defrag->transid,
-				BTRFS_DEFRAG_BATCH);
-	sb_end_write(fs_info->sb);
+	scoped_guard(super_write, fs_info->sb)
+		ret = btrfs_defrag_file(inode, ra, &range,
+					defrag->transid, BTRFS_DEFRAG_BATCH);
 	iput(&inode->vfs_inode);
 
 	if (ret < 0)

-- 
2.47.3

Re: [PATCH RFC 3/8] btrfs: use super write guard btrfs_run_defrag_inode()

[Daniel Vacek]

On Tue, 4 Nov 2025 at 13:14, Christian Brauner <brauner@kernel.org> wrote:
>
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
>  fs/btrfs/defrag.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/fs/btrfs/defrag.c b/fs/btrfs/defrag.c
> index 7b277934f66f..35fb8ee164dc 100644
> --- a/fs/btrfs/defrag.c
> +++ b/fs/btrfs/defrag.c
> @@ -254,10 +254,9 @@ static int btrfs_run_defrag_inode(struct btrfs_fs_info *fs_info,
>         range.extent_thresh = defrag->extent_thresh;
>         file_ra_state_init(ra, inode->vfs_inode.i_mapping);
>
> -       sb_start_write(fs_info->sb);
> -       ret = btrfs_defrag_file(inode, ra, &range, defrag->transid,
> -                               BTRFS_DEFRAG_BATCH);
> -       sb_end_write(fs_info->sb);
> +       scoped_guard(super_write, fs_info->sb)
> +               ret = btrfs_defrag_file(inode, ra, &range,
> +                                       defrag->transid, BTRFS_DEFRAG_BATCH);

We accept lines with up to ~100 characters in btrfs code.

--nX

>         iput(&inode->vfs_inode);
>
>         if (ret < 0)
>
> --
> 2.47.3
>
>

Re: [PATCH RFC 3/8] btrfs: use super write guard btrfs_run_defrag_inode()

[David Sterba]

On Tue, Nov 04, 2025 at 01:12:32PM +0100, Christian Brauner wrote:
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
>  fs/btrfs/defrag.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/defrag.c b/fs/btrfs/defrag.c
> index 7b277934f66f..35fb8ee164dc 100644
> --- a/fs/btrfs/defrag.c
> +++ b/fs/btrfs/defrag.c
> @@ -254,10 +254,9 @@ static int btrfs_run_defrag_inode(struct btrfs_fs_info *fs_info,
>  	range.extent_thresh = defrag->extent_thresh;
>  	file_ra_state_init(ra, inode->vfs_inode.i_mapping);
>  
> -	sb_start_write(fs_info->sb);
> -	ret = btrfs_defrag_file(inode, ra, &range, defrag->transid,
> -				BTRFS_DEFRAG_BATCH);
> -	sb_end_write(fs_info->sb);
> +	scoped_guard(super_write, fs_info->sb)
> +		ret = btrfs_defrag_file(inode, ra, &range,
> +					defrag->transid, BTRFS_DEFRAG_BATCH);

Please use explicit { } around scoped_guard() in btrfs code. It makes
the scope more obvious.

[PATCH RFC 4/8] btrfs: use super write guard in sb_start_write()

[Christian Brauner]

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 fs/btrfs/volumes.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 2bec544d8ba3..4152b0a5537a 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -4660,7 +4660,8 @@ static int balance_kthread(void *data)
 	struct btrfs_fs_info *fs_info = data;
 	int ret = 0;
 
-	sb_start_write(fs_info->sb);
+	guard(super_write)(fs_info->sb);
+
 	mutex_lock(&fs_info->balance_mutex);
 	if (fs_info->balance_ctl)
 		ret = btrfs_balance(fs_info, fs_info->balance_ctl, NULL);

-- 
2.47.3

Re: [PATCH RFC 4/8] btrfs: use super write guard in sb_start_write()

[Mateusz Guzik]

On Tue, Nov 04, 2025 at 01:12:33PM +0100, Christian Brauner wrote:
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
>  fs/btrfs/volumes.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 2bec544d8ba3..4152b0a5537a 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -4660,7 +4660,8 @@ static int balance_kthread(void *data)
>  	struct btrfs_fs_info *fs_info = data;
>  	int ret = 0;
>  
> -	sb_start_write(fs_info->sb);
> +	guard(super_write)(fs_info->sb);
> +
>  	mutex_lock(&fs_info->balance_mutex);
>  	if (fs_info->balance_ctl)
>  		ret = btrfs_balance(fs_info, fs_info->balance_ctl, NULL);
> 

this missed sb_end_write call removal

Re: [PATCH RFC 4/8] btrfs: use super write guard in sb_start_write()

[Christian Brauner]

On Tue, Nov 04, 2025 at 06:00:29PM +0100, Mateusz Guzik wrote:
> On Tue, Nov 04, 2025 at 01:12:33PM +0100, Christian Brauner wrote:
> > Signed-off-by: Christian Brauner <brauner@kernel.org>
> > ---
> >  fs/btrfs/volumes.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> > index 2bec544d8ba3..4152b0a5537a 100644
> > --- a/fs/btrfs/volumes.c
> > +++ b/fs/btrfs/volumes.c
> > @@ -4660,7 +4660,8 @@ static int balance_kthread(void *data)
> >  	struct btrfs_fs_info *fs_info = data;
> >  	int ret = 0;
> >  
> > -	sb_start_write(fs_info->sb);
> > +	guard(super_write)(fs_info->sb);
> > +
> >  	mutex_lock(&fs_info->balance_mutex);
> >  	if (fs_info->balance_ctl)
> >  		ret = btrfs_balance(fs_info, fs_info->balance_ctl, NULL);
> > 
> 
> this missed sb_end_write call removal

Thanks, fixed!

[PATCH RFC 5/8] ext4: use super write guard in write_mmp_block()

[Christian Brauner]

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 fs/ext4/mmp.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/fs/ext4/mmp.c b/fs/ext4/mmp.c
index ab1ff51302fb..6f57c181ff77 100644
--- a/fs/ext4/mmp.c
+++ b/fs/ext4/mmp.c
@@ -57,16 +57,12 @@ static int write_mmp_block_thawed(struct super_block *sb,
 
 static int write_mmp_block(struct super_block *sb, struct buffer_head *bh)
 {
-	int err;
-
 	/*
 	 * We protect against freezing so that we don't create dirty buffers
 	 * on frozen filesystem.
 	 */
-	sb_start_write(sb);
-	err = write_mmp_block_thawed(sb, bh);
-	sb_end_write(sb);
-	return err;
+	scoped_guard(super_write, sb)
+		return write_mmp_block_thawed(sb, bh);
 }
 
 /*

-- 
2.47.3

Re: [PATCH RFC 5/8] ext4: use super write guard in write_mmp_block()

[Jan Kara]

On Tue 04-11-25 13:12:34, Christian Brauner wrote:
> Signed-off-by: Christian Brauner <brauner@kernel.org>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ext4/mmp.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/ext4/mmp.c b/fs/ext4/mmp.c
> index ab1ff51302fb..6f57c181ff77 100644
> --- a/fs/ext4/mmp.c
> +++ b/fs/ext4/mmp.c
> @@ -57,16 +57,12 @@ static int write_mmp_block_thawed(struct super_block *sb,
>  
>  static int write_mmp_block(struct super_block *sb, struct buffer_head *bh)
>  {
> -	int err;
> -
>  	/*
>  	 * We protect against freezing so that we don't create dirty buffers
>  	 * on frozen filesystem.
>  	 */
> -	sb_start_write(sb);
> -	err = write_mmp_block_thawed(sb, bh);
> -	sb_end_write(sb);
> -	return err;
> +	scoped_guard(super_write, sb)
> +		return write_mmp_block_thawed(sb, bh);
>  }
>  
>  /*
> 
> -- 
> 2.47.3
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH RFC 5/8] ext4: use super write guard in write_mmp_block()

[Theodore Ts'o]

On Tue, Nov 04, 2025 at 01:12:34PM +0100, Christian Brauner wrote:
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
>  fs/ext4/mmp.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)

Acked-by: Theodore Ts'o <tytso@mit.edu>

Re: [PATCH RFC 5/8] ext4: use super write guard in write_mmp_block()

[Daniel Vacek]

On Tue, 4 Nov 2025 at 13:16, Christian Brauner <brauner@kernel.org> wrote:
>
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
>  fs/ext4/mmp.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/fs/ext4/mmp.c b/fs/ext4/mmp.c
> index ab1ff51302fb..6f57c181ff77 100644
> --- a/fs/ext4/mmp.c
> +++ b/fs/ext4/mmp.c
> @@ -57,16 +57,12 @@ static int write_mmp_block_thawed(struct super_block *sb,
>
>  static int write_mmp_block(struct super_block *sb, struct buffer_head *bh)
>  {
> -       int err;
> -
>         /*
>          * We protect against freezing so that we don't create dirty buffers
>          * on frozen filesystem.
>          */
> -       sb_start_write(sb);
> -       err = write_mmp_block_thawed(sb, bh);
> -       sb_end_write(sb);
> -       return err;
> +       scoped_guard(super_write, sb)
> +               return write_mmp_block_thawed(sb, bh);

Why the scoped_guard here? Should the simple guard(super_write)(sb) be
just as fine here?

--nX

>  }
>
>  /*
>
> --
> 2.47.3
>
>

Re: [PATCH RFC 5/8] ext4: use super write guard in write_mmp_block()

[Jan Kara]

On Wed 05-11-25 19:33:35, Daniel Vacek wrote:
> On Tue, 4 Nov 2025 at 13:16, Christian Brauner <brauner@kernel.org> wrote:
> >
> > Signed-off-by: Christian Brauner <brauner@kernel.org>
> > ---
> >  fs/ext4/mmp.c | 8 ++------
> >  1 file changed, 2 insertions(+), 6 deletions(-)
> >
> > diff --git a/fs/ext4/mmp.c b/fs/ext4/mmp.c
> > index ab1ff51302fb..6f57c181ff77 100644
> > --- a/fs/ext4/mmp.c
> > +++ b/fs/ext4/mmp.c
> > @@ -57,16 +57,12 @@ static int write_mmp_block_thawed(struct super_block *sb,
> >
> >  static int write_mmp_block(struct super_block *sb, struct buffer_head *bh)
> >  {
> > -       int err;
> > -
> >         /*
> >          * We protect against freezing so that we don't create dirty buffers
> >          * on frozen filesystem.
> >          */
> > -       sb_start_write(sb);
> > -       err = write_mmp_block_thawed(sb, bh);
> > -       sb_end_write(sb);
> > -       return err;
> > +       scoped_guard(super_write, sb)
> > +               return write_mmp_block_thawed(sb, bh);
> 
> Why the scoped_guard here? Should the simple guard(super_write)(sb) be
> just as fine here?

Not sure about Ted but I prefer scoped_guard() to plain guard() because the
scoping makes it more visually obvious where the unlocking happens. Of
course there has to be a balance as the indentation level can go through
the roof but that's not the case here...

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

Re: [PATCH RFC 5/8] ext4: use super write guard in write_mmp_block()

[Daniel Vacek]

On Thu, 6 Nov 2025 at 10:24, Jan Kara <jack@suse.cz> wrote:
>
> On Wed 05-11-25 19:33:35, Daniel Vacek wrote:
> > On Tue, 4 Nov 2025 at 13:16, Christian Brauner <brauner@kernel.org> wrote:
> > >
> > > Signed-off-by: Christian Brauner <brauner@kernel.org>
> > > ---
> > >  fs/ext4/mmp.c | 8 ++------
> > >  1 file changed, 2 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/fs/ext4/mmp.c b/fs/ext4/mmp.c
> > > index ab1ff51302fb..6f57c181ff77 100644
> > > --- a/fs/ext4/mmp.c
> > > +++ b/fs/ext4/mmp.c
> > > @@ -57,16 +57,12 @@ static int write_mmp_block_thawed(struct super_block *sb,
> > >
> > >  static int write_mmp_block(struct super_block *sb, struct buffer_head *bh)
> > >  {
> > > -       int err;
> > > -
> > >         /*
> > >          * We protect against freezing so that we don't create dirty buffers
> > >          * on frozen filesystem.
> > >          */
> > > -       sb_start_write(sb);
> > > -       err = write_mmp_block_thawed(sb, bh);
> > > -       sb_end_write(sb);
> > > -       return err;
> > > +       scoped_guard(super_write, sb)
> > > +               return write_mmp_block_thawed(sb, bh);
> >
> > Why the scoped_guard here? Should the simple guard(super_write)(sb) be
> > just as fine here?
>
> Not sure about Ted but I prefer scoped_guard() to plain guard() because the
> scoping makes it more visually obvious where the unlocking happens. Of
> course there has to be a balance as the indentation level can go through
> the roof but that's not the case here...

In a general case I completely agree. Though here you can see the end
of the block right on the next line so it looks quite obvious to me
how far the guarded region spans.

But the question is whether to use the guards at all. To me it's a
huge change in reading and understanding the code as more things are
implied and not explicit. Such implied things result in additional
cognitive load. I guess we can handle it eventually. Yet we can fail
from time to time and it is likely we will, at least in the beginning.

Well, my point is this is a new style we're not used to, yet. It just
takes time to adapt. (And usually a few failures to do so.)

--nX

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

[PATCH RFC 6/8] btrfs: use super write guard in relocating_repair_kthread()

[Christian Brauner]

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 fs/btrfs/volumes.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 4152b0a5537a..37b9daa14445 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -8178,7 +8178,8 @@ static int relocating_repair_kthread(void *data)
 	target = cache->start;
 	btrfs_put_block_group(cache);
 
-	sb_start_write(fs_info->sb);
+	guard(super_write)(fs_info->sb);
+
 	if (!btrfs_exclop_start(fs_info, BTRFS_EXCLOP_BALANCE)) {
 		btrfs_info(fs_info,
 			   "zoned: skip relocating block group %llu to repair: EBUSY",
@@ -8211,7 +8212,6 @@ static int relocating_repair_kthread(void *data)
 		btrfs_put_block_group(cache);
 	mutex_unlock(&fs_info->reclaim_bgs_lock);
 	btrfs_exclop_finish(fs_info);
-	sb_end_write(fs_info->sb);
 
 	return ret;
 }

-- 
2.47.3

[PATCH RFC 7/8] open: use super write guard in do_ftruncate()

[Christian Brauner]

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 fs/open.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/fs/open.c b/fs/open.c
index 3d64372ecc67..1d73a17192da 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -191,12 +191,9 @@ int do_ftruncate(struct file *file, loff_t length, int small)
 	if (error)
 		return error;
 
-	sb_start_write(inode->i_sb);
-	error = do_truncate(file_mnt_idmap(file), dentry, length,
-			    ATTR_MTIME | ATTR_CTIME, file);
-	sb_end_write(inode->i_sb);
-
-	return error;
+	scoped_guard(super_write, inode->i_sb)
+		return do_truncate(file_mnt_idmap(file), dentry, length,
+				   ATTR_MTIME | ATTR_CTIME, file);
 }
 
 int do_sys_ftruncate(unsigned int fd, loff_t length, int small)

-- 
2.47.3

Re: [PATCH RFC 7/8] open: use super write guard in do_ftruncate()

[Jan Kara]

On Tue 04-11-25 13:12:36, Christian Brauner wrote:
> Signed-off-by: Christian Brauner <brauner@kernel.org>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/open.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/open.c b/fs/open.c
> index 3d64372ecc67..1d73a17192da 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -191,12 +191,9 @@ int do_ftruncate(struct file *file, loff_t length, int small)
>  	if (error)
>  		return error;
>  
> -	sb_start_write(inode->i_sb);
> -	error = do_truncate(file_mnt_idmap(file), dentry, length,
> -			    ATTR_MTIME | ATTR_CTIME, file);
> -	sb_end_write(inode->i_sb);
> -
> -	return error;
> +	scoped_guard(super_write, inode->i_sb)
> +		return do_truncate(file_mnt_idmap(file), dentry, length,
> +				   ATTR_MTIME | ATTR_CTIME, file);
>  }
>  
>  int do_sys_ftruncate(unsigned int fd, loff_t length, int small)
> 
> -- 
> 2.47.3
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH RFC 7/8] open: use super write guard in do_ftruncate()

[Daniel Vacek]

On Tue, 4 Nov 2025 at 13:16, Christian Brauner <brauner@kernel.org> wrote:
>
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
>  fs/open.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/fs/open.c b/fs/open.c
> index 3d64372ecc67..1d73a17192da 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -191,12 +191,9 @@ int do_ftruncate(struct file *file, loff_t length, int small)
>         if (error)
>                 return error;
>
> -       sb_start_write(inode->i_sb);
> -       error = do_truncate(file_mnt_idmap(file), dentry, length,
> -                           ATTR_MTIME | ATTR_CTIME, file);
> -       sb_end_write(inode->i_sb);
> -
> -       return error;
> +       scoped_guard(super_write, inode->i_sb)
> +               return do_truncate(file_mnt_idmap(file), dentry, length,
> +                                  ATTR_MTIME | ATTR_CTIME, file);

Again, why scoped_guard? It does not make sense, or do I miss something?

--nX

>  }
>
>  int do_sys_ftruncate(unsigned int fd, loff_t length, int small)
>
> --
> 2.47.3
>
>

[PATCH RFC 8/8] xfs: use super write guard in xfs_file_ioctl()

[Christian Brauner]

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 fs/xfs/xfs_ioctl.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index a6bb7ee7a27a..f72e96f54cb5 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -1408,10 +1408,8 @@ xfs_file_ioctl(
 
 		trace_xfs_ioc_free_eofblocks(mp, &icw, _RET_IP_);
 
-		sb_start_write(mp->m_super);
-		error = xfs_blockgc_free_space(mp, &icw);
-		sb_end_write(mp->m_super);
-		return error;
+		scoped_guard(super_write, mp->m_super)
+			return xfs_blockgc_free_space(mp, &icw);
 	}
 
 	case XFS_IOC_EXCHANGE_RANGE:

-- 
2.47.3

Re: [PATCH RFC 8/8] xfs: use super write guard in xfs_file_ioctl()

[Darrick J. Wong]

On Tue, Nov 04, 2025 at 01:12:37PM +0100, Christian Brauner wrote:
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
>  fs/xfs/xfs_ioctl.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index a6bb7ee7a27a..f72e96f54cb5 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -1408,10 +1408,8 @@ xfs_file_ioctl(
>  
>  		trace_xfs_ioc_free_eofblocks(mp, &icw, _RET_IP_);
>  
> -		sb_start_write(mp->m_super);
> -		error = xfs_blockgc_free_space(mp, &icw);
> -		sb_end_write(mp->m_super);
> -		return error;
> +		scoped_guard(super_write, mp->m_super)
> +			return xfs_blockgc_free_space(mp, &icw);

Can we go full on Java?

#define with_sb_write(sb) scoped_guard(super_write, (sb))

	with_sb_write(mp->m_super)
		return xfs_blockgc_free_space(mp, &icw);

I still keep seeing scoped_guard() as a function call, not the sort of
thing that starts a new block.

[If I missed the bikeshedding war over this, I'll let this go]

--D

>  	}
>  
>  	case XFS_IOC_EXCHANGE_RANGE:
> 
> -- 
> 2.47.3
> 
> 

Re: [PATCH RFC 8/8] xfs: use super write guard in xfs_file_ioctl()

[Christian Brauner]

On Tue, Nov 04, 2025 at 09:08:45AM -0800, Darrick J. Wong wrote:
> On Tue, Nov 04, 2025 at 01:12:37PM +0100, Christian Brauner wrote:
> > Signed-off-by: Christian Brauner <brauner@kernel.org>
> > ---
> >  fs/xfs/xfs_ioctl.c | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> > 
> > diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> > index a6bb7ee7a27a..f72e96f54cb5 100644
> > --- a/fs/xfs/xfs_ioctl.c
> > +++ b/fs/xfs/xfs_ioctl.c
> > @@ -1408,10 +1408,8 @@ xfs_file_ioctl(
> >  
> >  		trace_xfs_ioc_free_eofblocks(mp, &icw, _RET_IP_);
> >  
> > -		sb_start_write(mp->m_super);
> > -		error = xfs_blockgc_free_space(mp, &icw);
> > -		sb_end_write(mp->m_super);
> > -		return error;
> > +		scoped_guard(super_write, mp->m_super)
> > +			return xfs_blockgc_free_space(mp, &icw);
> 
> Can we go full on Java?
> 
> #define with_sb_write(sb) scoped_guard(super_write, (sb))
> 
> 	with_sb_write(mp->m_super)
> 		return xfs_blockgc_free_space(mp, &icw);
> 
> I still keep seeing scoped_guard() as a function call, not the sort of
> thing that starts a new block.
> 
> [If I missed the bikeshedding war over this, I'll let this go]

It's an option and what I did for the creds stuff. The thing is thought
that scoped_guard() is more widespread by now and thus probably easier
to grasp for readers that are familiar with it. I'm not married to it.

Re: [PATCH RFC 8/8] xfs: use super write guard in xfs_file_ioctl()

[Darrick J. Wong]

On Tue, Nov 04, 2025 at 09:57:30PM +0100, Christian Brauner wrote:
> On Tue, Nov 04, 2025 at 09:08:45AM -0800, Darrick J. Wong wrote:
> > On Tue, Nov 04, 2025 at 01:12:37PM +0100, Christian Brauner wrote:
> > > Signed-off-by: Christian Brauner <brauner@kernel.org>
> > > ---
> > >  fs/xfs/xfs_ioctl.c | 6 ++----
> > >  1 file changed, 2 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> > > index a6bb7ee7a27a..f72e96f54cb5 100644
> > > --- a/fs/xfs/xfs_ioctl.c
> > > +++ b/fs/xfs/xfs_ioctl.c
> > > @@ -1408,10 +1408,8 @@ xfs_file_ioctl(
> > >  
> > >  		trace_xfs_ioc_free_eofblocks(mp, &icw, _RET_IP_);
> > >  
> > > -		sb_start_write(mp->m_super);
> > > -		error = xfs_blockgc_free_space(mp, &icw);
> > > -		sb_end_write(mp->m_super);
> > > -		return error;
> > > +		scoped_guard(super_write, mp->m_super)
> > > +			return xfs_blockgc_free_space(mp, &icw);
> > 
> > Can we go full on Java?
> > 
> > #define with_sb_write(sb) scoped_guard(super_write, (sb))
> > 
> > 	with_sb_write(mp->m_super)
> > 		return xfs_blockgc_free_space(mp, &icw);
> > 
> > I still keep seeing scoped_guard() as a function call, not the sort of
> > thing that starts a new block.
> > 
> > [If I missed the bikeshedding war over this, I'll let this go]
> 
> It's an option and what I did for the creds stuff. The thing is thought
> that scoped_guard() is more widespread by now and thus probably easier
> to grasp for readers that are familiar with it. I'm not married to it.

The wait_ macro would maintain sb_.*_write greppability, though I don't
know how important that might be.

--D

Re: [PATCH RFC 8/8] xfs: use super write guard in xfs_file_ioctl()

[Daniel Vacek]

On Tue, 4 Nov 2025 at 13:17, Christian Brauner <brauner@kernel.org> wrote:
>
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
>  fs/xfs/xfs_ioctl.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index a6bb7ee7a27a..f72e96f54cb5 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -1408,10 +1408,8 @@ xfs_file_ioctl(
>
>                 trace_xfs_ioc_free_eofblocks(mp, &icw, _RET_IP_);
>
> -               sb_start_write(mp->m_super);
> -               error = xfs_blockgc_free_space(mp, &icw);
> -               sb_end_write(mp->m_super);
> -               return error;
> +               scoped_guard(super_write, mp->m_super)
> +                       return xfs_blockgc_free_space(mp, &icw);

Again, scope_guard where not needed, right?

--nX

>         }
>
>         case XFS_IOC_EXCHANGE_RANGE:
>
> --
> 2.47.3
>
>