Re: [PATCH] generic/736: don't run it on tmpfs

[Chuck Lever III <chuck.lever@oracle.com>]

> On Jul 29, 2024, at 9:53 AM, yangerkun <yangerkun@huawei.com> 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@huawei.com> 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@landley.net/
>>> 
>>> 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 ?


> Thanks,
> Erkun.
> 
> 
> 
>> Thanks again for all your advise!
>>> 
>>> Thanks.
>>> 
>>>> 
>>>> Signed-off-by: Yang Erkun <yangerkun@huawei.com>
>>>> ---
>>>>   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