* [PATCH] f2fs: fix ctx->pos in f2fs_read_inline_dir() @ 2021-07-17 3:49 ` Yangtao Li 0 siblings, 0 replies; 8+ messages in thread From: Yangtao Li @ 2021-07-17 3:49 UTC (permalink / raw) To: jaegeuk, chao; +Cc: linux-f2fs-devel, linux-kernel, Yangtao Li 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 returning early in f2fs_read_inline_dir will cause the process to be unable to see the changes before reopening the file. So don't return early and remove the modification of ctx->pos in f2fs_read_inline_dir(). Signed-off-by: Yangtao Li <frank.li@vivo.com> --- fs/f2fs/inline.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c index 56a20d5c15da..fc6551139a3d 100644 --- a/fs/f2fs/inline.c +++ b/fs/f2fs/inline.c @@ -729,9 +729,6 @@ int f2fs_read_inline_dir(struct file *file, struct dir_context *ctx, make_dentry_ptr_inline(inode, &d, inline_dentry); - if (ctx->pos == d.max) - return 0; - ipage = f2fs_get_node_page(F2FS_I_SB(inode), inode->i_ino); if (IS_ERR(ipage)) return PTR_ERR(ipage); @@ -747,8 +744,6 @@ int f2fs_read_inline_dir(struct file *file, struct dir_context *ctx, make_dentry_ptr_inline(inode, &d, inline_dentry); err = f2fs_fill_dentries(ctx, &d, 0, fstr); - if (!err) - ctx->pos = d.max; f2fs_put_page(ipage, 0); return err < 0 ? err : 0; -- 2.32.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [f2fs-dev] [PATCH] f2fs: fix ctx->pos in f2fs_read_inline_dir() @ 2021-07-17 3:49 ` Yangtao Li 0 siblings, 0 replies; 8+ messages in thread From: Yangtao Li @ 2021-07-17 3:49 UTC (permalink / raw) To: jaegeuk, chao; +Cc: Yangtao Li, linux-kernel, linux-f2fs-devel 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 returning early in f2fs_read_inline_dir will cause the process to be unable to see the changes before reopening the file. So don't return early and remove the modification of ctx->pos in f2fs_read_inline_dir(). Signed-off-by: Yangtao Li <frank.li@vivo.com> --- fs/f2fs/inline.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c index 56a20d5c15da..fc6551139a3d 100644 --- a/fs/f2fs/inline.c +++ b/fs/f2fs/inline.c @@ -729,9 +729,6 @@ int f2fs_read_inline_dir(struct file *file, struct dir_context *ctx, make_dentry_ptr_inline(inode, &d, inline_dentry); - if (ctx->pos == d.max) - return 0; - ipage = f2fs_get_node_page(F2FS_I_SB(inode), inode->i_ino); if (IS_ERR(ipage)) return PTR_ERR(ipage); @@ -747,8 +744,6 @@ int f2fs_read_inline_dir(struct file *file, struct dir_context *ctx, make_dentry_ptr_inline(inode, &d, inline_dentry); err = f2fs_fill_dentries(ctx, &d, 0, fstr); - if (!err) - ctx->pos = d.max; f2fs_put_page(ipage, 0); return err < 0 ? err : 0; -- 2.32.0 _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [f2fs-dev] [PATCH] f2fs: fix ctx->pos in f2fs_read_inline_dir() 2021-07-17 3:49 ` [f2fs-dev] " Yangtao Li @ 2021-07-17 8:56 ` Chao Yu -1 siblings, 0 replies; 8+ messages in thread From: Chao Yu @ 2021-07-17 8:56 UTC (permalink / raw) To: Yangtao Li, jaegeuk; +Cc: linux-kernel, linux-f2fs-devel On 2021/7/17 11:49, 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. Nice catch! > > And returning early in f2fs_read_inline_dir will cause the process to be unable to see > the changes before reopening the file. > > So don't return early and remove the modification of ctx->pos in f2fs_read_inline_dir(). > > Signed-off-by: Yangtao Li <frank.li@vivo.com> > --- > fs/f2fs/inline.c | 5 ----- > 1 file changed, 5 deletions(-) > > diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c > index 56a20d5c15da..fc6551139a3d 100644 > --- a/fs/f2fs/inline.c > +++ b/fs/f2fs/inline.c > @@ -729,9 +729,6 @@ int f2fs_read_inline_dir(struct file *file, struct dir_context *ctx, > > make_dentry_ptr_inline(inode, &d, inline_dentry); > > - if (ctx->pos == d.max) > - return 0; > - > ipage = f2fs_get_node_page(F2FS_I_SB(inode), inode->i_ino); > if (IS_ERR(ipage)) > return PTR_ERR(ipage); > @@ -747,8 +744,6 @@ int f2fs_read_inline_dir(struct file *file, struct dir_context *ctx, > make_dentry_ptr_inline(inode, &d, inline_dentry); > > err = f2fs_fill_dentries(ctx, &d, 0, fstr); After this function, ctx->pos will point to start position of first free slot after last dir_entry, we can't guarantee that the free slot won't be used in above race condition, right? Moreover, w/o inline conversion, the race condition still can happen as below, right? dir_entry1: A dir_entry2: B dir_entry3: C free slot: _ Before: AAAABBBB___ ^ Thread B delete dir_entry2, and create dir_entry3. After: AAAACCCCC__ ^ Thanks, > - if (!err) > - ctx->pos = d.max; > > f2fs_put_page(ipage, 0); > return err < 0 ? err : 0; > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [f2fs-dev] [PATCH] f2fs: fix ctx->pos in f2fs_read_inline_dir() @ 2021-07-17 8:56 ` Chao Yu 0 siblings, 0 replies; 8+ messages in thread From: Chao Yu @ 2021-07-17 8:56 UTC (permalink / raw) To: Yangtao Li, jaegeuk; +Cc: linux-kernel, linux-f2fs-devel On 2021/7/17 11:49, 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. Nice catch! > > And returning early in f2fs_read_inline_dir will cause the process to be unable to see > the changes before reopening the file. > > So don't return early and remove the modification of ctx->pos in f2fs_read_inline_dir(). > > Signed-off-by: Yangtao Li <frank.li@vivo.com> > --- > fs/f2fs/inline.c | 5 ----- > 1 file changed, 5 deletions(-) > > diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c > index 56a20d5c15da..fc6551139a3d 100644 > --- a/fs/f2fs/inline.c > +++ b/fs/f2fs/inline.c > @@ -729,9 +729,6 @@ int f2fs_read_inline_dir(struct file *file, struct dir_context *ctx, > > make_dentry_ptr_inline(inode, &d, inline_dentry); > > - if (ctx->pos == d.max) > - return 0; > - > ipage = f2fs_get_node_page(F2FS_I_SB(inode), inode->i_ino); > if (IS_ERR(ipage)) > return PTR_ERR(ipage); > @@ -747,8 +744,6 @@ int f2fs_read_inline_dir(struct file *file, struct dir_context *ctx, > make_dentry_ptr_inline(inode, &d, inline_dentry); > > err = f2fs_fill_dentries(ctx, &d, 0, fstr); After this function, ctx->pos will point to start position of first free slot after last dir_entry, we can't guarantee that the free slot won't be used in above race condition, right? Moreover, w/o inline conversion, the race condition still can happen as below, right? dir_entry1: A dir_entry2: B dir_entry3: C free slot: _ Before: AAAABBBB___ ^ Thread B delete dir_entry2, and create dir_entry3. After: AAAACCCCC__ ^ Thanks, > - if (!err) > - ctx->pos = d.max; > > f2fs_put_page(ipage, 0); > return err < 0 ? err : 0; > _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re:Re: [f2fs-dev] [PATCH] f2fs: fix ctx->pos in f2fs_read_inline_dir() 2021-07-17 8:56 ` Chao Yu @ 2021-07-17 13:25 ` 李扬韬 -1 siblings, 0 replies; 8+ messages in thread From: 李扬韬 @ 2021-07-17 13:25 UTC (permalink / raw) To: Chao Yu; +Cc: jaegeuk, linux-kernel, linux-f2fs-devel HI Chao, From: Chao Yu <chao@kernel.org> Date: 2021-07-17 16:56:01 To: Yangtao Li <frank.li@vivo.com>,jaegeuk@kernel.org Cc: linux-kernel@vger.kernel.org,linux-f2fs-devel@lists.sourceforge.net Subject: Re: [f2fs-dev] [PATCH] f2fs: fix ctx->pos in f2fs_read_inline_dir()>On 2021/7/17 11:49, 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. > >Nice catch! > >> >> And returning early in f2fs_read_inline_dir will cause the process to be unable to see >> the changes before reopening the file. >> >> So don't return early and remove the modification of ctx->pos in f2fs_read_inline_dir(). >> >> Signed-off-by: Yangtao Li <frank.li@vivo.com> >> --- >> fs/f2fs/inline.c | 5 ----- >> 1 file changed, 5 deletions(-) >> >> diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c >> index 56a20d5c15da..fc6551139a3d 100644 >> --- a/fs/f2fs/inline.c >> +++ b/fs/f2fs/inline.c >> @@ -729,9 +729,6 @@ int f2fs_read_inline_dir(struct file *file, struct dir_context *ctx, >> >> make_dentry_ptr_inline(inode, &d, inline_dentry); >> >> - if (ctx->pos == d.max) >> - return 0; >> - >> ipage = f2fs_get_node_page(F2FS_I_SB(inode), inode->i_ino); >> if (IS_ERR(ipage)) >> return PTR_ERR(ipage); >> @@ -747,8 +744,6 @@ int f2fs_read_inline_dir(struct file *file, struct dir_context *ctx, >> make_dentry_ptr_inline(inode, &d, inline_dentry); >> >> err = f2fs_fill_dentries(ctx, &d, 0, fstr); > >After this function, ctx->pos will point to start position of first free slot after >last dir_entry, we can't guarantee that the free slot won't be used in above race >condition, right? > >Moreover, w/o inline conversion, the race condition still can happen as below, right? > >dir_entry1: A >dir_entry2: B >dir_entry3: C >free slot: _ > >Before: >AAAABBBB___ > ^ >Thread B delete dir_entry2, and create dir_entry3. > >After: >AAAACCCCC__ > ^ Taking into account the above situations, I think this case where de->name_len is 0 in f2fs_fill_dentries() should be normal and there is no way to avoid it. Maybe we shouldn't set fsck flag at this time? Because the file system is not damaged. MBR / Yangtao ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [f2fs-dev] [PATCH] f2fs: fix ctx->pos in f2fs_read_inline_dir() @ 2021-07-17 13:25 ` 李扬韬 0 siblings, 0 replies; 8+ messages in thread From: 李扬韬 @ 2021-07-17 13:25 UTC (permalink / raw) To: Chao Yu; +Cc: jaegeuk, linux-kernel, linux-f2fs-devel HI Chao, From: Chao Yu <chao@kernel.org> Date: 2021-07-17 16:56:01 To: Yangtao Li <frank.li@vivo.com>,jaegeuk@kernel.org Cc: linux-kernel@vger.kernel.org,linux-f2fs-devel@lists.sourceforge.net Subject: Re: [f2fs-dev] [PATCH] f2fs: fix ctx->pos in f2fs_read_inline_dir()>On 2021/7/17 11:49, 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. > >Nice catch! > >> >> And returning early in f2fs_read_inline_dir will cause the process to be unable to see >> the changes before reopening the file. >> >> So don't return early and remove the modification of ctx->pos in f2fs_read_inline_dir(). >> >> Signed-off-by: Yangtao Li <frank.li@vivo.com> >> --- >> fs/f2fs/inline.c | 5 ----- >> 1 file changed, 5 deletions(-) >> >> diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c >> index 56a20d5c15da..fc6551139a3d 100644 >> --- a/fs/f2fs/inline.c >> +++ b/fs/f2fs/inline.c >> @@ -729,9 +729,6 @@ int f2fs_read_inline_dir(struct file *file, struct dir_context *ctx, >> >> make_dentry_ptr_inline(inode, &d, inline_dentry); >> >> - if (ctx->pos == d.max) >> - return 0; >> - >> ipage = f2fs_get_node_page(F2FS_I_SB(inode), inode->i_ino); >> if (IS_ERR(ipage)) >> return PTR_ERR(ipage); >> @@ -747,8 +744,6 @@ int f2fs_read_inline_dir(struct file *file, struct dir_context *ctx, >> make_dentry_ptr_inline(inode, &d, inline_dentry); >> >> err = f2fs_fill_dentries(ctx, &d, 0, fstr); > >After this function, ctx->pos will point to start position of first free slot after >last dir_entry, we can't guarantee that the free slot won't be used in above race >condition, right? > >Moreover, w/o inline conversion, the race condition still can happen as below, right? > >dir_entry1: A >dir_entry2: B >dir_entry3: C >free slot: _ > >Before: >AAAABBBB___ > ^ >Thread B delete dir_entry2, and create dir_entry3. > >After: >AAAACCCCC__ > ^ Taking into account the above situations, I think this case where de->name_len is 0 in f2fs_fill_dentries() should be normal and there is no way to avoid it. Maybe we shouldn't set fsck flag at this time? Because the file system is not damaged. MBR / Yangtao _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [f2fs-dev] [PATCH] f2fs: fix ctx->pos in f2fs_read_inline_dir() 2021-07-17 13:25 ` 李扬韬 @ 2021-07-17 14:30 ` Chao Yu -1 siblings, 0 replies; 8+ messages in thread From: Chao Yu @ 2021-07-17 14:30 UTC (permalink / raw) To: 李扬韬; +Cc: jaegeuk, linux-kernel, linux-f2fs-devel On 2021/7/17 21:25, 李扬韬 wrote: > HI Chao, > > From: Chao Yu <chao@kernel.org> > Date: 2021-07-17 16:56:01 > To: Yangtao Li <frank.li@vivo.com>,jaegeuk@kernel.org > Cc: linux-kernel@vger.kernel.org,linux-f2fs-devel@lists.sourceforge.net > Subject: Re: [f2fs-dev] [PATCH] f2fs: fix ctx->pos in f2fs_read_inline_dir()>On 2021/7/17 11:49, 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. >> >> Nice catch! >> >>> >>> And returning early in f2fs_read_inline_dir will cause the process to be unable to see >>> the changes before reopening the file. >>> >>> So don't return early and remove the modification of ctx->pos in f2fs_read_inline_dir(). >>> >>> Signed-off-by: Yangtao Li <frank.li@vivo.com> >>> --- >>> fs/f2fs/inline.c | 5 ----- >>> 1 file changed, 5 deletions(-) >>> >>> diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c >>> index 56a20d5c15da..fc6551139a3d 100644 >>> --- a/fs/f2fs/inline.c >>> +++ b/fs/f2fs/inline.c >>> @@ -729,9 +729,6 @@ int f2fs_read_inline_dir(struct file *file, struct dir_context *ctx, >>> >>> make_dentry_ptr_inline(inode, &d, inline_dentry); >>> >>> - if (ctx->pos == d.max) >>> - return 0; >>> - >>> ipage = f2fs_get_node_page(F2FS_I_SB(inode), inode->i_ino); >>> if (IS_ERR(ipage)) >>> return PTR_ERR(ipage); >>> @@ -747,8 +744,6 @@ int f2fs_read_inline_dir(struct file *file, struct dir_context *ctx, >>> make_dentry_ptr_inline(inode, &d, inline_dentry); >>> >>> err = f2fs_fill_dentries(ctx, &d, 0, fstr); >> >> After this function, ctx->pos will point to start position of first free slot after >> last dir_entry, we can't guarantee that the free slot won't be used in above race >> condition, right? >> >> Moreover, w/o inline conversion, the race condition still can happen as below, right? >> >> dir_entry1: A >> dir_entry2: B >> dir_entry3: C >> free slot: _ >> >> Before: >> AAAABBBB___ >> ^ >> Thread B delete dir_entry2, and create dir_entry3. >> >> After: >> AAAACCCCC__ >> ^ > > Taking into account the above situations, I think this case where de->name_len is 0 in f2fs_fill_dentries() > should be normal and there is no way to avoid it. Maybe we shouldn't set fsck flag at this time? > Because the file system is not damaged. Yangtao, IMO, it should bypass tagging FSCK flag only if: a) bit_pos (:= ctx->pos % d->max) is non-zero & b) before bit_pos moves to first valid dir_entry. Thanks, > > MBR / Yangtao > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [f2fs-dev] [PATCH] f2fs: fix ctx->pos in f2fs_read_inline_dir() @ 2021-07-17 14:30 ` Chao Yu 0 siblings, 0 replies; 8+ messages in thread From: Chao Yu @ 2021-07-17 14:30 UTC (permalink / raw) To: 李扬韬; +Cc: jaegeuk, linux-kernel, linux-f2fs-devel On 2021/7/17 21:25, 李扬韬 wrote: > HI Chao, > > From: Chao Yu <chao@kernel.org> > Date: 2021-07-17 16:56:01 > To: Yangtao Li <frank.li@vivo.com>,jaegeuk@kernel.org > Cc: linux-kernel@vger.kernel.org,linux-f2fs-devel@lists.sourceforge.net > Subject: Re: [f2fs-dev] [PATCH] f2fs: fix ctx->pos in f2fs_read_inline_dir()>On 2021/7/17 11:49, 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. >> >> Nice catch! >> >>> >>> And returning early in f2fs_read_inline_dir will cause the process to be unable to see >>> the changes before reopening the file. >>> >>> So don't return early and remove the modification of ctx->pos in f2fs_read_inline_dir(). >>> >>> Signed-off-by: Yangtao Li <frank.li@vivo.com> >>> --- >>> fs/f2fs/inline.c | 5 ----- >>> 1 file changed, 5 deletions(-) >>> >>> diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c >>> index 56a20d5c15da..fc6551139a3d 100644 >>> --- a/fs/f2fs/inline.c >>> +++ b/fs/f2fs/inline.c >>> @@ -729,9 +729,6 @@ int f2fs_read_inline_dir(struct file *file, struct dir_context *ctx, >>> >>> make_dentry_ptr_inline(inode, &d, inline_dentry); >>> >>> - if (ctx->pos == d.max) >>> - return 0; >>> - >>> ipage = f2fs_get_node_page(F2FS_I_SB(inode), inode->i_ino); >>> if (IS_ERR(ipage)) >>> return PTR_ERR(ipage); >>> @@ -747,8 +744,6 @@ int f2fs_read_inline_dir(struct file *file, struct dir_context *ctx, >>> make_dentry_ptr_inline(inode, &d, inline_dentry); >>> >>> err = f2fs_fill_dentries(ctx, &d, 0, fstr); >> >> After this function, ctx->pos will point to start position of first free slot after >> last dir_entry, we can't guarantee that the free slot won't be used in above race >> condition, right? >> >> Moreover, w/o inline conversion, the race condition still can happen as below, right? >> >> dir_entry1: A >> dir_entry2: B >> dir_entry3: C >> free slot: _ >> >> Before: >> AAAABBBB___ >> ^ >> Thread B delete dir_entry2, and create dir_entry3. >> >> After: >> AAAACCCCC__ >> ^ > > Taking into account the above situations, I think this case where de->name_len is 0 in f2fs_fill_dentries() > should be normal and there is no way to avoid it. Maybe we shouldn't set fsck flag at this time? > Because the file system is not damaged. Yangtao, IMO, it should bypass tagging FSCK flag only if: a) bit_pos (:= ctx->pos % d->max) is non-zero & b) before bit_pos moves to first valid dir_entry. Thanks, > > MBR / Yangtao > > _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-07-17 14:31 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-07-17 3:49 [PATCH] f2fs: fix ctx->pos in f2fs_read_inline_dir() Yangtao Li 2021-07-17 3:49 ` [f2fs-dev] " Yangtao Li 2021-07-17 8:56 ` Chao Yu 2021-07-17 8:56 ` Chao Yu 2021-07-17 13:25 ` 李扬韬 2021-07-17 13:25 ` 李扬韬 2021-07-17 14:30 ` Chao Yu 2021-07-17 14:30 ` Chao Yu
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.