On Wed, 2020-02-26 at 14:28 -0700, Andreas Dilger wrote:

On Feb 26, 2020, at 9:29 AM, Matthew Wilcox [off-list ref]
wrote:

quoted

On Wed, Feb 26, 2020 at 11:13:53AM -0500, Waiman Long wrote:

quoted

A new sysctl parameter "dentry-dir-max" is introduced which
accepts a
value of 0 (default) for no limit or a positive integer 256 and
up. Small
dentry-dir-max numbers are forbidden to avoid excessive dentry
count
checking which can impact system performance.

This is always the wrong approach.  A sysctl is just a way of
blaming
the sysadmin for us not being very good at programming.

I agree that we need a way to limit the number of negative
dentries.
But that limit needs to be dynamic and depend on how the system is
being
used, not on how some overworked sysadmin has configured it.

So we need an initial estimate for the number of negative dentries
that
we need for good performance.  Maybe it's 1000.  It doesn't really
matter;
it's going to change dynamically.

Then we need a metric to let us know whether it needs to be
increased.
Perhaps that's "number of new negative dentries created in the last
second".  And we need to decide how much to increase it; maybe it's
by
50% or maybe by 10%.  Perhaps somewhere between 10-100% depending
on
how high the recent rate of negative dentry creation has been.

We also need a metric to let us know whether it needs to be
decreased.
I'm reluctant to say that memory pressure should be that metric
because
very large systems can let the number of dentries grow in an
unbounded
way.  Perhaps that metric is "number of hits in the negative dentry
cache in the last ten seconds".  Again, we'll need to decide how
much
to shrink the target number by.

OK, so now instead of a single tunable parameter we need three,
because
these numbers are totally made up and nobody knows the right values.
:-)
Defaulting the limit to "disabled/no limit" also has the problem that
99.99% of users won't even know this tunable exists, let alone how to
set it correctly, so they will continue to see these problems, and
the
code may as well not exist (i.e. pure overhead), while Waiman has a
better idea today of what would be reasonable defaults.

Why have these at all.

Not all file systems even produce negative hashed dentries.

The most beneficial use of them is to improve performance of rapid
fire lookups for non-existent names. Longer lived negative hashed
dentries don't give much benefit at all unless they suddenly have
lots of hits and that would cost a single allocation on the first
lookup if the dentry ttl expired and the dentry discarded.

A ttl (say jiffies) set at appropriate times could be a better
choice all round, no sysctl values at all.

Ian