On Mon, Jul 29, 2024 at 3:30 PM Chuck Lever III <chuck.lever@xxxxxxxxxx> wrote: > > > > > On Jul 29, 2024, at 9:53 AM, 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. > > > > Looking forward to your comments! > > Is this the same bug as https://bugzilla.kernel.org/show_bug.cgi?id=219094 ? Yes, my last comment there explicitly points to this thread. > > > > 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 > >>>> > >>>> > > -- > Chuck Lever > >
