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

[Remi Pommarel <repk@triplefau.lt>]

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.

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

-- 
Remi