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

[yangerkun <yangerkun@huawei.com>]

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!

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
>>>
>>>