* [PATCH 1/9] fs/9p: future-proof qid2ino 32-bit support
2024-01-06 2:11 [PATCH 0/9] fs/9p: simplify inode lookup operations Eric Van Hensbergen
@ 2024-01-06 2:11 ` Eric Van Hensbergen
2024-01-06 4:01 ` Eric Van Hensbergen
2024-01-06 2:11 ` [PATCH 2/9] fs/9p: switch vfsmount to use v9fs_get_new_inode Eric Van Hensbergen
` (7 subsequent siblings)
8 siblings, 1 reply; 13+ messages in thread
From: Eric Van Hensbergen @ 2024-01-06 2:11 UTC (permalink / raw)
To: v9fs; +Cc: Eric Van Hensbergen, linux_oss, asmadeus, rminnich, lucho
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 1/9] fs/9p: future-proof qid2ino 32-bit support
2024-01-06 2:11 ` [PATCH 1/9] fs/9p: future-proof qid2ino 32-bit support Eric Van Hensbergen
@ 2024-01-06 4:01 ` Eric Van Hensbergen
0 siblings, 0 replies; 13+ messages in thread
From: Eric Van Hensbergen @ 2024-01-06 4:01 UTC (permalink / raw)
To: v9fs
Cc: Christian Schoenebeck, Dominique Martinet, Ron Minnich,
Latchesar Ionkov
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
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/9] fs/9p: switch vfsmount to use v9fs_get_new_inode
2024-01-06 2:11 [PATCH 0/9] fs/9p: simplify inode lookup operations Eric Van Hensbergen
2024-01-06 2:11 ` [PATCH 1/9] fs/9p: future-proof qid2ino 32-bit support Eric Van Hensbergen
@ 2024-01-06 2:11 ` Eric Van Hensbergen
2024-01-06 2:11 ` [PATCH 3/9] fs/9p: convert mkdir to use get_new_inode Eric Van Hensbergen
` (6 subsequent siblings)
8 siblings, 0 replies; 13+ messages in thread
From: Eric Van Hensbergen @ 2024-01-06 2:11 UTC (permalink / raw)
To: v9fs; +Cc: Eric Van Hensbergen, linux_oss, asmadeus, rminnich, lucho
In the process of cleaning up inode number allocation, I noticed several functions which didn't use the standard helper
allocators. This patch fixes the allocation in the mount entrypoint.
Signed-off-by: Eric Van Hensbergen <ericvh@kernel.org>
---
fs/9p/vfs_super.c | 29 +----------------------------
1 file changed, 1 insertion(+), 28 deletions(-)
diff --git a/fs/9p/vfs_super.c b/fs/9p/vfs_super.c
index 73db55c050bf..8d14cc0b3916 100644
--- a/fs/9p/vfs_super.c
+++ b/fs/9p/vfs_super.c
@@ -110,7 +110,6 @@ static struct dentry *v9fs_mount(struct file_system_type *fs_type, int flags,
struct inode *inode = NULL;
struct dentry *root = NULL;
struct v9fs_session_info *v9ses = NULL;
- umode_t mode = 0777 | S_ISVTX;
struct p9_fid *fid;
int retval = 0;
@@ -140,7 +139,7 @@ static struct dentry *v9fs_mount(struct file_system_type *fs_type, int flags,
else
sb->s_d_op = &v9fs_dentry_operations;
- inode = v9fs_get_inode(sb, S_IFDIR | mode, 0);
+ inode = v9fs_get_new_inode_from_fid(v9ses, fid, sb);
if (IS_ERR(inode)) {
retval = PTR_ERR(inode);
goto release_sb;
@@ -152,32 +151,6 @@ static struct dentry *v9fs_mount(struct file_system_type *fs_type, int flags,
goto release_sb;
}
sb->s_root = root;
- if (v9fs_proto_dotl(v9ses)) {
- struct p9_stat_dotl *st = NULL;
-
- st = p9_client_getattr_dotl(fid, P9_STATS_BASIC);
- if (IS_ERR(st)) {
- retval = PTR_ERR(st);
- goto release_sb;
- }
- d_inode(root)->i_ino = v9fs_qid2ino(&st->qid);
- v9fs_stat2inode_dotl(st, d_inode(root), 0);
- kfree(st);
- } else {
- struct p9_wstat *st = NULL;
-
- st = p9_client_stat(fid);
- if (IS_ERR(st)) {
- retval = PTR_ERR(st);
- goto release_sb;
- }
-
- d_inode(root)->i_ino = v9fs_qid2ino(&st->qid);
- v9fs_stat2inode(st, d_inode(root), sb, 0);
-
- p9stat_free(st);
- kfree(st);
- }
retval = v9fs_get_acl(inode, fid);
if (retval)
goto release_sb;
--
2.41.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 3/9] fs/9p: convert mkdir to use get_new_inode
2024-01-06 2:11 [PATCH 0/9] fs/9p: simplify inode lookup operations Eric Van Hensbergen
2024-01-06 2:11 ` [PATCH 1/9] fs/9p: future-proof qid2ino 32-bit support Eric Van Hensbergen
2024-01-06 2:11 ` [PATCH 2/9] fs/9p: switch vfsmount to use v9fs_get_new_inode Eric Van Hensbergen
@ 2024-01-06 2:11 ` Eric Van Hensbergen
2024-01-06 2:11 ` [PATCH 4/9] fs/9p: remove walk and inode allocation from symlink Eric Van Hensbergen
` (5 subsequent siblings)
8 siblings, 0 replies; 13+ messages in thread
From: Eric Van Hensbergen @ 2024-01-06 2:11 UTC (permalink / raw)
To: v9fs; +Cc: Eric Van Hensbergen, linux_oss, asmadeus, rminnich, lucho
mkdir had different code paths for inode creation, cache used
the get_new_inode_from_fid helper, but non-cached used
v9fs_get_inode. Collapsed into a single implementation across
both as there should be no difference.
Signed-off-by: Eric Van Hensbergen <ericvh@kernel.org>
---
fs/9p/vfs_inode_dotl.c | 35 ++++++++++-------------------------
1 file changed, 10 insertions(+), 25 deletions(-)
diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
index c7319af2f471..981278d0788e 100644
--- a/fs/9p/vfs_inode_dotl.c
+++ b/fs/9p/vfs_inode_dotl.c
@@ -401,32 +401,17 @@ static int v9fs_vfs_mkdir_dotl(struct mnt_idmap *idmap,
}
/* instantiate inode and assign the unopened fid to the dentry */
- if (v9ses->cache & (CACHE_META|CACHE_LOOSE)) {
- inode = v9fs_get_new_inode_from_fid(v9ses, fid, dir->i_sb);
- if (IS_ERR(inode)) {
- err = PTR_ERR(inode);
- p9_debug(P9_DEBUG_VFS, "inode creation failed %d\n",
- err);
- goto error;
- }
- v9fs_fid_add(dentry, &fid);
- v9fs_set_create_acl(inode, fid, dacl, pacl);
- d_instantiate(dentry, inode);
- err = 0;
- } else {
- /*
- * Not in cached mode. No need to populate
- * inode with stat. We need to get an inode
- * so that we can set the acl with dentry
- */
- inode = v9fs_get_inode(dir->i_sb, mode, 0);
- if (IS_ERR(inode)) {
- err = PTR_ERR(inode);
- goto error;
- }
- v9fs_set_create_acl(inode, fid, dacl, pacl);
- d_instantiate(dentry, inode);
+ inode = v9fs_get_new_inode_from_fid(v9ses, fid, dir->i_sb);
+ if (IS_ERR(inode)) {
+ err = PTR_ERR(inode);
+ p9_debug(P9_DEBUG_VFS, "inode creation failed %d\n",
+ err);
+ goto error;
}
+ v9fs_fid_add(dentry, &fid);
+ v9fs_set_create_acl(inode, fid, dacl, pacl);
+ d_instantiate(dentry, inode);
+ err = 0;
inc_nlink(dir);
v9fs_invalidate_inode_attr(dir);
error:
--
2.41.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 4/9] fs/9p: remove walk and inode allocation from symlink
2024-01-06 2:11 [PATCH 0/9] fs/9p: simplify inode lookup operations Eric Van Hensbergen
` (2 preceding siblings ...)
2024-01-06 2:11 ` [PATCH 3/9] fs/9p: convert mkdir to use get_new_inode Eric Van Hensbergen
@ 2024-01-06 2:11 ` Eric Van Hensbergen
2024-01-06 2:11 ` [PATCH 5/9] fs/9p: Eliminate redundant non-cache path in mknod Eric Van Hensbergen
` (4 subsequent siblings)
8 siblings, 0 replies; 13+ messages in thread
From: Eric Van Hensbergen @ 2024-01-06 2:11 UTC (permalink / raw)
To: v9fs; +Cc: Eric Van Hensbergen, linux_oss, asmadeus, rminnich, lucho
Symlink had a bunch of extra operations which essentially
end up discarded. It was walking the fid to the new file and
creating an inode for it, but those semantics are part of
tsymlink. This did prepopulate the cache, but that also seems
potentially unnecessary and frought with peril.
Signed-off-by: Eric Van Hensbergen <ericvh@kernel.org>
---
fs/9p/vfs_inode_dotl.c | 31 -------------------------------
1 file changed, 31 deletions(-)
diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
index 981278d0788e..c435952f6355 100644
--- a/fs/9p/vfs_inode_dotl.c
+++ b/fs/9p/vfs_inode_dotl.c
@@ -690,7 +690,6 @@ v9fs_vfs_symlink_dotl(struct mnt_idmap *idmap, struct inode *dir,
kgid_t gid;
const unsigned char *name;
struct p9_qid qid;
- struct inode *inode;
struct p9_fid *dfid;
struct p9_fid *fid = NULL;
struct v9fs_session_info *v9ses;
@@ -717,36 +716,6 @@ v9fs_vfs_symlink_dotl(struct mnt_idmap *idmap, struct inode *dir,
}
v9fs_invalidate_inode_attr(dir);
- if (v9ses->cache & (CACHE_META|CACHE_LOOSE)) {
- /* Now walk from the parent so we can get an unopened fid. */
- fid = p9_client_walk(dfid, 1, &name, 1);
- if (IS_ERR(fid)) {
- err = PTR_ERR(fid);
- p9_debug(P9_DEBUG_VFS, "p9_client_walk failed %d\n",
- err);
- goto error;
- }
-
- /* instantiate inode and assign the unopened fid to dentry */
- inode = v9fs_get_new_inode_from_fid(v9ses, fid, dir->i_sb);
- if (IS_ERR(inode)) {
- err = PTR_ERR(inode);
- p9_debug(P9_DEBUG_VFS, "inode creation failed %d\n",
- err);
- goto error;
- }
- v9fs_fid_add(dentry, &fid);
- d_instantiate(dentry, inode);
- err = 0;
- } else {
- /* Not in cached mode. No need to populate inode with stat */
- inode = v9fs_get_inode(dir->i_sb, S_IFLNK, 0);
- if (IS_ERR(inode)) {
- err = PTR_ERR(inode);
- goto error;
- }
- d_instantiate(dentry, inode);
- }
error:
p9_fid_put(fid);
--
2.41.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 5/9] fs/9p: Eliminate redundant non-cache path in mknod
2024-01-06 2:11 [PATCH 0/9] fs/9p: simplify inode lookup operations Eric Van Hensbergen
` (3 preceding siblings ...)
2024-01-06 2:11 ` [PATCH 4/9] fs/9p: remove walk and inode allocation from symlink Eric Van Hensbergen
@ 2024-01-06 2:11 ` Eric Van Hensbergen
2024-01-06 2:11 ` [PATCH 6/9] fs/9p: Eliminate now unused v9fs_get_inode Eric Van Hensbergen
` (3 subsequent siblings)
8 siblings, 0 replies; 13+ messages in thread
From: Eric Van Hensbergen @ 2024-01-06 2:11 UTC (permalink / raw)
To: v9fs; +Cc: Eric Van Hensbergen, linux_oss, asmadeus, rminnich, lucho
Like symlink, mknod had a seperate path with different inode
allocation -- but this seems unnecessary, so eliminating this path.
Signed-off-by: Eric Van Hensbergen <ericvh@kernel.org>
---
fs/9p/vfs_inode_dotl.c | 36 ++++++++++--------------------------
1 file changed, 10 insertions(+), 26 deletions(-)
diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
index c435952f6355..8eee13bae887 100644
--- a/fs/9p/vfs_inode_dotl.c
+++ b/fs/9p/vfs_inode_dotl.c
@@ -838,33 +838,17 @@ v9fs_vfs_mknod_dotl(struct mnt_idmap *idmap, struct inode *dir,
err);
goto error;
}
-
- /* instantiate inode and assign the unopened fid to the dentry */
- if (v9ses->cache & (CACHE_META|CACHE_LOOSE)) {
- inode = v9fs_get_new_inode_from_fid(v9ses, fid, dir->i_sb);
- if (IS_ERR(inode)) {
- err = PTR_ERR(inode);
- p9_debug(P9_DEBUG_VFS, "inode creation failed %d\n",
- err);
- goto error;
- }
- v9fs_set_create_acl(inode, fid, dacl, pacl);
- v9fs_fid_add(dentry, &fid);
- d_instantiate(dentry, inode);
- err = 0;
- } else {
- /*
- * Not in cached mode. No need to populate inode with stat.
- * socket syscall returns a fd, so we need instantiate
- */
- inode = v9fs_get_inode(dir->i_sb, mode, rdev);
- if (IS_ERR(inode)) {
- err = PTR_ERR(inode);
- goto error;
- }
- v9fs_set_create_acl(inode, fid, dacl, pacl);
- d_instantiate(dentry, inode);
+ inode = v9fs_get_new_inode_from_fid(v9ses, fid, dir->i_sb);
+ if (IS_ERR(inode)) {
+ err = PTR_ERR(inode);
+ p9_debug(P9_DEBUG_VFS, "inode creation failed %d\n",
+ err);
+ goto error;
}
+ v9fs_set_create_acl(inode, fid, dacl, pacl);
+ v9fs_fid_add(dentry, &fid);
+ d_instantiate(dentry, inode);
+ err = 0;
error:
p9_fid_put(fid);
v9fs_put_acl(dacl, pacl);
--
2.41.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 6/9] fs/9p: Eliminate now unused v9fs_get_inode
2024-01-06 2:11 [PATCH 0/9] fs/9p: simplify inode lookup operations Eric Van Hensbergen
` (4 preceding siblings ...)
2024-01-06 2:11 ` [PATCH 5/9] fs/9p: Eliminate redundant non-cache path in mknod Eric Van Hensbergen
@ 2024-01-06 2:11 ` Eric Van Hensbergen
2024-01-06 2:11 ` [PATCH 7/9] fs/9p: rework qid2ino logic Eric Van Hensbergen
` (2 subsequent siblings)
8 siblings, 0 replies; 13+ messages in thread
From: Eric Van Hensbergen @ 2024-01-06 2:11 UTC (permalink / raw)
To: v9fs; +Cc: Eric Van Hensbergen, linux_oss, asmadeus, rminnich, lucho
Now with all inode allocation going through get_from_fid
functions we can remove v9fs_get_inode and reduce us down
to a single inode allocation path.
Signed-off-by: Eric Van Hensbergen <ericvh@kernel.org>
---
fs/9p/v9fs_vfs.h | 2 --
fs/9p/vfs_inode.c | 29 -----------------------------
2 files changed, 31 deletions(-)
diff --git a/fs/9p/v9fs_vfs.h b/fs/9p/v9fs_vfs.h
index 731e3d14b67d..ad0310deb6c8 100644
--- a/fs/9p/v9fs_vfs.h
+++ b/fs/9p/v9fs_vfs.h
@@ -40,8 +40,6 @@ extern struct kmem_cache *v9fs_inode_cache;
struct inode *v9fs_alloc_inode(struct super_block *sb);
void v9fs_free_inode(struct inode *inode);
-struct inode *v9fs_get_inode(struct super_block *sb, umode_t mode,
- dev_t rdev);
int v9fs_init_inode(struct v9fs_session_info *v9ses,
struct inode *inode, umode_t mode, dev_t rdev);
void v9fs_evict_inode(struct inode *inode);
diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
index 6d149ba12bc0..02761009946b 100644
--- a/fs/9p/vfs_inode.c
+++ b/fs/9p/vfs_inode.c
@@ -333,35 +333,6 @@ int v9fs_init_inode(struct v9fs_session_info *v9ses,
}
-/**
- * v9fs_get_inode - helper function to setup an inode
- * @sb: superblock
- * @mode: mode to setup inode with
- * @rdev: The device numbers to set
- */
-
-struct inode *v9fs_get_inode(struct super_block *sb, umode_t mode, dev_t rdev)
-{
- int err;
- struct inode *inode;
- struct v9fs_session_info *v9ses = sb->s_fs_info;
-
- p9_debug(P9_DEBUG_VFS, "super block: %p mode: %ho\n", sb, mode);
-
- inode = new_inode(sb);
- if (!inode) {
- pr_warn("%s (%d): Problem allocating inode\n",
- __func__, task_pid_nr(current));
- return ERR_PTR(-ENOMEM);
- }
- err = v9fs_init_inode(v9ses, inode, mode, rdev);
- if (err) {
- iput(inode);
- return ERR_PTR(err);
- }
- return inode;
-}
-
/**
* v9fs_evict_inode - Remove an inode from the inode cache
* @inode: inode to release
--
2.41.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 7/9] fs/9p: rework qid2ino logic
2024-01-06 2:11 [PATCH 0/9] fs/9p: simplify inode lookup operations Eric Van Hensbergen
` (5 preceding siblings ...)
2024-01-06 2:11 ` [PATCH 6/9] fs/9p: Eliminate now unused v9fs_get_inode Eric Van Hensbergen
@ 2024-01-06 2:11 ` Eric Van Hensbergen
2024-01-08 11:28 ` asmadeus
2024-01-06 2:11 ` [PATCH 8/9] fs/9p: simplify iget path to remove unnecessary paths Eric Van Hensbergen
2024-01-06 2:11 ` [PATCH 9/9] fs/9p: Further simplify inode lookup Eric Van Hensbergen
8 siblings, 1 reply; 13+ messages in thread
From: Eric Van Hensbergen @ 2024-01-06 2:11 UTC (permalink / raw)
To: v9fs; +Cc: Eric Van Hensbergen, linux_oss, asmadeus, rminnich, lucho
This changes from a function to a macro because we can
figure out if we are 32 or 64 bit at compile time.
Signed-off-by: Eric Van Hensbergen <ericvh@kernel.org>
---
fs/9p/v9fs_vfs.h | 7 ++++++-
fs/9p/vfs_dir.c | 6 +++---
fs/9p/vfs_inode.c | 31 +------------------------------
fs/9p/vfs_inode_dotl.c | 6 ++----
4 files changed, 12 insertions(+), 38 deletions(-)
diff --git a/fs/9p/v9fs_vfs.h b/fs/9p/v9fs_vfs.h
index ad0310deb6c8..789e1188d5dc 100644
--- a/fs/9p/v9fs_vfs.h
+++ b/fs/9p/v9fs_vfs.h
@@ -43,7 +43,12 @@ void v9fs_free_inode(struct inode *inode);
int v9fs_init_inode(struct v9fs_session_info *v9ses,
struct inode *inode, umode_t mode, dev_t rdev);
void v9fs_evict_inode(struct inode *inode);
-ino_t v9fs_qid2ino(struct p9_qid *qid);
+#if (ULONG_MAX == 0xffffffffUL)
+#define QID2INO(q) (ino_t) (((q)->path+2) ^ (((q)->path) >> 32))
+#else
+#define QID2INO(q) (ino_t) ((q)->path+2)
+#endif
+
void v9fs_stat2inode(struct p9_wstat *stat, struct inode *inode,
struct super_block *sb, unsigned int flags);
void v9fs_stat2inode_dotl(struct p9_stat_dotl *stat, struct inode *inode,
diff --git a/fs/9p/vfs_dir.c b/fs/9p/vfs_dir.c
index 4102759a5cb5..13a53da75ec8 100644
--- a/fs/9p/vfs_dir.c
+++ b/fs/9p/vfs_dir.c
@@ -125,9 +125,9 @@ static int v9fs_dir_readdir(struct file *file, struct dir_context *ctx)
p9_debug(P9_DEBUG_VFS, "returned %d\n", err);
return -EIO;
}
-
+
over = !dir_emit(ctx, st.name, strlen(st.name),
- v9fs_qid2ino(&st.qid), dt_type(&st));
+ QID2INO(&st.qid), dt_type(&st));
p9stat_free(&st);
if (over)
return 0;
@@ -184,7 +184,7 @@ static int v9fs_dir_readdir_dotl(struct file *file, struct dir_context *ctx)
if (!dir_emit(ctx, curdirent.d_name,
strlen(curdirent.d_name),
- v9fs_qid2ino(&curdirent.qid),
+ QID2INO(&curdirent.qid),
curdirent.d_type))
return 0;
diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
index 02761009946b..fe8cbcdf4b5f 100644
--- a/fs/9p/vfs_inode.c
+++ b/fs/9p/vfs_inode.c
@@ -407,7 +407,6 @@ static struct inode *v9fs_qid_iget(struct super_block *sb,
dev_t rdev;
int retval;
umode_t umode;
- unsigned long i_ino;
struct inode *inode;
struct v9fs_session_info *v9ses = sb->s_fs_info;
int (*test)(struct inode *inode, void *data);
@@ -417,8 +416,7 @@ static struct inode *v9fs_qid_iget(struct super_block *sb,
else
test = v9fs_test_inode;
- i_ino = v9fs_qid2ino(qid);
- inode = iget5_locked(sb, i_ino, test, v9fs_set_inode, st);
+ inode = iget5_locked(sb, QID2INO(qid), test, v9fs_set_inode, st);
if (!inode)
return ERR_PTR(-ENOMEM);
if (!(inode->i_state & I_NEW))
@@ -428,7 +426,6 @@ static struct inode *v9fs_qid_iget(struct super_block *sb,
* FIXME!! we may need support for stale inodes
* later.
*/
- inode->i_ino = i_ino;
umode = p9mode2unixmode(v9ses, st, &rdev);
retval = v9fs_init_inode(v9ses, inode, umode, rdev);
if (retval)
@@ -1159,32 +1156,6 @@ v9fs_stat2inode(struct p9_wstat *stat, struct inode *inode,
v9inode->cache_validity &= ~V9FS_INO_INVALID_ATTR;
}
-/**
- * v9fs_qid2ino - convert qid into inode number
- * @qid: qid to hash
- *
- * 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)
-{
- ino_t i = 0;
-
- 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) qid->path;
-
- return i;
-}
/**
* v9fs_vfs_get_link - follow a symlink path
diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
index 8eee13bae887..2699d7b3b8e8 100644
--- a/fs/9p/vfs_inode_dotl.c
+++ b/fs/9p/vfs_inode_dotl.c
@@ -100,7 +100,6 @@ static struct inode *v9fs_qid_iget_dotl(struct super_block *sb,
int new)
{
int retval;
- unsigned long i_ino;
struct inode *inode;
struct v9fs_session_info *v9ses = sb->s_fs_info;
int (*test)(struct inode *inode, void *data);
@@ -110,8 +109,7 @@ static struct inode *v9fs_qid_iget_dotl(struct super_block *sb,
else
test = v9fs_test_inode_dotl;
- i_ino = v9fs_qid2ino(qid);
- inode = iget5_locked(sb, i_ino, test, v9fs_set_inode_dotl, st);
+ inode = iget5_locked(sb, QID2INO(qid), test, v9fs_set_inode_dotl, st);
if (!inode)
return ERR_PTR(-ENOMEM);
if (!(inode->i_state & I_NEW))
@@ -121,7 +119,7 @@ static struct inode *v9fs_qid_iget_dotl(struct super_block *sb,
* FIXME!! we may need support for stale inodes
* later.
*/
- inode->i_ino = i_ino;
+ inode->i_ino = QID2INO(qid);
retval = v9fs_init_inode(v9ses, inode,
st->st_mode, new_decode_dev(st->st_rdev));
if (retval)
--
2.41.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 7/9] fs/9p: rework qid2ino logic
2024-01-06 2:11 ` [PATCH 7/9] fs/9p: rework qid2ino logic Eric Van Hensbergen
@ 2024-01-08 11:28 ` asmadeus
2024-01-08 12:56 ` Eric Van Hensbergen
0 siblings, 1 reply; 13+ messages in thread
From: asmadeus @ 2024-01-08 11:28 UTC (permalink / raw)
To: Eric Van Hensbergen; +Cc: v9fs, linux_oss, rminnich, lucho
Eric Van Hensbergen wrote on Sat, Jan 06, 2024 at 02:11:14AM +0000:
> This changes from a function to a macro because we can
> figure out if we are 32 or 64 bit at compile time.
>
> Signed-off-by: Eric Van Hensbergen <ericvh@kernel.org>
Couple of remarks on this one.
patches 2-6 so far all looked good to me.
> ---
> fs/9p/v9fs_vfs.h | 7 ++++++-
> fs/9p/vfs_dir.c | 6 +++---
> fs/9p/vfs_inode.c | 31 +------------------------------
> fs/9p/vfs_inode_dotl.c | 6 ++----
> 4 files changed, 12 insertions(+), 38 deletions(-)
>
> diff --git a/fs/9p/v9fs_vfs.h b/fs/9p/v9fs_vfs.h
> index ad0310deb6c8..789e1188d5dc 100644
> --- a/fs/9p/v9fs_vfs.h
> +++ b/fs/9p/v9fs_vfs.h
> @@ -43,7 +43,12 @@ void v9fs_free_inode(struct inode *inode);
> int v9fs_init_inode(struct v9fs_session_info *v9ses,
> struct inode *inode, umode_t mode, dev_t rdev);
> void v9fs_evict_inode(struct inode *inode);
> -ino_t v9fs_qid2ino(struct p9_qid *qid);
> +#if (ULONG_MAX == 0xffffffffUL)
The standard preprocessor condition in the kernel for this is checking
BITS_PER_LONG, e.g.
include/asm-generic/bitops/fls64.h
18:#if BITS_PER_LONG == 32
26:#elif BITS_PER_LONG == 64
33:#else
34:#error BITS_PER_LONG not 32 or 64
35:#endif
> +#define QID2INO(q) (ino_t) (((q)->path+2) ^ (((q)->path) >> 32))
> +#else
> +#define QID2INO(q) (ino_t) ((q)->path+2)
> +#endif
> +
> void v9fs_stat2inode(struct p9_wstat *stat, struct inode *inode,
> struct super_block *sb, unsigned int flags);
> void v9fs_stat2inode_dotl(struct p9_stat_dotl *stat, struct inode *inode,
> diff --git a/fs/9p/vfs_dir.c b/fs/9p/vfs_dir.c
> index 4102759a5cb5..13a53da75ec8 100644
> --- a/fs/9p/vfs_dir.c
> +++ b/fs/9p/vfs_dir.c
> @@ -125,9 +125,9 @@ static int v9fs_dir_readdir(struct file *file, struct dir_context *ctx)
> p9_debug(P9_DEBUG_VFS, "returned %d\n", err);
> return -EIO;
> }
> -
> +
stray white space?
> over = !dir_emit(ctx, st.name, strlen(st.name),
> - v9fs_qid2ino(&st.qid), dt_type(&st));
> + QID2INO(&st.qid), dt_type(&st));
> p9stat_free(&st);
> if (over)
> return 0;
> @@ -184,7 +184,7 @@ static int v9fs_dir_readdir_dotl(struct file *file, struct dir_context *ctx)
>
> if (!dir_emit(ctx, curdirent.d_name,
> strlen(curdirent.d_name),
> - v9fs_qid2ino(&curdirent.qid),
> + QID2INO(&curdirent.qid),
> curdirent.d_type))
> return 0;
>
> diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
> index 02761009946b..fe8cbcdf4b5f 100644
> --- a/fs/9p/vfs_inode.c
> +++ b/fs/9p/vfs_inode.c
> @@ -407,7 +407,6 @@ static struct inode *v9fs_qid_iget(struct super_block *sb,
> dev_t rdev;
> int retval;
> umode_t umode;
> - unsigned long i_ino;
> struct inode *inode;
> struct v9fs_session_info *v9ses = sb->s_fs_info;
> int (*test)(struct inode *inode, void *data);
> @@ -417,8 +416,7 @@ static struct inode *v9fs_qid_iget(struct super_block *sb,
> else
> test = v9fs_test_inode;
>
> - i_ino = v9fs_qid2ino(qid);
> - inode = iget5_locked(sb, i_ino, test, v9fs_set_inode, st);
> + inode = iget5_locked(sb, QID2INO(qid), test, v9fs_set_inode, st);
> if (!inode)
> return ERR_PTR(-ENOMEM);
> if (!(inode->i_state & I_NEW))
> @@ -428,7 +426,6 @@ static struct inode *v9fs_qid_iget(struct super_block *sb,
> * FIXME!! we may need support for stale inodes
> * later.
> */
> - inode->i_ino = i_ino;
This is gone, but v9fs_qid_iget_dotl() still sets i_ino -- didn't check
which is needed but we probably want to stay coherent between the two.
> umode = p9mode2unixmode(v9ses, st, &rdev);
> retval = v9fs_init_inode(v9ses, inode, umode, rdev);
> if (retval)
> @@ -1159,32 +1156,6 @@ v9fs_stat2inode(struct p9_wstat *stat, struct inode *inode,
> v9inode->cache_validity &= ~V9FS_INO_INVALID_ATTR;
> }
>
> -/**
> - * v9fs_qid2ino - convert qid into inode number
> - * @qid: qid to hash
> - *
> - * 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)
> -{
> - ino_t i = 0;
> -
> - 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) qid->path;
> -
> - return i;
> -}
>
> /**
> * v9fs_vfs_get_link - follow a symlink path
> diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
> index 8eee13bae887..2699d7b3b8e8 100644
> --- a/fs/9p/vfs_inode_dotl.c
> +++ b/fs/9p/vfs_inode_dotl.c
> @@ -100,7 +100,6 @@ static struct inode *v9fs_qid_iget_dotl(struct super_block *sb,
> int new)
> {
> int retval;
> - unsigned long i_ino;
> struct inode *inode;
> struct v9fs_session_info *v9ses = sb->s_fs_info;
> int (*test)(struct inode *inode, void *data);
> @@ -110,8 +109,7 @@ static struct inode *v9fs_qid_iget_dotl(struct super_block *sb,
> else
> test = v9fs_test_inode_dotl;
>
> - i_ino = v9fs_qid2ino(qid);
> - inode = iget5_locked(sb, i_ino, test, v9fs_set_inode_dotl, st);
> + inode = iget5_locked(sb, QID2INO(qid), test, v9fs_set_inode_dotl, st);
> if (!inode)
> return ERR_PTR(-ENOMEM);
> if (!(inode->i_state & I_NEW))
> @@ -121,7 +119,7 @@ static struct inode *v9fs_qid_iget_dotl(struct super_block *sb,
> * FIXME!! we may need support for stale inodes
> * later.
> */
> - inode->i_ino = i_ino;
> + inode->i_ino = QID2INO(qid);
> retval = v9fs_init_inode(v9ses, inode,
> st->st_mode, new_decode_dev(st->st_rdev));
> if (retval)
>
--
Dominique Martinet | Asmadeus
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 7/9] fs/9p: rework qid2ino logic
2024-01-08 11:28 ` asmadeus
@ 2024-01-08 12:56 ` Eric Van Hensbergen
0 siblings, 0 replies; 13+ messages in thread
From: Eric Van Hensbergen @ 2024-01-08 12:56 UTC (permalink / raw)
To: asmadeus; +Cc: v9fs, linux_oss, rminnich, lucho
On Mon, Jan 08, 2024 at 08:28:15PM +0900, asmadeus@codewreck.org wrote:
> Eric Van Hensbergen wrote on Sat, Jan 06, 2024 at 02:11:14AM +0000:
> >
> > diff --git a/fs/9p/v9fs_vfs.h b/fs/9p/v9fs_vfs.h
> > index ad0310deb6c8..789e1188d5dc 100644
> > --- a/fs/9p/v9fs_vfs.h
> > +++ b/fs/9p/v9fs_vfs.h
> > @@ -43,7 +43,12 @@ void v9fs_free_inode(struct inode *inode);
> > int v9fs_init_inode(struct v9fs_session_info *v9ses,
> > struct inode *inode, umode_t mode, dev_t rdev);
> > void v9fs_evict_inode(struct inode *inode);
> > -ino_t v9fs_qid2ino(struct p9_qid *qid);
> > +#if (ULONG_MAX == 0xffffffffUL)
>
> The standard preprocessor condition in the kernel for this is checking
> BITS_PER_LONG, e.g.
> include/asm-generic/bitops/fls64.h
> 18:#if BITS_PER_LONG == 32
> 26:#elif BITS_PER_LONG == 64
> 33:#else
> 34:#error BITS_PER_LONG not 32 or 64
> 35:#endif
>
Thanks for that, will update to be consistent with rest of kernel.
> > -
> > +
>
> stray white space?
>
oops, will fix.
> > @@ -428,7 +426,6 @@ static struct inode *v9fs_qid_iget(struct super_block *sb,
> > * FIXME!! we may need support for stale inodes
> > * later.
> > */
> > - inode->i_ino = i_ino;
>
> This is gone, but v9fs_qid_iget_dotl() still sets i_ino -- didn't check
> which is needed but we probably want to stay coherent between the two.
>
Good catch, deleted it from .L (and later here but in a different patch)
because its largely vestigial since this gets set in iget_locked anyways.
Will fix up to be more consistent.
-eric
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 8/9] fs/9p: simplify iget path to remove unnecessary paths
2024-01-06 2:11 [PATCH 0/9] fs/9p: simplify inode lookup operations Eric Van Hensbergen
` (6 preceding siblings ...)
2024-01-06 2:11 ` [PATCH 7/9] fs/9p: rework qid2ino logic Eric Van Hensbergen
@ 2024-01-06 2:11 ` Eric Van Hensbergen
2024-01-06 2:11 ` [PATCH 9/9] fs/9p: Further simplify inode lookup Eric Van Hensbergen
8 siblings, 0 replies; 13+ messages in thread
From: Eric Van Hensbergen @ 2024-01-06 2:11 UTC (permalink / raw)
To: v9fs; +Cc: Eric Van Hensbergen, linux_oss, asmadeus, rminnich, lucho
Remove the additional comparison operators and switch to
simply lookup by inode number (aka qid.path).
Signed-off-by: Eric Van Hensbergen <ericvh@kernel.org>
---
fs/9p/v9fs_vfs.h | 2 +-
fs/9p/vfs_inode.c | 67 +++++++++++---------------------------------------
fs/9p/vfs_inode_dotl.c | 67 +++++++++-----------------------------------------
3 files changed, 27 insertions(+), 109 deletions(-)
diff --git a/fs/9p/v9fs_vfs.h b/fs/9p/v9fs_vfs.h
index 789e1188d5dc..791231b31b95 100644
--- a/fs/9p/v9fs_vfs.h
+++ b/fs/9p/v9fs_vfs.h
@@ -41,7 +41,7 @@ extern struct kmem_cache *v9fs_inode_cache;
struct inode *v9fs_alloc_inode(struct super_block *sb);
void v9fs_free_inode(struct inode *inode);
int v9fs_init_inode(struct v9fs_session_info *v9ses,
- struct inode *inode, umode_t mode, dev_t rdev);
+ struct inode *inode, struct p9_qid *qid, umode_t mode, dev_t rdev);
void v9fs_evict_inode(struct inode *inode);
#if (ULONG_MAX == 0xffffffffUL)
#define QID2INO(q) (ino_t) (((q)->path+2) ^ (((q)->path) >> 32))
diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
index fe8cbcdf4b5f..766496579b28 100644
--- a/fs/9p/vfs_inode.c
+++ b/fs/9p/vfs_inode.c
@@ -253,9 +253,12 @@ static void v9fs_set_netfs_context(struct inode *inode)
}
int v9fs_init_inode(struct v9fs_session_info *v9ses,
- struct inode *inode, umode_t mode, dev_t rdev)
+ struct inode *inode, struct p9_qid *qid, umode_t mode, dev_t rdev)
{
int err = 0;
+ struct v9fs_inode *v9inode = V9FS_I(inode);
+
+ memcpy(&v9inode->qid, qid, sizeof(struct p9_qid));
inode_init_owner(&nop_mnt_idmap, inode, NULL, mode);
inode->i_blocks = 0;
@@ -359,75 +362,33 @@ void v9fs_evict_inode(struct inode *inode)
#endif
}
-static int v9fs_test_inode(struct inode *inode, void *data)
-{
- int umode;
- dev_t rdev;
- struct v9fs_inode *v9inode = V9FS_I(inode);
- struct p9_wstat *st = (struct p9_wstat *)data;
- struct v9fs_session_info *v9ses = v9fs_inode2v9ses(inode);
-
- umode = p9mode2unixmode(v9ses, st, &rdev);
- /* don't match inode of different type */
- if (inode_wrong_type(inode, umode))
- return 0;
-
- /* compare qid details */
- if (memcmp(&v9inode->qid.version,
- &st->qid.version, sizeof(v9inode->qid.version)))
- return 0;
-
- if (v9inode->qid.type != st->qid.type)
- return 0;
-
- if (v9inode->qid.path != st->qid.path)
- return 0;
- return 1;
-}
-
-static int v9fs_test_new_inode(struct inode *inode, void *data)
-{
- return 0;
-}
-
-static int v9fs_set_inode(struct inode *inode, void *data)
-{
- struct v9fs_inode *v9inode = V9FS_I(inode);
- struct p9_wstat *st = (struct p9_wstat *)data;
-
- memcpy(&v9inode->qid, &st->qid, sizeof(st->qid));
- return 0;
-}
-
static struct inode *v9fs_qid_iget(struct super_block *sb,
struct p9_qid *qid,
- struct p9_wstat *st,
- int new)
+ struct p9_wstat *st)
{
dev_t rdev;
int retval;
umode_t umode;
struct inode *inode;
struct v9fs_session_info *v9ses = sb->s_fs_info;
- int (*test)(struct inode *inode, void *data);
- if (new)
- test = v9fs_test_new_inode;
- else
- test = v9fs_test_inode;
-
- inode = iget5_locked(sb, QID2INO(qid), test, v9fs_set_inode, st);
- if (!inode)
+ inode = iget_locked(sb, QID2INO(qid));
+ if (unlikely(!inode))
return ERR_PTR(-ENOMEM);
if (!(inode->i_state & I_NEW))
return inode;
+ if (unlikely(st == NULL)) {
+ retval = -EINVAL;
+ goto error;
+ }
+
/*
* initialize the inode with the stat info
* FIXME!! we may need support for stale inodes
* later.
*/
umode = p9mode2unixmode(v9ses, st, &rdev);
- retval = v9fs_init_inode(v9ses, inode, umode, rdev);
+ retval = v9fs_init_inode(v9ses, inode, qid, umode, rdev);
if (retval)
goto error;
@@ -452,7 +413,7 @@ v9fs_inode_from_fid(struct v9fs_session_info *v9ses, struct p9_fid *fid,
if (IS_ERR(st))
return ERR_CAST(st);
- inode = v9fs_qid_iget(sb, &st->qid, st, new);
+ inode = v9fs_qid_iget(sb, &st->qid, st);
p9stat_free(st);
kfree(st);
return inode;
diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
index 2699d7b3b8e8..2200c5f77d58 100644
--- a/fs/9p/vfs_inode_dotl.c
+++ b/fs/9p/vfs_inode_dotl.c
@@ -52,75 +52,31 @@ static kgid_t v9fs_get_fsgid_for_create(struct inode *dir_inode)
return current_fsgid();
}
-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;
-
- /* don't match inode of different type */
- if (inode_wrong_type(inode, st->st_mode))
- return 0;
-
- if (inode->i_generation != st->st_gen)
- return 0;
-
- /* compare qid details */
- if (memcmp(&v9inode->qid.version,
- &st->qid.version, sizeof(v9inode->qid.version)))
- return 0;
-
- if (v9inode->qid.type != st->qid.type)
- return 0;
-
- if (v9inode->qid.path != st->qid.path)
- return 0;
- return 1;
-}
-
-/* Always get a new inode */
-static int v9fs_test_new_inode_dotl(struct inode *inode, void *data)
-{
- 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;
-
- memcpy(&v9inode->qid, &st->qid, sizeof(st->qid));
- inode->i_generation = st->st_gen;
- return 0;
-}
-
static struct inode *v9fs_qid_iget_dotl(struct super_block *sb,
struct p9_qid *qid,
struct p9_fid *fid,
- struct p9_stat_dotl *st,
- int new)
+ struct p9_stat_dotl *st)
{
int retval;
struct inode *inode;
struct v9fs_session_info *v9ses = sb->s_fs_info;
- int (*test)(struct inode *inode, void *data);
-
- if (new)
- test = v9fs_test_new_inode_dotl;
- else
- test = v9fs_test_inode_dotl;
- inode = iget5_locked(sb, QID2INO(qid), test, v9fs_set_inode_dotl, st);
- if (!inode)
+ inode = iget_locked(sb, QID2INO(qid));
+ if (unlikely(!inode))
return ERR_PTR(-ENOMEM);
if (!(inode->i_state & I_NEW))
return inode;
+ if (unlikely(st == NULL)) {
+ retval = -EINVAL;
+ goto error;
+ }
+
/*
* initialize the inode with the stat info
* FIXME!! we may need support for stale inodes
* later.
*/
- inode->i_ino = QID2INO(qid);
- retval = v9fs_init_inode(v9ses, inode,
+ retval = v9fs_init_inode(v9ses, inode, qid,
st->st_mode, new_decode_dev(st->st_rdev));
if (retval)
goto error;
@@ -143,14 +99,15 @@ struct inode *
v9fs_inode_from_fid_dotl(struct v9fs_session_info *v9ses, struct p9_fid *fid,
struct super_block *sb, int new)
{
- struct p9_stat_dotl *st;
+ struct p9_stat_dotl *st = NULL;
struct inode *inode = NULL;
st = p9_client_getattr_dotl(fid, P9_STATS_BASIC | P9_STATS_GEN);
if (IS_ERR(st))
return ERR_CAST(st);
- inode = v9fs_qid_iget_dotl(sb, &st->qid, fid, st, new);
+ inode = v9fs_qid_iget_dotl(sb, &st->qid, fid, st);
+
kfree(st);
return inode;
}
--
2.41.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 9/9] fs/9p: Further simplify inode lookup
2024-01-06 2:11 [PATCH 0/9] fs/9p: simplify inode lookup operations Eric Van Hensbergen
` (7 preceding siblings ...)
2024-01-06 2:11 ` [PATCH 8/9] fs/9p: simplify iget path to remove unnecessary paths Eric Van Hensbergen
@ 2024-01-06 2:11 ` Eric Van Hensbergen
8 siblings, 0 replies; 13+ messages in thread
From: Eric Van Hensbergen @ 2024-01-06 2:11 UTC (permalink / raw)
To: v9fs; +Cc: Eric Van Hensbergen, linux_oss, asmadeus, rminnich, lucho
Collapse function call chain and shortcut helper functions where
they are no longer necessary. Move stat/getattr code into the iget
functions so that it only gets called when the inode doesn't exist
yet.
Signed-off-by: Eric Van Hensbergen <ericvh@kernel.org>
---
fs/9p/v9fs.h | 31 +++++--------------------------
fs/9p/vfs_inode.c | 46 +++++++++++++++-------------------------------
fs/9p/vfs_inode_dotl.c | 47 ++++++++++++++++-------------------------------
fs/9p/vfs_super.c | 2 +-
4 files changed, 37 insertions(+), 89 deletions(-)
diff --git a/fs/9p/v9fs.h b/fs/9p/v9fs.h
index 698c43dd5dc8..9defa12208f9 100644
--- a/fs/9p/v9fs.h
+++ b/fs/9p/v9fs.h
@@ -179,16 +179,13 @@ extern int v9fs_vfs_rename(struct mnt_idmap *idmap,
struct inode *old_dir, struct dentry *old_dentry,
struct inode *new_dir, struct dentry *new_dentry,
unsigned int flags);
-extern struct inode *v9fs_inode_from_fid(struct v9fs_session_info *v9ses,
- struct p9_fid *fid,
- struct super_block *sb, int new);
+extern struct inode *v9fs_fid_iget(struct super_block *sb, struct p9_fid *fid);
extern const struct inode_operations v9fs_dir_inode_operations_dotl;
extern const struct inode_operations v9fs_file_inode_operations_dotl;
extern const struct inode_operations v9fs_symlink_inode_operations_dotl;
extern const struct netfs_request_ops v9fs_req_ops;
-extern struct inode *v9fs_inode_from_fid_dotl(struct v9fs_session_info *v9ses,
- struct p9_fid *fid,
- struct super_block *sb, int new);
+extern struct inode *v9fs_fid_iget_dotl(struct super_block *sb,
+ struct p9_fid *fid);
/* other default globals */
#define V9FS_PORT 564
@@ -230,27 +227,9 @@ v9fs_get_inode_from_fid(struct v9fs_session_info *v9ses, struct p9_fid *fid,
struct super_block *sb)
{
if (v9fs_proto_dotl(v9ses))
- return v9fs_inode_from_fid_dotl(v9ses, fid, sb, 0);
+ return v9fs_fid_iget_dotl(sb, fid);
else
- return v9fs_inode_from_fid(v9ses, fid, sb, 0);
-}
-
-/**
- * v9fs_get_new_inode_from_fid - Helper routine to populate an inode by
- * issuing a attribute request
- * @v9ses: session information
- * @fid: fid to issue attribute request for
- * @sb: superblock on which to create inode
- *
- */
-static inline struct inode *
-v9fs_get_new_inode_from_fid(struct v9fs_session_info *v9ses, struct p9_fid *fid,
- struct super_block *sb)
-{
- if (v9fs_proto_dotl(v9ses))
- return v9fs_inode_from_fid_dotl(v9ses, fid, sb, 1);
- else
- return v9fs_inode_from_fid(v9ses, fid, sb, 1);
+ return v9fs_fid_iget(sb, fid);
}
#endif
diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
index 766496579b28..76e40c057c56 100644
--- a/fs/9p/vfs_inode.c
+++ b/fs/9p/vfs_inode.c
@@ -362,37 +362,40 @@ void v9fs_evict_inode(struct inode *inode)
#endif
}
-static struct inode *v9fs_qid_iget(struct super_block *sb,
- struct p9_qid *qid,
- struct p9_wstat *st)
+struct inode *v9fs_fid_iget(struct super_block *sb, struct p9_fid *fid)
{
dev_t rdev;
int retval;
umode_t umode;
struct inode *inode;
+ struct p9_wstat *st;
struct v9fs_session_info *v9ses = sb->s_fs_info;
- inode = iget_locked(sb, QID2INO(qid));
+ inode = iget_locked(sb, QID2INO(&fid->qid));
if (unlikely(!inode))
return ERR_PTR(-ENOMEM);
if (!(inode->i_state & I_NEW))
return inode;
- if (unlikely(st == NULL)) {
- retval = -EINVAL;
- goto error;
- }
/*
* initialize the inode with the stat info
* FIXME!! we may need support for stale inodes
* later.
*/
+ st = p9_client_stat(fid);
+ if (IS_ERR(st)) {
+ retval= PTR_ERR(st);
+ goto error;
+ }
+
umode = p9mode2unixmode(v9ses, st, &rdev);
- retval = v9fs_init_inode(v9ses, inode, qid, umode, rdev);
+ retval = v9fs_init_inode(v9ses, inode, &fid->qid, umode, rdev);
+ v9fs_stat2inode(st, inode, sb, 0);
+ p9stat_free(st);
+ kfree(st);
if (retval)
goto error;
- v9fs_stat2inode(st, inode, sb, 0);
v9fs_cache_inode_get_cookie(inode);
unlock_new_inode(inode);
return inode;
@@ -402,23 +405,6 @@ static struct inode *v9fs_qid_iget(struct super_block *sb,
}
-struct inode *
-v9fs_inode_from_fid(struct v9fs_session_info *v9ses, struct p9_fid *fid,
- struct super_block *sb, int new)
-{
- struct p9_wstat *st;
- struct inode *inode = NULL;
-
- st = p9_client_stat(fid);
- if (IS_ERR(st))
- return ERR_CAST(st);
-
- inode = v9fs_qid_iget(sb, &st->qid, st);
- p9stat_free(st);
- kfree(st);
- return inode;
-}
-
/**
* v9fs_at_to_dotl_flags- convert Linux specific AT flags to
* plan 9 AT flag.
@@ -565,7 +551,7 @@ v9fs_create(struct v9fs_session_info *v9ses, struct inode *dir,
/*
* instantiate inode and assign the unopened fid to the dentry
*/
- inode = v9fs_get_new_inode_from_fid(v9ses, fid, dir->i_sb);
+ inode = v9fs_get_inode_from_fid(v9ses, fid, dir->i_sb);
if (IS_ERR(inode)) {
err = PTR_ERR(inode);
p9_debug(P9_DEBUG_VFS,
@@ -693,10 +679,8 @@ struct dentry *v9fs_vfs_lookup(struct inode *dir, struct dentry *dentry,
inode = NULL;
else if (IS_ERR(fid))
inode = ERR_CAST(fid);
- else if (v9ses->cache & (CACHE_META|CACHE_LOOSE))
+ else
inode = v9fs_get_inode_from_fid(v9ses, fid, dir->i_sb);
- else
- inode = v9fs_get_new_inode_from_fid(v9ses, fid, dir->i_sb);
/*
* If we had a rename on the server and a parallel lookup
* for the new name, then make sure we instantiate with
diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
index 2200c5f77d58..6f8cdbcef444 100644
--- a/fs/9p/vfs_inode_dotl.c
+++ b/fs/9p/vfs_inode_dotl.c
@@ -52,32 +52,33 @@ static kgid_t v9fs_get_fsgid_for_create(struct inode *dir_inode)
return current_fsgid();
}
-static struct inode *v9fs_qid_iget_dotl(struct super_block *sb,
- struct p9_qid *qid,
- struct p9_fid *fid,
- struct p9_stat_dotl *st)
+struct inode *v9fs_fid_iget_dotl(struct super_block *sb, struct p9_fid *fid)
{
int retval;
struct inode *inode;
+ struct p9_stat_dotl *st;
struct v9fs_session_info *v9ses = sb->s_fs_info;
- inode = iget_locked(sb, QID2INO(qid));
+ inode = iget_locked(sb, QID2INO(&fid->qid));
if (unlikely(!inode))
return ERR_PTR(-ENOMEM);
if (!(inode->i_state & I_NEW))
return inode;
- if (unlikely(st == NULL)) {
- retval = -EINVAL;
- goto error;
- }
/*
* initialize the inode with the stat info
* FIXME!! we may need support for stale inodes
* later.
*/
- retval = v9fs_init_inode(v9ses, inode, qid,
+ st = p9_client_getattr_dotl(fid, P9_STATS_BASIC | P9_STATS_GEN);
+ if (IS_ERR(st)) {
+ retval = PTR_ERR(st);
+ goto error;
+ }
+
+ retval = v9fs_init_inode(v9ses, inode, &fid->qid,
st->st_mode, new_decode_dev(st->st_rdev));
+ kfree(st);
if (retval)
goto error;
@@ -87,7 +88,8 @@ static struct inode *v9fs_qid_iget_dotl(struct super_block *sb,
if (retval)
goto error;
- unlock_new_inode(inode);
+ unlock_new_inode(inode);
+
return inode;
error:
iget_failed(inode);
@@ -95,23 +97,6 @@ static struct inode *v9fs_qid_iget_dotl(struct super_block *sb,
}
-struct inode *
-v9fs_inode_from_fid_dotl(struct v9fs_session_info *v9ses, struct p9_fid *fid,
- struct super_block *sb, int new)
-{
- struct p9_stat_dotl *st = NULL;
- struct inode *inode = NULL;
-
- st = p9_client_getattr_dotl(fid, P9_STATS_BASIC | P9_STATS_GEN);
- if (IS_ERR(st))
- return ERR_CAST(st);
-
- inode = v9fs_qid_iget_dotl(sb, &st->qid, fid, st);
-
- kfree(st);
- return inode;
-}
-
struct dotl_openflag_map {
int open_flag;
int dotl_flag;
@@ -261,7 +246,7 @@ v9fs_vfs_atomic_open_dotl(struct inode *dir, struct dentry *dentry,
p9_debug(P9_DEBUG_VFS, "p9_client_walk failed %d\n", err);
goto out;
}
- inode = v9fs_get_new_inode_from_fid(v9ses, fid, dir->i_sb);
+ inode = v9fs_fid_iget_dotl(dir->i_sb, fid);
if (IS_ERR(inode)) {
err = PTR_ERR(inode);
p9_debug(P9_DEBUG_VFS, "inode creation failed %d\n", err);
@@ -356,7 +341,7 @@ static int v9fs_vfs_mkdir_dotl(struct mnt_idmap *idmap,
}
/* instantiate inode and assign the unopened fid to the dentry */
- inode = v9fs_get_new_inode_from_fid(v9ses, fid, dir->i_sb);
+ inode = v9fs_fid_iget_dotl(dir->i_sb, fid);
if (IS_ERR(inode)) {
err = PTR_ERR(inode);
p9_debug(P9_DEBUG_VFS, "inode creation failed %d\n",
@@ -793,7 +778,7 @@ v9fs_vfs_mknod_dotl(struct mnt_idmap *idmap, struct inode *dir,
err);
goto error;
}
- inode = v9fs_get_new_inode_from_fid(v9ses, fid, dir->i_sb);
+ inode = v9fs_fid_iget_dotl(dir->i_sb, fid);
if (IS_ERR(inode)) {
err = PTR_ERR(inode);
p9_debug(P9_DEBUG_VFS, "inode creation failed %d\n",
diff --git a/fs/9p/vfs_super.c b/fs/9p/vfs_super.c
index 8d14cc0b3916..6d9a98c57185 100644
--- a/fs/9p/vfs_super.c
+++ b/fs/9p/vfs_super.c
@@ -139,7 +139,7 @@ static struct dentry *v9fs_mount(struct file_system_type *fs_type, int flags,
else
sb->s_d_op = &v9fs_dentry_operations;
- inode = v9fs_get_new_inode_from_fid(v9ses, fid, sb);
+ inode = v9fs_get_inode_from_fid(v9ses, fid, sb);
if (IS_ERR(inode)) {
retval = PTR_ERR(inode);
goto release_sb;
--
2.41.0
^ permalink raw reply related [flat|nested] 13+ messages in thread