Re: [PATCH] btrfs: don't commit the super block when unmounting a shutdown filesystem

[Filipe Manana]

On Mon, Feb 16, 2026 at 12:23 AM Miquel Sabaté Solà <mssola@xxxxxxxxxx> wrote:
>
> When unmounting a filesystem we will try, among many other things, to
> commit the super block. On a filesystem that was shutdown, though, this
> will always fail with -EROFS as writes are forbidden on this context;
> and an error will be reported.
>
> Don't commit the super block on this situation, which should be fine as
> the filesystem is frozen before shutdown and, therefore, it should be at
> a consistent state.
>
> Signed-off-by: Miquel Sabaté Solà <mssola@xxxxxxxxxx>
> ---
>  fs/btrfs/disk-io.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 600287ac8eb7..cd2ce6348d88 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -4380,9 +4380,18 @@ void __cold close_ctree(struct btrfs_fs_info *fs_info)
>                  */
>                 btrfs_flush_workqueue(fs_info->delayed_workers);
>
> -               ret = btrfs_commit_super(fs_info);
> -               if (ret)
> -                       btrfs_err(fs_info, "commit super ret %d", ret);
> +               /*
> +                * If the filesystem is shutdown, then an attempt to commit the
> +                * super block (or any write) will just fail. Since we freeze
> +                * the filesystem before shutting it down, the filesystem should
> +                * be in a consistent state and not committing the super block
> +                * should be fine.

Looks good to me, but I'll rephrase this with:

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index cd2ce6348d88..84829f5e97a8 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -4383,9 +4383,8 @@ void __cold close_ctree(struct btrfs_fs_info *fs_info)
                /*
                 * If the filesystem is shutdown, then an attempt to commit the
                 * super block (or any write) will just fail. Since we freeze
-                * the filesystem before shutting it down, the filesystem should
-                * be in a consistent state and not committing the super block
-                * should be fine.
+                * the filesystem before shutting it down, the filesystem is in
+                * a consistent state and we don't need to commit super blocks.
                 */

The way it's phrased gives the reader an idea that we are not sure
about what we are doing, which sounds bad.

Reviewed-by: Filipe Manana <fdmanana@xxxxxxxx>

Pushed it to the github for-next branch, thanks.

> +                */
> +               if (!btrfs_is_shutdown(fs_info)) {
> +                       ret = btrfs_commit_super(fs_info);
> +                       if (ret)
> +                               btrfs_err(fs_info, "commit super ret %d", ret);
> +               }
>         }
>
>         kthread_stop(fs_info->transaction_kthread);
> --
> 2.53.0
>
>