Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Date:   Tue, 5 Apr 2022 22:18:01 +0100
From:   Matthew Wilcox <willy@infradead.org>
To:     Stephen Brennan <stephen.s.brennan@oracle.com>
Cc:     Dave Chinner <david@fromorbit.com>,
        Hillf Danton <hdanton@sina.com>,
        Roman Gushchin <roman.gushchin@linux.dev>,
        MM <linux-mm@kvack.org>,
        Mel Gorman <mgorman@techsingularity.net>,
        Yu Zhao <yuzhao@google.com>,
        David Hildenbrand <david@redhat.com>,
        LKML <linux-kernel@vger.kernel.org>
Subject: Re: [RFC] mm/vmscan: add periodic slab shrinker
Message-ID: <YkyyCcdJq69tO6ba@casper.infradead.org>
References: <20220402072103.5140-1-hdanton@sina.com>
 <20220403005618.5263-1-hdanton@sina.com>
 <20220404010948.GV1609613@dread.disaster.area>
 <87ilrn5ttl.fsf@stepbren-lnx.us.oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <87ilrn5ttl.fsf@stepbren-lnx.us.oracle.com>
Precedence: bulk

On Tue, Apr 05, 2022 at 10:22:14AM -0700, Stephen Brennan wrote:
> I can't speak for every slab cache, but I've been coming to the same
> conclusion myself regarding the dentry cache. I think that the rate of
> stepping through the LRU should be tied to the rate of allocations.
> Truly in-use objects shouldn't be harmed by this, as they should get
> referenced and rotated to the beginning of the LRU. But the one-offs
> which are bloating the cache will be found and removed.

I agree with all this.

> I've implemented a version of this patch which just takes one step
> through the LRU on each d_alloc. It's quite interesting to watch it
> work. You can create 5 million negative dentries in directory /A via
> stat(), and then create 5 million negative dentries in directory /B. The
> total dentry slab size reaches 5 million but never goes past it, since
> the old negative dentries from /A aren't really in use, and they get
> pruned at the same rate as negative dentries from /B get created. On the
> other hand, if you *continue* to stat() on the dentries of /A while you
> create negative dentries in /B, then the cache grows to 10 million,
> since the /A dentries are actually in use.
> 
> Maybe a solution could involve some generic list_lru machinery that can
> let you do these sorts of incremental scans? Maybe batching them up so
> instead of doing one every allocation, you do them every 100 or 1000?
> It would still be up to the individual user to put this to good use in
> the object allocation path.

I feel like we need to allow the list to both shrink and grow, depending
on how useful the entries in it are.  So one counter per LRU, incremented
on every add.  When that counter gets to 100, reset it to 0 and scan
110 entries.  Maybe 0 of them can be reclaimed; maybe 110 of them can be.
But the list can shrink over time instead of being a "one in, one out"
scenario.

Clearly 110 is a magic number, but intuitively, attempting to shrink
by 10% feels reasonable.  Need to strike a balance between shrinking
quickly enough and giving the cache time to figure out which entries
are actually useful.