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

["Miquel Sabaté Solà" <mssola@mssola.com>]

[-- Attachment #1: Type: text/plain, Size: 3179 bytes --]

Filipe Manana @ 2026-02-17 18:11 GMT:

> On Mon, Feb 16, 2026 at 12:23 AM Miquel Sabaté Solà <mssola@mssola.com> 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@mssola.com>
>> ---
>>  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.

Yes, I also like the new wording better, thanks!

>
> Reviewed-by: Filipe Manana <fdmanana@suse.com>
>
> 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
>>
>>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 897 bytes --]