Re: [PATCH v4 0/5] Fix easy bits of the negative dentry problem

[NeilBrown]

On Tue, 20 Oct 2026, Ian Kent wrote:
> On Mon, 2026-10-19 at 13:29 +1100, NeilBrown wrote:
> > On Mon, 19 Oct 2026, Ian Kent wrote:
> > 
> > > 
> > > As is so often the case you've come up with an interesting and
> > > useful
> > > proposal, ;)
> > > 
> > > Nothing stood to me out as problematic given the scope of the
> > > series.
> > 
> > Thanks :-)
> > 
> > 
> > > 
> > > > 
> > > > 
> > > >  [PATCH v4 1/5] VFS: don't count references through ->d_parent
> > > >  [PATCH v4 2/5] fsnotify: update children flags only when adding
> > > > mask
> > > >  [PATCH v4 3/5] fsnotify: don't hold i_lock while walking
> > > > d_children
> > > >  [PATCH v4 4/5] Add d_for_each_positive_child family of iterators
> > > >  [PATCH v4 5/5] Use d_for_each_positive_child() in fsnotify.
> > > 
> > > Did you have any thoughts on how to contain the number of negative
> > > dentries, (perhaps) including directory entry counts, in an
> > > intelligent
> > > way (meaning "as you go" rather than "brute force" when needed)?
> > > 
> > > A significant problem, particularly on systems that count their
> > > memory
> > > in TBs rather than GBs.
> > 
> > I would need to understand *why* this is a significant problem, which
> > as
> > yet I don't.
> 
> I think there is something more needed to understand the problem.
> 
> The source of my concern was originally the traversal of the entire
> list of child directory dentries in the notification code to set a
> flag that depended on a parent dentry setting.

And why this is a problem?
If it holds a lock for extended period, that is bad.  My patches address
that.
If it causes a noticeable delay for users, then that might be bad.
I think I can fix that, but I would like a report of "task X takes 5
seconds which is a problem because ...."

> 
> From the information I got (which is usually somewhat limited) it
> was obvious the number of dentries was very large and there was a
> large proportion of negative dentries. The obvious assumption is
> that the implicated list traversal was causing the lock held too
> long messages due to the number of entries. There was evidence in
> the vmcore analysis the proportion negative dentries in the child
> list was unusually large too, similar to the proportion of system
> negative dentries.
> 
> I should be able to grab the vmcore again and get more details if
> that would be useful to you.

No, I don't think the vmcore would help thanks.  What I want is user
experience reports which describe a symptom which is problematic.
Saying "there are too mean negative dentries" is, but itself,
uninteresting.  Saying "When I do X it takes a long time" is much more
interesting.

> 
> While the unbounded accumulation of negative dentries has often
> appeared to be a problem, and the case Miklos and I were looking
> at implicated that, I also thought that traversing the entire list
> of children to set a flag based on the parent dentry was not good
> design. After mentioning that Jan replied (wrt. using the parent
> dentry instead of setting a flag on the child) saying dget_parent()
> was high overhead. I let that go even though I didn't understand
> why dget_parent() was needed. Using dget_parent() would only ensure
> the parent dentry didn't disappear from under you (it wouldn't
> protect you from deletes or renames) but dentries are rcu managed
> so an rcu_read_lock() should do the same.

I think the point is that adding/removing fsnotify watches is relatively
rare, and testing if a watch is present is extremely common.  I wouldn't
say that dget_parent() is the problem, I would say that dereferencing
->d_parent at all is the problem.  Almost always there are no fsnotify
watchers, and we need to be able to know that with information that is
already is a cacheline.  So a flag in the dentry is the right thing.  If
it takes a long time to update the flag, then that is a price worth
paying.  If we can make the update faster, then maybe that is worth
while.

> 
> So what am I saying, I'm saying that I think the list traversal
> that was causing this particular problem can be eliminated. But
> that doesn't do anything to come up with a mechanism to better
> manage negative dentry accumulation which I think would be better
> done as part of the dentry life cycle than as a shrink operation.
> That's just my preference.

It isn't clear to me that we need to manage negative dentry
accumulation.  But I'd be happy to see some evidence.

> 
> > Allocating things that don't get used much isn't intrinsically a
> > problem.  We have a shrinker which will gradually removed them - or
> > less
> > gradually when memory is short.
> 
> But if there are Terabytes of memory it often doesn't become in
> short supply.

So why does the large number of dentries matter?  Genuine question.


> 
> This might be able to be better managed by updated or possibly
> expanded settings based more on number of (negative) dentries
> than memory size.
> 
> TBH I haven't looked at that aspect.
> 
> > 
> > But maybe shrinker activity itself is a problem?  Is that it?
> 
> Mostly only in so much as it not happening.
> 
> Problems with shrinker operation don't come up as much as they
> used to so I don't see that as a problem myself.
> 
> > 
> > I think it could make sense to separate positive and negative
> > dentries
> > on the d_children list.  This is quite do-able, though there is a
> > small
> > cost when a dentry transitions from one to the other.
> 
> Not sure about that.
> 
> One thing a negative dentry needs to do is maintain it's
> linkages in directories and such so it can be located on
> lookup to effect caching benefit. So separate lists might
> be cumbersome and possibly problematic.

dentries are on lots of lists.

They are on the d_hash list from the hash table, and this allows the
name to be found in the parent directory.
They are on d_children which is mostly used for freeing up children
when the directory is removed from cache.  It is also used
occasionally for readdir and for a few things like the fsnotify usage.

Neither of these would be affected by using the d_lru list in a new way.
There are currently two sorts of lists a dentry can be on via d_lru.
Adding a third that is specific for negative dentries would be easy and
low impact.

> 
> > 
> > The only other thing I remember from the lwn article and the various
> > other articles and emails it referenced, was something about poor LRU
> > performance.  Lots of largely useless negative dentries push more
> > useful
> > positive dentries to the end of the LRU and we lose them too soon.
> > I think that can be addressed by having separate LRUs for positive
> > and
> > negative dentries, and giving them different weights.  I think the
> > cost of this would be tiny, but I'm not confident of the benefit.
> 
> Or maybe adding negative dentries to the end and positives to the
> front might be simpler.

An LRU must have most recently used at one end and least recently used at
the other, or it isn't an LRU.  Treating negative dentries as not
recently used would be wrong.

> 
> A ttl might be useful too, use it to discard stale entries as they
> are encountered (although the discard policy would probably be
> somewhat more involved taking into account positive vs. negatives
> etc.).
> 
> This in line with my preference to mange these "on-the-go" so to
> speak, not necessarily related to any functional parameter
> improvement that might be possible.

I think the big disconnect between your thinking and mine is that you
see a problem, deduce that it is caused by a large number of negative
dentries, and try to manage the large number of negative dentries.

I see the same problem, deduce that it is caused by holding a lock when
there is no need to hold the lock, and I rearrange the code so that the
lock isn't held.

Neither approach is intrinsically "right" or "wrong", but one has not
been addressed after several years, and the other is addressed with a
handful of patches.

Thanks,
NeilBrown