On 12/17/25 09:46, Zhiguo Niu wrote:
> Chao Yu <[email protected]> 于2025年12月16日周二 16:49写道:
>>
>> On 12/16/25 09:36, Zhiguo Niu wrote:
>>> Chao Yu via Linux-f2fs-devel <[email protected]>
>>> 于2025年12月15日周一 20:34写道:
>>>>
>>>> In order to avoid loading corrupted nat entry from disk.
>>>>
>>>> Cc: [email protected]
>>>> Signed-off-by: Chao Yu <[email protected]>
>>>> ---
>>>>  fs/f2fs/node.c | 9 +++++----
>>>>  1 file changed, 5 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
>>>> index ce471e033774..13c88dfd790d 100644
>>>> --- a/fs/f2fs/node.c
>>>> +++ b/fs/f2fs/node.c
>>>> @@ -630,14 +630,15 @@ int f2fs_get_node_info(struct f2fs_sb_info *sbi, 
>>>> nid_t nid,
>>>>         node_info_from_raw_nat(ni, &ne);
>>>>         f2fs_folio_put(folio, true);
>>>>  sanity_check:
>>>> -       if (__is_valid_data_blkaddr(ni->blk_addr) &&
>>>> +       if (unlikely(ni->nid != nid ||
>>> Hi Chao,
>>> (ni->nid==nid) should be always true? because the code:
>>>
>>> ni->flag = 0;
>>> ni->nid = nid;
>>> retry:
>>> or am I missing something?
>>
>> Zhiguo,
>>
>> Oh, I may missed something, let's ignore this patch.
>>
>>>
>>>> +               (__is_valid_data_blkaddr(ni->blk_addr) &&
>>> btw, Is it possible to detect that some valid Nid entries contain
>>> incorrect content?
>>> such as  ino/block_addr=NULL_ADDR in nid=4 entry?
>>
>> Something like this?
>>
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index 65ca1a5eaa88..c458df92bb0d 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -4928,16 +4928,16 @@ static inline bool is_journalled_quota(struct 
>> f2fs_sb_info *sbi)
>>         return false;
>>  }
>>
>> -static inline bool f2fs_quota_file(struct inode *inode)
>> +static inline bool f2fs_quota_file(struct f2fs_sb_info *sbi, nid_t ino)
>>  {
>>  #ifdef CONFIG_QUOTA
>>         int i;
>>
>> -       if (!f2fs_sb_has_quota_ino(F2FS_I_SB(inode)))
>> +       if (!f2fs_sb_has_quota_ino(sbi))
>>                 return false;
>>
>>         for (i = 0; i < MAXQUOTAS; i++) {
>> -               if (f2fs_qf_ino(F2FS_I_SB(inode)->sb, i) == inode->i_ino)
>> +               if (f2fs_qf_ino(sbi->sb, i) == ino)
>>                         return true;
>>         }
>>  #endif
>> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
>> index 921fb02c0f49..d1270b25ad7d 100644
>> --- a/fs/f2fs/inode.c
>> +++ b/fs/f2fs/inode.c
>> @@ -621,7 +621,7 @@ make_now:
>>                 inode->i_fop = &f2fs_file_operations;
>>                 inode->i_mapping->a_ops = &f2fs_dblock_aops;
>>                 if (IS_IMMUTABLE(inode) && !f2fs_compressed_file(inode) &&
>> -                   !f2fs_quota_file(inode))
>> +                   !f2fs_quota_file(sbi, inode->i_ino))
>>                         mapping_set_folio_min_order(inode->i_mapping, 0);
>>         } else if (S_ISDIR(inode->i_mode)) {
>>                 inode->i_op = &f2fs_dir_inode_operations;
>> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
>> index 7feead595ba5..10448e115ea0 100644
>> --- a/fs/f2fs/node.c
>> +++ b/fs/f2fs/node.c
>> @@ -643,6 +643,17 @@ sanity_check:
>>                 return -EFSCORRUPTED;
>>         }
>>
> Hi Chao
>> +       if (unlikely(f2fs_quota_file(sbi, ni->nid) &&
>> +               __is_valid_data_blkaddr(ni->blk_addr))) {
>  __is_valid_data_blkaddr(ni->blk_addr) --> !
> __is_valid_data_blkaddr(ni->blk_addr)??

Zhiguo,

Oh, yes.

>> +               set_sbi_flag(sbi, SBI_NEED_FSCK);
>> +               f2fs_err_ratelimited(sbi,
>> +                       "f2fs_get_node_info of %pS: inconsistent nat entry 
>> from qf_ino, "
>> +                       "ino:%u, nid:%u, blkaddr:%u, ver:%u, flag:%u",
>> +                       __builtin_return_address(0),
>> +                       ni->ino, ni->nid, ni->blk_addr, ni->version, 
>> ni->flag);
>> +               f2fs_handle_error(sbi, ERROR_INCONSISTENT_NAT);
>> +       }
>> +
> I think this is ok for quota file,
> and This is not easy to apply to all common cases( nid entry not only
> for quota), right? ^^

Yes, I guess partial of them may be common case, which may happen in race
condition, e.g. truncate vs read.

Thanks,

> Thanks!
>>         /* cache nat entry */
>>         if (need_cache)
>>                 cache_nat_entry(sbi, nid, &ne);
>>
>> Thanks,
>>
>>> Thanks
>>>>                 !f2fs_is_valid_blkaddr(sbi, ni->blk_addr,
>>>> -                                       DATA_GENERIC_ENHANCE)) {
>>>> +                                       DATA_GENERIC_ENHANCE)))) {
>>>>                 set_sbi_flag(sbi, SBI_NEED_FSCK);
>>>>                 f2fs_err_ratelimited(sbi,
>>>> -                       "f2fs_get_node_info of %pS: inconsistent nat 
>>>> entry, "
>>>> +                       "f2fs_get_node_info of %pS: nid:%u, inconsistent 
>>>> nat entry, "
>>>>                         "ino:%u, nid:%u, blkaddr:%u, ver:%u, flag:%u",
>>>> -                       __builtin_return_address(0),
>>>> +                       __builtin_return_address(0), nid,
>>>>                         ni->ino, ni->nid, ni->blk_addr, ni->version, 
>>>> ni->flag);
>>>>                 f2fs_handle_error(sbi, ERROR_INCONSISTENT_NAT);
>>>>                 return -EFSCORRUPTED;
>>>> --
>>>> 2.49.0
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> Linux-f2fs-devel mailing list
>>>> [email protected]
>>>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>>



_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Reply via email to