It's possible for v9fs_fid_find "find by dentry" branch to not turn up
anything despite having an entry set (because e.g. uid doesn't match),
in which case the calling code will generally make an extra lookup
to the server.
In this case we might have had better luck looking by inode, so fall
back to look up by inode if we have one and the lookup by dentry failed.
Signed-off-by: Dominique Martinet <[email protected]>
---
There's no hurry with this patch, I'll just queue it up for next cycle
in ~2 months, just sending before I forget.
fs/9p/fid.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/9p/fid.c b/fs/9p/fid.c
index de009a33e0e2..c72825fb0ece 100644
--- a/fs/9p/fid.c
+++ b/fs/9p/fid.c
@@ -131,9 +131,9 @@ static struct p9_fid *v9fs_fid_find(struct dentry *dentry, kuid_t uid, int any)
}
}
spin_unlock(&dentry->d_lock);
- } else {
- if (dentry->d_inode)
- ret = v9fs_fid_find_inode(dentry->d_inode, false, uid, any);
+ }
+ if (!ret && dentry->d_inode)
+ ret = v9fs_fid_find_inode(dentry->d_inode, false, uid, any);
}
return ret;
--
2.44.0
Hi Dominique,
kernel test robot noticed the following build warnings:
[auto build test WARNING on v6.9]
[also build test WARNING on linus/master next-20240523]
[cannot apply to ericvh-v9fs/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Dominique-Martinet/9p-v9fs_fid_find-also-lookup-by-inode-if-not-found-dentry/20240523-193912
base: v6.9
patch link: https://lore.kernel.org/r/20240523113638.1196299-1-asmadeus%40codewreck.org
patch subject: [PATCH] 9p: v9fs_fid_find: also lookup by inode if not found dentry
config: riscv-defconfig (https://download.01.org/0day-ci/archive/20240524/[email protected]/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project 7aa382fd7257d9bd4f7fc50bb7078a3c26a1628c)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240524/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
In file included from fs/9p/fid.c:17:
In file included from fs/9p/v9fs.h:11:
In file included from include/linux/backing-dev.h:16:
In file included from include/linux/writeback.h:13:
In file included from include/linux/blk_types.h:10:
In file included from include/linux/bvec.h:10:
In file included from include/linux/highmem.h:8:
In file included from include/linux/cacheflush.h:5:
In file included from arch/riscv/include/asm/cacheflush.h:9:
In file included from include/linux/mm.h:2210:
include/linux/vmstat.h:522:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
522 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~
>> fs/9p/fid.c:137:2: warning: non-void function does not return a value [-Wreturn-type]
137 | }
| ^
fs/9p/fid.c:139:2: error: expected identifier or '('
139 | return ret;
| ^
fs/9p/fid.c:140:1: error: extraneous closing brace ('}')
140 | }
| ^
2 warnings and 2 errors generated.
vim +137 fs/9p/fid.c
987a64850996db Greg Kurz 2020-09-23 103
987a64850996db Greg Kurz 2020-09-23 104
3ed8491c8a75ce Eric Van Hensbergen 2005-09-09 105 /**
ba17674fe02909 Latchesar Ionkov 2007-10-17 106 * v9fs_fid_find - retrieve a fid that belongs to the specified uid
3ed8491c8a75ce Eric Van Hensbergen 2005-09-09 107 * @dentry: dentry to look for fid in
ba17674fe02909 Latchesar Ionkov 2007-10-17 108 * @uid: return fid that belongs to the specified user
ba17674fe02909 Latchesar Ionkov 2007-10-17 109 * @any: if non-zero, return any fid associated with the dentry
3ed8491c8a75ce Eric Van Hensbergen 2005-09-09 110 *
3ed8491c8a75ce Eric Van Hensbergen 2005-09-09 111 */
3ed8491c8a75ce Eric Van Hensbergen 2005-09-09 112
b464255699077c Eric W. Biederman 2013-01-30 113 static struct p9_fid *v9fs_fid_find(struct dentry *dentry, kuid_t uid, int any)
3ed8491c8a75ce Eric Van Hensbergen 2005-09-09 114 {
ba17674fe02909 Latchesar Ionkov 2007-10-17 115 struct p9_fid *fid, *ret;
bd238fb431f319 Latchesar Ionkov 2007-07-10 116
4b8e992392a246 Al Viro 2014-08-19 117 p9_debug(P9_DEBUG_VFS, " dentry: %pd (%p) uid %d any %d\n",
4b8e992392a246 Al Viro 2014-08-19 118 dentry, dentry, from_kuid(&init_user_ns, uid),
b464255699077c Eric W. Biederman 2013-01-30 119 any);
ba17674fe02909 Latchesar Ionkov 2007-10-17 120 ret = NULL;
aaeb7ecfb48ad4 Al Viro 2013-02-28 121 /* we'll recheck under lock if there's anything to look in */
22e424feb6658c Dominique Martinet 2022-01-29 122 if (dentry->d_fsdata) {
aaeb7ecfb48ad4 Al Viro 2013-02-28 123 struct hlist_head *h = (struct hlist_head *)&dentry->d_fsdata;
9a268faa5f8627 Sohaib Mohamed 2021-10-01 124
634095dab2a200 Al Viro 2013-02-27 125 spin_lock(&dentry->d_lock);
56a79b7b021bf1 Linus Torvalds 2013-03-03 126 hlist_for_each_entry(fid, h, dlist) {
b464255699077c Eric W. Biederman 2013-01-30 127 if (any || uid_eq(fid->uid, uid)) {
ba17674fe02909 Latchesar Ionkov 2007-10-17 128 ret = fid;
b48dbb998d70b7 Dominique Martinet 2022-06-12 129 p9_fid_get(ret);
ba17674fe02909 Latchesar Ionkov 2007-10-17 130 break;
ba17674fe02909 Latchesar Ionkov 2007-10-17 131 }
ba17674fe02909 Latchesar Ionkov 2007-10-17 132 }
634095dab2a200 Al Viro 2013-02-27 133 spin_unlock(&dentry->d_lock);
7894f99a4c6ef6 Dominique Martinet 2024-05-23 134 }
7894f99a4c6ef6 Dominique Martinet 2024-05-23 135 if (!ret && dentry->d_inode)
1543b4c5071c54 Eric Van Hensbergen 2023-03-27 136 ret = v9fs_fid_find_inode(dentry->d_inode, false, uid, any);
ba17674fe02909 Latchesar Ionkov 2007-10-17 @137 }
bd238fb431f319 Latchesar Ionkov 2007-07-10 138
ba17674fe02909 Latchesar Ionkov 2007-10-17 139 return ret;
3ed8491c8a75ce Eric Van Hensbergen 2005-09-09 140 }
3ed8491c8a75ce Eric Van Hensbergen 2005-09-09 141
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi Dominique,
kernel test robot noticed the following build warnings:
[auto build test WARNING on v6.9]
[also build test WARNING on linus/master next-20240523]
[cannot apply to ericvh-v9fs/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Dominique-Martinet/9p-v9fs_fid_find-also-lookup-by-inode-if-not-found-dentry/20240523-193912
base: v6.9
patch link: https://lore.kernel.org/r/20240523113638.1196299-1-asmadeus%40codewreck.org
patch subject: [PATCH] 9p: v9fs_fid_find: also lookup by inode if not found dentry
config: x86_64-defconfig (https://download.01.org/0day-ci/archive/20240524/[email protected]/config)
compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240524/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
fs/9p/fid.c: In function 'v9fs_fid_find':
>> fs/9p/fid.c:137:9: warning: no return statement in function returning non-void [-Wreturn-type]
137 | }
| ^
fs/9p/fid.c: At top level:
fs/9p/fid.c:139:9: error: expected identifier or '(' before 'return'
139 | return ret;
| ^~~~~~
fs/9p/fid.c:140:1: error: expected identifier or '(' before '}' token
140 | }
| ^
vim +137 fs/9p/fid.c
987a64850996db Greg Kurz 2020-09-23 103
987a64850996db Greg Kurz 2020-09-23 104
3ed8491c8a75ce Eric Van Hensbergen 2005-09-09 105 /**
ba17674fe02909 Latchesar Ionkov 2007-10-17 106 * v9fs_fid_find - retrieve a fid that belongs to the specified uid
3ed8491c8a75ce Eric Van Hensbergen 2005-09-09 107 * @dentry: dentry to look for fid in
ba17674fe02909 Latchesar Ionkov 2007-10-17 108 * @uid: return fid that belongs to the specified user
ba17674fe02909 Latchesar Ionkov 2007-10-17 109 * @any: if non-zero, return any fid associated with the dentry
3ed8491c8a75ce Eric Van Hensbergen 2005-09-09 110 *
3ed8491c8a75ce Eric Van Hensbergen 2005-09-09 111 */
3ed8491c8a75ce Eric Van Hensbergen 2005-09-09 112
b464255699077c Eric W. Biederman 2013-01-30 113 static struct p9_fid *v9fs_fid_find(struct dentry *dentry, kuid_t uid, int any)
3ed8491c8a75ce Eric Van Hensbergen 2005-09-09 114 {
ba17674fe02909 Latchesar Ionkov 2007-10-17 115 struct p9_fid *fid, *ret;
bd238fb431f319 Latchesar Ionkov 2007-07-10 116
4b8e992392a246 Al Viro 2014-08-19 117 p9_debug(P9_DEBUG_VFS, " dentry: %pd (%p) uid %d any %d\n",
4b8e992392a246 Al Viro 2014-08-19 118 dentry, dentry, from_kuid(&init_user_ns, uid),
b464255699077c Eric W. Biederman 2013-01-30 119 any);
ba17674fe02909 Latchesar Ionkov 2007-10-17 120 ret = NULL;
aaeb7ecfb48ad4 Al Viro 2013-02-28 121 /* we'll recheck under lock if there's anything to look in */
22e424feb6658c Dominique Martinet 2022-01-29 122 if (dentry->d_fsdata) {
aaeb7ecfb48ad4 Al Viro 2013-02-28 123 struct hlist_head *h = (struct hlist_head *)&dentry->d_fsdata;
9a268faa5f8627 Sohaib Mohamed 2021-10-01 124
634095dab2a200 Al Viro 2013-02-27 125 spin_lock(&dentry->d_lock);
56a79b7b021bf1 Linus Torvalds 2013-03-03 126 hlist_for_each_entry(fid, h, dlist) {
b464255699077c Eric W. Biederman 2013-01-30 127 if (any || uid_eq(fid->uid, uid)) {
ba17674fe02909 Latchesar Ionkov 2007-10-17 128 ret = fid;
b48dbb998d70b7 Dominique Martinet 2022-06-12 129 p9_fid_get(ret);
ba17674fe02909 Latchesar Ionkov 2007-10-17 130 break;
ba17674fe02909 Latchesar Ionkov 2007-10-17 131 }
ba17674fe02909 Latchesar Ionkov 2007-10-17 132 }
634095dab2a200 Al Viro 2013-02-27 133 spin_unlock(&dentry->d_lock);
7894f99a4c6ef6 Dominique Martinet 2024-05-23 134 }
7894f99a4c6ef6 Dominique Martinet 2024-05-23 135 if (!ret && dentry->d_inode)
1543b4c5071c54 Eric Van Hensbergen 2023-03-27 136 ret = v9fs_fid_find_inode(dentry->d_inode, false, uid, any);
ba17674fe02909 Latchesar Ionkov 2007-10-17 @137 }
bd238fb431f319 Latchesar Ionkov 2007-07-10 138
ba17674fe02909 Latchesar Ionkov 2007-10-17 139 return ret;
3ed8491c8a75ce Eric Van Hensbergen 2005-09-09 140 }
3ed8491c8a75ce Eric Van Hensbergen 2005-09-09 141
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
It's possible for v9fs_fid_find "find by dentry" branch to not turn up
anything despite having an entry set (because e.g. uid doesn't match),
in which case the calling code will generally make an extra lookup
to the server.
In this case we might have had better luck looking by inode, so fall
back to look up by inode if we have one and the lookup by dentry failed.
Signed-off-by: Dominique Martinet <[email protected]>
---
v1 -> v2: fix build error
Sorry, shouldn't send patches without testing even if I'm not planning
to apply them for a while...
fs/9p/fid.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/fs/9p/fid.c b/fs/9p/fid.c
index de009a33e0e2..f84412290a30 100644
--- a/fs/9p/fid.c
+++ b/fs/9p/fid.c
@@ -131,10 +131,9 @@ static struct p9_fid *v9fs_fid_find(struct dentry *dentry, kuid_t uid, int any)
}
}
spin_unlock(&dentry->d_lock);
- } else {
- if (dentry->d_inode)
- ret = v9fs_fid_find_inode(dentry->d_inode, false, uid, any);
}
+ if (!ret && dentry->d_inode)
+ ret = v9fs_fid_find_inode(dentry->d_inode, false, uid, any);
return ret;
}
--
2.44.0
On Thursday, May 23, 2024 11:00:23 PM CEST Dominique Martinet wrote:
> It's possible for v9fs_fid_find "find by dentry" branch to not turn up
> anything despite having an entry set (because e.g. uid doesn't match),
> in which case the calling code will generally make an extra lookup
> to the server.
>
> In this case we might have had better luck looking by inode, so fall
> back to look up by inode if we have one and the lookup by dentry failed.
I'm not sure how that's supposed to happen. Both lookups check for (uid, any),
just on two separate fid lists (dentry vs. inode). But OTOH I don't see no
harm either, so:
Reviewed-by: Christian Schoenebeck <[email protected]>
> Signed-off-by: Dominique Martinet <[email protected]>
> ---
> v1 -> v2: fix build error
>
> Sorry, shouldn't send patches without testing even if I'm not planning
> to apply them for a while...
>
> fs/9p/fid.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/fs/9p/fid.c b/fs/9p/fid.c
> index de009a33e0e2..f84412290a30 100644
> --- a/fs/9p/fid.c
> +++ b/fs/9p/fid.c
> @@ -131,10 +131,9 @@ static struct p9_fid *v9fs_fid_find(struct dentry *dentry, kuid_t uid, int any)
> }
> }
> spin_unlock(&dentry->d_lock);
> - } else {
> - if (dentry->d_inode)
> - ret = v9fs_fid_find_inode(dentry->d_inode, false, uid, any);
> }
> + if (!ret && dentry->d_inode)
> + ret = v9fs_fid_find_inode(dentry->d_inode, false, uid, any);
>
> return ret;
> }
>