Re: [PATCH 1/9] fs/9p: future-proof qid2ino 32-bit support

From: Eric Van Hensbergen <ericvh@kernel.org>
To: v9fs@lists.linux.dev
Cc: Christian Schoenebeck <linux_oss@crudebyte.com>,
	Dominique Martinet <asmadeus@codewreck.org>,
	 Ron Minnich <rminnich@gmail.com>,
	Latchesar Ionkov <lucho@ionkov.net>
Subject: Re: [PATCH 1/9] fs/9p: future-proof qid2ino 32-bit support
Date: Fri, 5 Jan 2024 22:01:35 -0600	[thread overview]
Message-ID: <CAFkjPTn4aFieUDkN-cysxjSk8VSi+dUdffS=gbEu8LaSESMkmw@mail.gmail.com> (raw)
In-Reply-To: <20240106-ericvh-fix-cache-dups-v1-1-538c2074f363@kernel.org>

Ooops -- realized after I sent the series out that this one can be
dropped since I completely switch up the format of qid2ino to a macro
in a future patch.

On Fri, Jan 5, 2024 at 8:11 PM Eric Van Hensbergen <ericvh@kernel.org> wrote:
>
> The qid2ino code had a check to allow a reduced precision qid to inode number mapping, but it only checked if the sizes were
> equal.  Change to check if the ino_t is smaller than qid->path
> before using the reduced precision version.
>
> Signed-off-by: Eric Van Hensbergen <ericvh@kernel.org>
> ---
>  fs/9p/vfs_inode.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
> index b845ee18a80b..6d149ba12bc0 100644
> --- a/fs/9p/vfs_inode.c
> +++ b/fs/9p/vfs_inode.c
> @@ -1192,18 +1192,25 @@ v9fs_stat2inode(struct p9_wstat *stat, struct inode *inode,
>   * v9fs_qid2ino - convert qid into inode number
>   * @qid: qid to hash
>   *
> - * BUG: potential for inode number collisions?
> + *  We add 2 to qid->path to keep reserved inode
> + *  numbers reserved.
> + *
> + *  Its possible in the future we will make qid->path
> + *  just a hash to lookup inodes, but this breaks too
> + *  much right now.
> + *
>   */
>
>  ino_t v9fs_qid2ino(struct p9_qid *qid)
>  {
> -       u64 path = qid->path + 2;
>         ino_t i = 0;
>
> -       if (sizeof(ino_t) == sizeof(path))
> -               memcpy(&i, &path, sizeof(ino_t));
> +       WARN_ON((qid->path+2)==0);
> +
> +       if (sizeof(ino_t) < sizeof(qid->path))
> +               i = (ino_t) ((qid->path+2) ^ (qid->path >> 32));
>         else
> -               i = (ino_t) (path ^ (path >> 32));
> +               i = (ino_t) qid->path;
>
>         return i;
>  }
>
> --
> 2.41.0
>