Messages in this thread | | | Subject | Re: [PATCH v3] f2fs: Reduce the scope of setting fsck tag when de->name_len is zero | From | Chao Yu <> | Date | Wed, 4 Aug 2021 11:32:59 +0800 |
| |
Hello, Yangtao,
I did some cleanup in your patch, and resend for you, please note that. :)
Thanks,
On 2021/7/20 20:31, 李扬韬 wrote: > HI Chao, > > From: Chao Yu <chao@kernel.org> > Date: 2021-07-20 18:23:15 > To: Yangtao Li <frank.li@vivo.com>,jaegeuk@kernel.org > Cc: linux-f2fs-devel@lists.sourceforge.net,linux-kernel@vger.kernel.org > Subject: Re: [PATCH v3] f2fs: Reduce the scope of setting fsck tag when de->name_len is zero>On 2021/7/20 15:06, Yangtao Li wrote: >>> I recently found a case where de->name_len is 0 in f2fs_fill_dentries() easily reproduced, >>> and finally set the fsck flag. >>> >>> Thread A Thread B >>> >>> f2fs_readdir >>> f2fs_read_inline_dir >>> ctx->pos = d.max >>> f2fs_add_dentry >>> f2fs_add_inline_entry >>> do_convert_inline_dir >>> f2fs_add_regular_entry >>> f2fs_readdir >>> f2fs_fill_dentries >>> set_sbi_flag(sbi, SBI_NEED_FSCK) >>> >>> Process A opens the folder, and has been reading without closing it. During this period, >>> Process B created a file under the folder (occupying multiple f2fs_dir_entry, exceeding >>> the d.max of the inline dir). After creation, process A uses the d.max of inline dir to >>> read it again, and it will read that de->name_len is 0. >>> >>> And Chao pointed out that w/o inline conversion, the race condition still can happen as below >>> >>> dir_entry1: A >>> dir_entry2: B >>> dir_entry3: C >>> free slot: _ >>> ctx->pos: ^ >>> >>> Before: >>> AAAABBBB___ >>> ^ >> >> please use blank instead of tab before '^' > > I don't know exactly what happened. In fact, in v2, spaces were used. Then it was changed to tab in v3. > >> >>> Thread B delete dir_entry2, and create dir_entry3. >>> >>> After: >>> AAAACCCCC__ >>> ^ >> >> Ditto >> >>> >>> In these scenarios, the file system is not damaged, and it's hard to avoid it. But we can bypass >>> tagging FSCK flag if: >>> a) bit_pos (:= ctx->pos % d->max) is non-zero & b) before bit_pos moves to first >>> valid dir_entry. >>> >>> Signed-off-by: Yangtao Li <frank.li@vivo.com> >>> --- >>> fs/f2fs/dir.c | 14 +++++++++----- >>> 1 file changed, 9 insertions(+), 5 deletions(-) >>> >>> diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c >>> index 456651682daf..bfe942733b5e 100644 >>> --- a/fs/f2fs/dir.c >>> +++ b/fs/f2fs/dir.c >>> @@ -1000,6 +1000,7 @@ int f2fs_fill_dentries(struct dir_context *ctx, struct f2fs_dentry_ptr *d, >>> struct f2fs_sb_info *sbi = F2FS_I_SB(d->inode); >>> struct blk_plug plug; >>> bool readdir_ra = sbi->readdir_ra == 1; >>> + bool found_valid_dirent = false; >> >> One more blank before '='. >> >> bool found_valid_dirent = false; >> > > OK. > > Thx, > > >
| |