Re: [RFC PATCH] vfs: limit directory child dentry retention

[Ian Kent <raven@themaw.net>]

On 7/4/26 18:35, Christian Brauner wrote:
> On Wed, Apr 01, 2026 at 10:10:55AM +0800, Ian Kent wrote:
>> On 31/3/26 17:39, Christian Brauner wrote:
>>> On Tue, Mar 31, 2026 at 09:29:09AM +0800, Ian Kent wrote:
>>>> If there's a very large number of children present in a directory dentry
>>>> then the benifit from retaining stale child dentries for re-use can
>>>> become ineffective. Even hashed lookup can become ineffective as hash
>>>> chains grow, time taken to umount a file system can increase a lot, as
>>>> well as child dentry traversals resulting in lock held too long log
>>>> messages.
>>> Fwiw, there's also e6957c99dca5 ("vfs: Add a sysctl for automated deletion of dentry")
>> I'm pretty sure I saw that earlier on but had forgotten about it when I
>>
>> reviewed the bug this time around. It is essentially 681ce8623567 ("vfs:
>>
>> Delete the associated dentry when deleting a file") with opt-in of course.
>>
>>
>>> This patch introduces the concept conditionally, where the associated
>>> dentry is deleted only when the user explicitly opts for it during file
>>> removal. A new sysctl fs.automated_deletion_of_dentry is added for this
>>> purpose. Its default value is set to 0.
>> I meant to update Documentation/admin-guide/sysctl/fs.rst to also say that
>>
>> setting dir-stale-max to 0 disables it. I also get the impression you might
>>
>> feel better about this if the default was 0 as well.
>>
>>
>> The thing that I don't much like with the d_delete() approach is that it
>>
>> fails to cater for files that have been closed and are otherwise unused
>>
>> who's dentries make there way to the LRU eventually resulting in the bad
>>
>> behaviour being discussed.
>>
>>
>>> I have no massive objections to your approach. It feels a bit hacky tbh
>>> as it seems to degrade performance for new workloads in favor old
>>> workloads. The LRU should sort this out though.
>> My aim was to improve performance so I'm a bit puzzled by the comment.
>>
>>
>> The problem is the sheer number of dentry objects and the consequences
>>
>> of that. Hash table chains growing will affect performance, umounting
>>
>> the mount will take ages, and there are cases of child dentry traversals
>>
>> in the VFS. Once you get a large number of stale dentries that necessarily
>>
>> need to stay linked into the structures to get the benefit of caching your
>>
>> exposed to this problem.
>>
>>
>> The LRU mechanism is so far unable to cope with this.
> I meant when when you start limiting the number of negative dentries and
> start to not accumulate more that some workload was relying on this. It
> was just a theoretical musing. That initial "delete on unlink" thing led
> to regressions for a bunch of workloads when we did it unconditionally
> initially.

Yes, I did take that on board.

It's trivial to initially disable it with a doc update describng this.


But this only works (gets accepted) if people think it's worth while and

I'm not sure my proposed case is convincing enough.


Ian