Chao Yu <[email protected]> 于2025年12月17日周三 16:03写道： > > 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. Hi Chao, Thanks for this explaination, so Could you resend this official patch? Thanks! > 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
