mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Ian Kent <raven@themaw.net>
To: NeilBrown <neil@brown.name>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>,
	Christian Brauner	 <brauner@kernel.org>, Jan Kara <jack@suse.cz>,
	Amir Goldstein <amir73il@gmail.com>,
		linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	Jake Edge	 <jake@lwn.net>
Subject: Re: [PATCH v4 0/5] Fix easy bits of the negative dentry problem
Date: Tue, 20 Oct 2026 09:29:31 +0800	[thread overview]
Message-ID: <d0bc6eaac2fc84cb43b569f49ee503057784b45c.camel@themaw.net> (raw)
In-Reply-To: <179237697885.1724252.11956785709205194840@noble.neil.brown.name>

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.

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.

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.

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.

> 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.

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.

> 
> 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.

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.


Ian

      reply	other threads:[~2026-10-20  1:29 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-19  0:00 NeilBrown
2026-10-19  0:00 ` [PATCH v4 1/5] VFS: don't count references through ->d_parent NeilBrown
2026-10-19  0:00 ` [PATCH v4 2/5] fsnotify: update children flags only when adding mask bits NeilBrown
2026-10-19  0:00 ` [PATCH v4 3/5] fsnotify: don't hold i_lock while walking d_children in fsnotify_set_children_dentry_flags() NeilBrown
2026-10-19  0:00 ` [PATCH v4 4/5] Add d_for_each_positive_child family of iterators NeilBrown
2026-10-19  0:00 ` [PATCH v4 5/5] Use d_for_each_positive_child() in fsnotify NeilBrown
2026-10-19  1:59 ` [PATCH v4 0/5] Fix easy bits of the negative dentry problem Ian Kent
2026-10-19  2:29   ` NeilBrown
2026-10-20  1:29     ` Ian Kent [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d0bc6eaac2fc84cb43b569f49ee503057784b45c.camel@themaw.net \
    --to=raven@themaw.net \
    --cc=amir73il@gmail.com \
    --cc=brauner@kernel.org \
    --cc=jack@suse.cz \
    --cc=jake@lwn.net \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=neil@brown.name \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®