Re: [PATCH v3 2/4] 9p: Add mount option for negative dentry cache retention

[Christian Schoenebeck]

On Wednesday, 4 March 2026 09:25:27 CET Remi Pommarel wrote:
> On Wed, Mar 04, 2026 at 06:45:35AM +0900, Dominique Martinet wrote:
> > Christian Schoenebeck wrote on Tue, Mar 03, 2026 at 03:53:47PM +0100:
> > > > +	fsparam_string	("source",		Opt_source),
> > > > +	fsparam_u32hex	("debug",		Opt_debug),
> > > > +	fsparam_uid	("dfltuid",		Opt_dfltuid),
> > > > +	fsparam_gid	("dfltgid",		Opt_dfltgid),
> > > > +	fsparam_u32	("afid",		Opt_afid),
> > > > +	fsparam_string	("uname",		Opt_uname),
> > > > +	fsparam_string	("aname",		Opt_remotename),
> > > > +	fsparam_flag	("nodevmap",		Opt_nodevmap),
> > > > +	fsparam_flag	("noxattr",		Opt_noxattr),
> > > > +	fsparam_flag	("directio",		Opt_directio),
> > > > +	fsparam_flag	("ignoreqv",		Opt_ignoreqv),
> > > > +	fsparam_string	("cache",		Opt_cache),
> > > > +	fsparam_string	("cachetag",		Opt_cachetag),
> > > > +	fsparam_string	("access",		Opt_access),
> > > > +	fsparam_flag	("posixacl",		Opt_posixacl),
> > > > +	fsparam_u32	("locktimeout",		Opt_locktimeout),
> > > > +	fsparam_flag	("ndentrycache",	Opt_ndentrycache),
> > > > +	fsparam_u32	("ndentrycache",	Opt_ndentrycachetmo),
> > > 
> > > That double entry is surprising. So this mount option is supposed to be
> > > used like ndentrycache=n for a specific timeout value (in ms) and just
> > > ndentrycache (without any assignment) for infinite timeout. That's a
> > > bit weird.
> Yes I have seen this used in several other fs (see init_itable mount
> option for ext4fs or compress one for btrfs). I do agree that is a bit
> weird but this allow the whole 32bit range for timeout.
> 
> > Could make it a s32 and say <0 means infinite? I think we have that
> > somewhere
> 
> I did that on previous version, but was afraid that ~20days timeout max
> value may be too restrictive?
> 
> I do agree that this is a bit odd though and if you both think s32 is
> better that is fine with me.

What about just making this mount option a string and doing the parsing on our
end? That would have the benefit of simply allowing arguments like "i00s",
"5d", "1y", and if you really wanted "inf".

I would find units for this much more useful in practice than allowing
infinite. Like discussed before, it is in general a bad idea to configure
negative dentries to persist for good due to the huge amount of bogus entries
that pile up.

> > > Documentation/filesystems/9p.rst should be updated as well BTW.
> > > 
> > > Nevertheless, like mentioned before, I really think the string "timeout"
> > > should be used, at least in a user visible mount option. Keep in mind
> > > that
> > > timeouts are a common issue to look at, so it is common to just grep for
> > > "timeout" in a code base or documentation. An abbrevation like "tmo" or
> > > leaving it out entirely is for me therefore IMHO inappropriate.
> > > 
> > > You found "ndentrycachetimeout" too horribly long, or was that again
> > > just
> > > motivated by the code indention below? I personally find those indention
> > > alignments completely irrelevant, not sure how Dominique sees that.
> > > Personally I avoid them, as they cost unnecessary time on git blame.
> > 
> > I rarely use blame at all and it's possible to ignore whitespaces for
> > blame, but I'd tend to agree here, I don't care if this stays aligned.
> > 
> > OTOH ndentrycachetimeout as a mount option is a mouthful,
> > negativetimeout or negtimeout sounds clear enough to me?
> > I can't think of anything else that'd be negative related
> > to timeouts, but perhaps it's the lack of sleep speaking
> 
> No strong opinion on the option name though so any name that suits you
> is alse fine by me.

Another suggestion: "ndtimeout"?

/Christian