在 2024/7/29 22:32, Filipe Manana 写道:
On Mon, Jul 29, 2024 at 2:54 PM yangerkun <yangerkun@xxxxxxxxxx> wrote:
Hi,
在 2024/7/24 21:30, yangerkun 写道:
Hi, All,
Sorry for the delay relay(something happened, and cannot use pc
before...).
在 2024/7/21 1:26, Filipe Manana 写道:
On Sat, Jul 20, 2024 at 9:38 AM Yang Erkun <yangerkun@xxxxxxxxxx> wrote:
We use offset_readdir for tmpfs, and every we call rename, the offset
for the parent dir will increase by 1. So for tmpfs we will always
fail since the infinite readdir.
Having an infinite readdir sounds like a bug, or at least an
inconvenience and surprising for users.
We had that problem in btrfs which affected users/applications, see:
https://lore.kernel.org/linux-btrfs/2c8c55ec-04c6-e0dc-9c5c-8c7924778c35@xxxxxxxxxxx/
which was surprising for them since every other filesystem they
used/tested didn't have that problem.
Why not fix tmpfs?
Thanks for all your advise, I will give a detail analysis first(maybe
until last week I can do it), and after we give a conclusion about does
this behavior a bug or something expected to occur, I will choose the
next step!
The case generic/736 do something like below:
1. create 5000 files(1 2 3 ...) under one dir(testdir)
2. call readdir(man 3 readdir) once, and get entry
3. rename(entry, "TEMPFILE"), then rename("TMPFILE", entry)
4. loop 2~3, until readdir return nothing of we loop too many times(15000)
For tmpfs before a2e459555c5f("shmem: stable directory offsets"), every
rename called, the new dentry will insert to d_subdirs *head* of parent
dentry, and dcache_readdir won't reenter this dentry if we have already
enter the dentry, so in step 4 we will break the test since readdir
return nothing (I have try to change __d_move the insert to the "tail"
of d_sub_dirs, problem can still happend).
But after commit a2e459555c5f("shmem: stable directory offsets"),
simple_offset_rename will just add the new dentry to the maple tree of
&SHMEM_I(inode)->dir_offsets->mt with the key always inc by 1(since
simple_offset_add we will find free entry start with octx->newx_offset,
so the entry freed in simple_offset_remove won't be found). And the same
case upper will be break since we loop too many times(we can fall into
infinite readdir without this break).
I prefer this is really a bug, and for the way to fix it, I think we can
just use the same logic what 9b378f6ad48cf("btrfs: fix infinite
directory reads") has did, introduce a last_index when we open the dir,
and then readdir will not return the entry which index greater than the
last index.
Don't forget to reset the index to whatever is the current last index
when rewind() is called.
We ended up with that bug in btrfs later, see:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e60aa5da14d01fed8411202dbe4adf6c44bd2a57
Thanks for your reminder, will change offset_dir_llseek too!
Anyway, if the same mistake is made, it would be caught by a test case
for fstests I submitted after:
https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/commit/?id=68b958f5dc4ab13cfd86f7fb82621f9f022b7626
Looking forward to your comments!
Thanks,
Erkun.
Thanks again for all your advise!
Thanks.
Signed-off-by: Yang Erkun <yangerkun@xxxxxxxxxx>
---
tests/generic/736 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/generic/736 b/tests/generic/736
index d2432a82..9fafa8df 100755
--- a/tests/generic/736
+++ b/tests/generic/736
@@ -18,7 +18,7 @@ _cleanup()
rm -fr $target_dir
}
-_supported_fs generic
+_supported_fs generic ^tmpfs
_require_test
_require_test_program readdir-while-renames
--
2.39.2
