Re: [PATCH v6 0/7] fs/dcache: Track & limit # of negative dentries

[Waiman Long <longman@redhat.com>]

On 07/16/2018 07:40 PM, Andrew Morton wrote:
> On Mon, 16 Jul 2018 05:41:15 -0700 Matthew Wilcox <willy@infradead.org> wrote:
>
>> On Mon, Jul 16, 2018 at 11:09:01AM +0200, Michal Hocko wrote:
>>> On Fri 13-07-18 10:36:14, Dave Chinner wrote:
>>> [...]
>>>> By limiting the number of negative dentries in this case, internal
>>>> slab fragmentation is reduced such that reclaim cost never gets out
>>>> of control. While it appears to "fix" the symptoms, it doesn't
>>>> address the underlying problem. It is a partial solution at best but
>>>> at worst it's another opaque knob that nobody knows how or when to
>>>> tune.
>>> Would it help to put all the negative dentries into its own slab cache?
>> Maybe the dcache should be more sensitive to its own needs.  In __d_alloc,
>> it could check whether there are a high proportion of negative dentries
>> and start recycling some existing negative dentries.
> Well, yes.
>
> The proposed patchset adds all this background reclaiming.  Problem is
> a) that background reclaiming sometimes can't keep up so a synchronous
> direct-reclaim was added on top and b) reclaiming dentries in the
> background will cause non-dentry-allocating tasks to suffer because of
> activity from the dentry-allocating tasks, which is inappropriate.

I have taken out the background reclaiming in the latest v7 patch for
the concern people have on duplicating the reclaim effort. We can always
add it back on later on if we want to.

> I expect a better design is something like
>
> __d_alloc()
> {
> 	...
> 	while (too many dentries)
> 		call the dcache shrinker
> 	...
> }
>
> and that's it.  This way we have a hard upper limit and only the tasks
> which are creating dentries suffer the cost.

Yes, that is certainly one way of doing it.

Cheers,
Longman