[v2] f2fs: fix to preserve previous reserve_{blocks,node} value when remount

[PATCH v2] f2fs: fix to preserve previous reserve_{blocks,node} value when remount

Posted by Zhiguo Niu 1 month ago

The following steps will change previous value of reserve_{blocks,node},
this dones not match the original intention.

1.mount -t f2fs -o reserve_root=8192 imgfile test_mount/
F2FS-fs (loop56): Mounted with checkpoint version = 1b69f8c7
mount info:
/dev/block/loop56 on /data/test_mount type f2fs (xxx,reserve_root=8192,reserve_node=0,resuid=0,resgid=0,xxx)

2.mount -t f2fs -o remount,reserve_root=4096 /data/test_mount
F2FS-fs (loop56): Preserve previous reserve_root=8192
check mount info: reserve_root change to 4096
/dev/block/loop56 on /data/test_mount type f2fs (xxx,reserve_root=4096,reserve_node=0,resuid=0,resgid=0,xxx)

Prior to commit d18535132523 ("f2fs: separate the options parsing and options checking"),
the value of reserve_{blocks,node} was only set during the first mount, along with
the corresponding mount option F2FS_MOUNT_RESERVE_{ROOT,NODE} . If the mount option
F2FS_MOUNT_RESERVE_{ROOT,NODE} was found to have been set during the mount/remount,
the previously value of reserve_{blocks,node} would also be preserved, as shown in
the code below.
             if (test_opt(sbi, RESERVE_ROOT)) {
                   f2fs_info(sbi, "Preserve previous reserve_root=%u",
                          F2FS_OPTION(sbi).root_reserved_blocks);
             } else {
                   F2FS_OPTION(sbi).root_reserved_blocks = arg;
                   set_opt(sbi, RESERVE_ROOT);
             }
But commit d18535132523 ("f2fs: separate the options parsing and options checking")
only preserved the previous mount option; it did not preserve the previous value of
reserve_{blocks,node}. Since value of reserve_{blocks,node} value is assigned
or not depends on ctx->spec_mask, ctx->spec_mask should be alos handled in
f2fs_check_opt_consistency.

This patch will clear the corresponding ctx->spec_mask bits in f2fs_check_opt_consistency
to preserve the previously values of reserve_{blocks,node} if it already have a value.

Fixes: d18535132523 ("f2fs: separate the options parsing and options checking")
Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com>
---
v2: add more detail info in commit msg suggested by Chao
---
 fs/f2fs/super.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 8774c60..39f8e88 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1515,6 +1515,7 @@ static int f2fs_check_opt_consistency(struct fs_context *fc,
                        F2FS_OPTION(sbi).root_reserved_blocks);
                ctx_clear_opt(ctx, F2FS_MOUNT_RESERVE_ROOT);
                ctx->opt_mask &= ~BIT(F2FS_MOUNT_RESERVE_ROOT);
+               ctx->spec_mask &= ~F2FS_SPEC_reserve_root;
        }
        if (test_opt(sbi, RESERVE_NODE) &&
                        (ctx->opt_mask & BIT(F2FS_MOUNT_RESERVE_NODE)) &&
@@ -1523,6 +1524,7 @@ static int f2fs_check_opt_consistency(struct fs_context *fc,
                        F2FS_OPTION(sbi).root_reserved_nodes);
                ctx_clear_opt(ctx, F2FS_MOUNT_RESERVE_NODE);
                ctx->opt_mask &= ~BIT(F2FS_MOUNT_RESERVE_NODE);
+               ctx->spec_mask &= ~F2FS_SPEC_reserve_node;
        }

        err = f2fs_check_test_dummy_encryption(fc, sb);
--
1.9.1

________________________________
 This email (including its attachments) is intended only for the person or entity to which it is addressed and may contain information that is privileged, confidential or otherwise protected from disclosure. Unauthorized use, dissemination, distribution or copying of this email or the information herein or taking any action in reliance on the contents of this email or the information herein, by anyone other than the intended recipient, or an employee or agent responsible for delivering the message to the intended recipient, is strictly prohibited. If you are not the intended recipient, please do not read, copy, use or disclose any part of this e-mail to others. Please notify the sender immediately and permanently delete this e-mail and any attachments if you received it in error. Internet communications cannot be guaranteed to be timely, secure, error-free or virus-free. The sender does not accept liability for any errors or omissions.
本邮件及其附件具有保密性质，受法律保护不得泄露，仅发送给本邮件所指特定收件人。严禁非经授权使用、宣传、发布或复制本邮件或其内容。若非该特定收件人，请勿阅读、复制、 使用或披露本邮件的任何内容。若误收本邮件，请从系统中永久性删除本邮件及所有附件，并以回复邮件的方式即刻告知发件人。无法保证互联网通信及时、安全、无误或防毒。发件人对任何错漏均不承担责任。

Re: [PATCH v2] f2fs: fix to preserve previous reserve_{blocks,node} value when remount

Posted by Chao Yu 1 month ago

On 3/5/26 11:22, Zhiguo Niu wrote:
> The following steps will change previous value of reserve_{blocks,node},
> this dones not match the original intention.
> 
> 1.mount -t f2fs -o reserve_root=8192 imgfile test_mount/
> F2FS-fs (loop56): Mounted with checkpoint version = 1b69f8c7
> mount info:
> /dev/block/loop56 on /data/test_mount type f2fs (xxx,reserve_root=8192,reserve_node=0,resuid=0,resgid=0,xxx)
> 
> 2.mount -t f2fs -o remount,reserve_root=4096 /data/test_mount
> F2FS-fs (loop56): Preserve previous reserve_root=8192
> check mount info: reserve_root change to 4096
> /dev/block/loop56 on /data/test_mount type f2fs (xxx,reserve_root=4096,reserve_node=0,resuid=0,resgid=0,xxx)
> 
> Prior to commit d18535132523 ("f2fs: separate the options parsing and options checking"),
> the value of reserve_{blocks,node} was only set during the first mount, along with
> the corresponding mount option F2FS_MOUNT_RESERVE_{ROOT,NODE} . If the mount option
> F2FS_MOUNT_RESERVE_{ROOT,NODE} was found to have been set during the mount/remount,
> the previously value of reserve_{blocks,node} would also be preserved, as shown in
> the code below.
>              if (test_opt(sbi, RESERVE_ROOT)) {
>                    f2fs_info(sbi, "Preserve previous reserve_root=%u",
>                           F2FS_OPTION(sbi).root_reserved_blocks);
>              } else {
>                    F2FS_OPTION(sbi).root_reserved_blocks = arg;
>                    set_opt(sbi, RESERVE_ROOT);
>              }
> But commit d18535132523 ("f2fs: separate the options parsing and options checking")
> only preserved the previous mount option; it did not preserve the previous value of
> reserve_{blocks,node}. Since value of reserve_{blocks,node} value is assigned
> or not depends on ctx->spec_mask, ctx->spec_mask should be alos handled in
> f2fs_check_opt_consistency.
> 
> This patch will clear the corresponding ctx->spec_mask bits in f2fs_check_opt_consistency
> to preserve the previously values of reserve_{blocks,node} if it already have a value.
> 
> Fixes: d18535132523 ("f2fs: separate the options parsing and options checking")
> Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com>
> ---
> v2: add more detail info in commit msg suggested by Chao

Thanks Zhiguo, the commit message makes sense to me, and it looks good now.

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,

Re: [PATCH v2] f2fs: fix to preserve previous reserve_{blocks,node} value when remount

Posted by Chao Yu 2 weeks, 2 days ago

Jaegeuk,

I guess you may missed this email. :)

On 3/9/26 10:49, Chao Yu wrote:
> On 3/5/26 11:22, Zhiguo Niu wrote:
>> The following steps will change previous value of reserve_{blocks,node},
>> this dones not match the original intention.
>>
>> 1.mount -t f2fs -o reserve_root=8192 imgfile test_mount/
>> F2FS-fs (loop56): Mounted with checkpoint version = 1b69f8c7
>> mount info:
>> /dev/block/loop56 on /data/test_mount type f2fs (xxx,reserve_root=8192,reserve_node=0,resuid=0,resgid=0,xxx)
>>
>> 2.mount -t f2fs -o remount,reserve_root=4096 /data/test_mount
>> F2FS-fs (loop56): Preserve previous reserve_root=8192
>> check mount info: reserve_root change to 4096
>> /dev/block/loop56 on /data/test_mount type f2fs (xxx,reserve_root=4096,reserve_node=0,resuid=0,resgid=0,xxx)
>>
>> Prior to commit d18535132523 ("f2fs: separate the options parsing and options checking"),
>> the value of reserve_{blocks,node} was only set during the first mount, along with
>> the corresponding mount option F2FS_MOUNT_RESERVE_{ROOT,NODE} . If the mount option
>> F2FS_MOUNT_RESERVE_{ROOT,NODE} was found to have been set during the mount/remount,
>> the previously value of reserve_{blocks,node} would also be preserved, as shown in
>> the code below.
>>              if (test_opt(sbi, RESERVE_ROOT)) {
>>                    f2fs_info(sbi, "Preserve previous reserve_root=%u",
>>                           F2FS_OPTION(sbi).root_reserved_blocks);
>>              } else {
>>                    F2FS_OPTION(sbi).root_reserved_blocks = arg;
>>                    set_opt(sbi, RESERVE_ROOT);
>>              }
>> But commit d18535132523 ("f2fs: separate the options parsing and options checking")
>> only preserved the previous mount option; it did not preserve the previous value of
>> reserve_{blocks,node}. Since value of reserve_{blocks,node} value is assigned
>> or not depends on ctx->spec_mask, ctx->spec_mask should be alos handled in
>> f2fs_check_opt_consistency.
>>
>> This patch will clear the corresponding ctx->spec_mask bits in f2fs_check_opt_consistency
>> to preserve the previously values of reserve_{blocks,node} if it already have a value.
>>
>> Fixes: d18535132523 ("f2fs: separate the options parsing and options checking")
>> Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com>
>> ---
>> v2: add more detail info in commit msg suggested by Chao
> 
> Thanks Zhiguo, the commit message makes sense to me, and it looks good now.
> 
> Reviewed-by: Chao Yu <chao@kernel.org>
> 
> Thanks,
>