[RFC PATCH 1/6] fs/9p: Add ability to identify inode by path for .L

[Tingmao Wang]

On 4/6/25 21:43, Tingmao Wang wrote:
> [...]
>  
> +struct iget_data {
> +	struct p9_stat_dotl *st;
> +	struct dentry *dentry;
> +};
> +
>  static int v9fs_test_inode_dotl(struct inode *inode, void *data)
>  {
>  	struct v9fs_inode *v9inode = V9FS_I(inode);
> -	struct p9_stat_dotl *st = (struct p9_stat_dotl *)data;
> +	struct p9_stat_dotl *st = ((struct iget_data *)data)->st;
> +	struct dentry *dentry = ((struct iget_data *)data)->dentry;
> +	struct v9fs_session_info *v9ses = v9fs_inode2v9ses(inode);
>  
>  	/* don't match inode of different type */
>  	if (inode_wrong_type(inode, st->st_mode))
> @@ -74,22 +81,74 @@ static int v9fs_test_inode_dotl(struct inode *inode, void *data)
>  
>  	if (v9inode->qid.path != st->qid.path)
>  		return 0;
> +
> +	if (v9fs_inode_ident_path(v9ses)) {
> +		if (!ino_path_compare(v9inode->path, dentry)) {
> +			p9_debug(P9_DEBUG_VFS, "Refusing to reuse inode %p based on path mismatch",
> +				 inode);
> +			return 0;
> +		}
> +	}
>  	return 1;
>  }
>  
>  /* Always get a new inode */>  static int v9fs_test_new_inode_dotl(struct inode *inode, void *data)

Looking back, this function should probably be renamed to something like
"v9fs_test_inode_uncached" since it now no longer "always get a new
inode".

Actually, maybe this should be merged with v9fs_test_inode_dotl - the
behavior is basically the same when inodeident=path is enabled.  Maybe the
approach could be that v9fs always re-use inodes (as long as qid matches,
and when inodeident=path, the path matches as well), but if in uncached
mode, it will also always refresh metadata?  (Basically inodes has to be
re-used, even in uncached mode, for Landlock and Fanotify using inode
marks to work)

Doing so does mean that if one sets inodeident=none, in a pathological
9pfs where different file/dirs have same qids, the inode will mistakenly
be re-used (like before be2ca38253 (Revert "fs/9p: simplify iget to remove
unnecessary paths")), but given that the user has specifically set
inodeident=none (i.e. not the default as proposed in this patch), I wonder
if this is acceptable behaviour?

>  {
> +	struct v9fs_inode *v9inode = V9FS_I(inode);
> +	struct p9_stat_dotl *st = ((struct iget_data *)data)->st;
> +	struct dentry *dentry = ((struct iget_data *)data)->dentry;
> +	struct v9fs_session_info *v9ses = v9fs_inode2v9ses(inode);
> +
> +	/*
> +	 * Don't reuse inode of different type, even if we have
> +	 * inodeident=path and path matches.
> +	 */
> +	if (inode_wrong_type(inode, st->st_mode))
> +		return 0;
> +
> +	/*
> +	 * We're only getting here if QID2INO stays the same anyway, so
> +	 * mirroring the qid checks in v9fs_test_inode_dotl
> +	 * (but maybe that check is unnecessary anyway? at least on 64bit)
> +	 */
> +
> +	if (v9inode->qid.type != st->qid.type)
> +		return 0;
> +
> +	if (v9inode->qid.path != st->qid.path)
> +		return 0;
> +
> +	if (v9fs_inode_ident_path(v9ses) && dentry && v9inode->path) {
> +		if (ino_path_compare(V9FS_I(inode)->path, dentry)) {
> +			p9_debug(P9_DEBUG_VFS,
> +				 "Reusing inode %p based on path match", inode);
> +			return 1;
> +		}
> +	}
> +
>  	return 0;
>  }
>  
>  static int v9fs_set_inode_dotl(struct inode *inode,  void *data)
>  {
>  	struct v9fs_inode *v9inode = V9FS_I(inode);
> -	struct p9_stat_dotl *st = (struct p9_stat_dotl *)data;
> +	struct v9fs_session_info *v9ses = v9fs_inode2v9ses(inode);
> +	struct iget_data *idata = data;
> +	struct p9_stat_dotl *st = idata->st;
> +	struct dentry *dentry = idata->dentry;
>  
>  	memcpy(&v9inode->qid, &st->qid, sizeof(st->qid));
>  	inode->i_generation = st->st_gen;
> +	if (v9fs_inode_ident_path(v9ses)) {
> +		if (dentry) {
> +			v9inode->path = make_ino_path(dentry);
> +			if (!v9inode->path)
> +				return -ENOMEM;
> +		} else {
> +			v9inode->path = NULL;
> +		}
> +	}

I realized that this leaves v9inode->path uninitialized if
inodeident=none.  The proper way is

	v9inode->path = NULL;
	if (v9fs_inode_ident_path(v9ses) && dentry) {
		v9inode->path = make_ino_path(dentry);
		if (!v9inode->path)
			return -ENOMEM;
	}

Same change applies for the non-.L version.

>  	return 0;
>  }
>  
> [...]