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

[Christian Schoenebeck]

On Friday, 27 February 2026 08:56:53 CET Remi Pommarel wrote:
> Introduce a new mount option, ndentrycache, for v9fs that allows users
> to specify how long negative dentries are retained in the cache. The
> retention time can be set in milliseconds (e.g. ndentrycache=10000 for
> a 10secs retention time), or negative entries can be kept until the
> buffer cache management removes them by using the option without a
> value (i.e. ndentrycache).
> 
> For consistency reasons, this option should only be used in exclusive
> or read-only mount scenarios, aligning with the cache=loose usage.
> 
> Signed-off-by: Remi Pommarel <repk@xxxxxxxxxxxx>
> ---
>  fs/9p/v9fs.c | 73 ++++++++++++++++++++++++++++++++--------------------
>  fs/9p/v9fs.h | 23 ++++++++++-------
>  2 files changed, 58 insertions(+), 38 deletions(-)
> 
> diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c
> index c5dca81a553e..a26bd9070786 100644
> --- a/fs/9p/v9fs.c
> +++ b/fs/9p/v9fs.c
> @@ -39,11 +39,12 @@ enum {
>  	 * source if we rejected it as EINVAL */
>  	Opt_source,
>  	/* Options that take integer arguments */
> -	Opt_debug, Opt_dfltuid, Opt_dfltgid, Opt_afid,
> +	Opt_debug, Opt_dfltuid, Opt_dfltgid, Opt_afid, Opt_ndentrycachetmo,
>  	/* String options */
>  	Opt_uname, Opt_remotename, Opt_cache, Opt_cachetag,
>  	/* Options that take no arguments */
>  	Opt_nodevmap, Opt_noxattr, Opt_directio, Opt_ignoreqv,
> +	Opt_ndentrycache,
>  	/* Access options */
>  	Opt_access, Opt_posixacl,
>  	/* Lock timeout option */
> @@ -77,41 +78,43 @@ static const struct constant_table p9_versions[] = {
>   * the client, and all the transports.
>   */
>  const struct fs_parameter_spec v9fs_param_spec[] = {
> -	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_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.

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.

/Christian
 
>  	/* client options */
> -	fsparam_u32	("msize",	Opt_msize),
> -	fsparam_flag	("noextend",	Opt_legacy),
> -	fsparam_string	("trans",	Opt_trans),
> -	fsparam_enum	("version",	Opt_version, p9_versions),
> +	fsparam_u32	("msize",		Opt_msize),
> +	fsparam_flag	("noextend",		Opt_legacy),
> +	fsparam_string	("trans",		Opt_trans),
> +	fsparam_enum	("version",		Opt_version, p9_versions),
> 
>  	/* fd transport options */
> -	fsparam_u32	("rfdno",	Opt_rfdno),
> -	fsparam_u32	("wfdno",	Opt_wfdno),
> +	fsparam_u32	("rfdno",		Opt_rfdno),
> +	fsparam_u32	("wfdno",		Opt_wfdno),
> 
>  	/* rdma transport options */
> -	fsparam_u32	("sq",		Opt_sq_depth),
> -	fsparam_u32	("rq",		Opt_rq_depth),
> -	fsparam_u32	("timeout",	Opt_timeout),
> +	fsparam_u32	("sq",			Opt_sq_depth),
> +	fsparam_u32	("rq",			Opt_rq_depth),
> +	fsparam_u32	("timeout",		Opt_timeout),
> 
>  	/* fd and rdma transprt options */
> -	fsparam_u32	("port",	Opt_port),
> -	fsparam_flag	("privport",	Opt_privport),
> +	fsparam_u32	("port",		Opt_port),
> +	fsparam_flag	("privport",		Opt_privport),
>  	{}
>  };
> 
> @@ -159,6 +162,10 @@ int v9fs_show_options(struct seq_file *m, struct dentry
> *root) from_kgid_munged(&init_user_ns, v9ses->dfltgid));
>  	if (v9ses->afid != ~0)
>  		seq_printf(m, ",afid=%u", v9ses->afid);
> +	if (v9ses->ndentry_timeout_ms == NDENTRY_TMOUT_NEVER)
> +		seq_printf(m, ",ndentrycache");
> +	else if (v9ses->flags & V9FS_NDENTRY_TMOUT_SET)
> +		seq_printf(m, ",ndentrycache=%u", v9ses->ndentry_timeout_ms);
>  	if (strcmp(v9ses->uname, V9FS_DEFUSER) != 0)
>  		seq_printf(m, ",uname=%s", v9ses->uname);
>  	if (strcmp(v9ses->aname, V9FS_DEFANAME) != 0)
> @@ -337,6 +344,16 @@ int v9fs_parse_param(struct fs_context *fc, struct
> fs_parameter *param) session_opts->session_lock_timeout =
> (long)result.uint_32 * HZ; break;
> 
> +	case Opt_ndentrycache:
> +		session_opts->flags |= V9FS_NDENTRY_TMOUT_SET;
> +		session_opts->ndentry_timeout_ms = NDENTRY_TMOUT_NEVER;
> +		break;
> +
> +	case Opt_ndentrycachetmo:
> +		session_opts->flags |= V9FS_NDENTRY_TMOUT_SET;
> +		session_opts->ndentry_timeout_ms = result.uint_64;
> +		break;
> +
>  	/* Options for client */
>  	case Opt_msize:
>  		if (result.uint_32 < 4096) {
> diff --git a/fs/9p/v9fs.h b/fs/9p/v9fs.h
> index 8410f7883109..2e42729c6c20 100644
> --- a/fs/9p/v9fs.h
> +++ b/fs/9p/v9fs.h
> @@ -24,6 +24,8 @@
>   * @V9FS_ACCESS_ANY: use a single attach for all users
>   * @V9FS_ACCESS_MASK: bit mask of different ACCESS options
>   * @V9FS_POSIX_ACL: POSIX ACLs are enforced
> + * @V9FS_NDENTRY_TMOUT_SET: Has negative dentry timeout retention time been
> + *			    overriden by ndentrycache mount option
>   *
>   * Session flags reflect options selected by users at mount time
>   */
> @@ -34,16 +36,17 @@
>  #define V9FS_ACL_MASK V9FS_POSIX_ACL
> 
>  enum p9_session_flags {
> -	V9FS_PROTO_2000U    = 0x01,
> -	V9FS_PROTO_2000L    = 0x02,
> -	V9FS_ACCESS_SINGLE  = 0x04,
> -	V9FS_ACCESS_USER    = 0x08,
> -	V9FS_ACCESS_CLIENT  = 0x10,
> -	V9FS_POSIX_ACL      = 0x20,
> -	V9FS_NO_XATTR       = 0x40,
> -	V9FS_IGNORE_QV      = 0x80, /* ignore qid.version for cache hints */
> -	V9FS_DIRECT_IO      = 0x100,
> -	V9FS_SYNC           = 0x200
> +	V9FS_PROTO_2000U       = 0x01,
> +	V9FS_PROTO_2000L       = 0x02,
> +	V9FS_ACCESS_SINGLE     = 0x04,
> +	V9FS_ACCESS_USER       = 0x08,
> +	V9FS_ACCESS_CLIENT     = 0x10,
> +	V9FS_POSIX_ACL         = 0x20,
> +	V9FS_NO_XATTR          = 0x40,
> +	V9FS_IGNORE_QV         = 0x80, /* ignore qid.version for cache hints */
> +	V9FS_DIRECT_IO         = 0x100,
> +	V9FS_SYNC              = 0x200,
> +	V9FS_NDENTRY_TMOUT_SET = 0x400,
>  };
> 
>  /**