On 2021/7/17 21:25, 李扬韬 wrote:
HI Chao,
From: Chao Yu <[email protected]>
Date: 2021-07-17 16:56:01
To: Yangtao Li <[email protected]>,[email protected]
Cc: [email protected],[email protected]
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 <[email protected]>
---
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
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
