[PATCH] 9p: v9fs_fid_find: also lookup by inode if not found dentry

public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed

* [PATCH] 9p: v9fs_fid_find: also lookup by inode if not found dentry
@ 2024-05-23 11:36 Dominique Martinet
  2024-05-23 20:25 ` kernel test robot
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Dominique Martinet @ 2024-05-23 11:36 UTC (permalink / raw)
  To: Eric Van Hensbergen, Latchesar Ionkov, Dominique Martinet,
	Christian Schoenebeck
  Cc: v9fs, linux-kernel

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 <asmadeus@codewreck.org>
---
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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] 9p: v9fs_fid_find: also lookup by inode if not found dentry
  2024-05-23 11:36 [PATCH] 9p: v9fs_fid_find: also lookup by inode if not found dentry Dominique Martinet
@ 2024-05-23 20:25 ` kernel test robot
  2024-05-23 20:46 ` kernel test robot
  2024-05-23 21:00 ` [PATCH v2] " Dominique Martinet
  2 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2024-05-23 20:25 UTC (permalink / raw)
  To: Dominique Martinet, Eric Van Hensbergen, Latchesar Ionkov,
	Christian Schoenebeck
  Cc: llvm, oe-kbuild-all, v9fs, linux-kernel

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/202405240422.ZWrRj5Ck-lkp@intel.com/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/202405240422.ZWrRj5Ck-lkp@intel.com/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 <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202405240422.ZWrRj5Ck-lkp@intel.com/

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] 9p: v9fs_fid_find: also lookup by inode if not found dentry
  2024-05-23 11:36 [PATCH] 9p: v9fs_fid_find: also lookup by inode if not found dentry Dominique Martinet
  2024-05-23 20:25 ` kernel test robot
@ 2024-05-23 20:46 ` kernel test robot
  2024-05-23 21:00 ` [PATCH v2] " Dominique Martinet
  2 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2024-05-23 20:46 UTC (permalink / raw)
  To: Dominique Martinet, Eric Van Hensbergen, Latchesar Ionkov,
	Christian Schoenebeck
  Cc: oe-kbuild-all, v9fs, linux-kernel

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/202405240419.mUEPAfWQ-lkp@intel.com/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/202405240419.mUEPAfWQ-lkp@intel.com/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 <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202405240419.mUEPAfWQ-lkp@intel.com/

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2] 9p: v9fs_fid_find: also lookup by inode if not found dentry
  2024-05-23 11:36 [PATCH] 9p: v9fs_fid_find: also lookup by inode if not found dentry Dominique Martinet
  2024-05-23 20:25 ` kernel test robot
  2024-05-23 20:46 ` kernel test robot
@ 2024-05-23 21:00 ` Dominique Martinet
  2024-05-26  8:43   ` Christian Schoenebeck
  2 siblings, 1 reply; 5+ messages in thread
From: Dominique Martinet @ 2024-05-23 21:00 UTC (permalink / raw)
  To: Eric Van Hensbergen, Latchesar Ionkov, Dominique Martinet,
	Christian Schoenebeck
  Cc: v9fs, linux-kernel

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 <asmadeus@codewreck.org>
---
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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] 9p: v9fs_fid_find: also lookup by inode if not found dentry
  2024-05-23 21:00 ` [PATCH v2] " Dominique Martinet
@ 2024-05-26  8:43   ` Christian Schoenebeck
  0 siblings, 0 replies; 5+ messages in thread
From: Christian Schoenebeck @ 2024-05-26  8:43 UTC (permalink / raw)
  To: Eric Van Hensbergen, Latchesar Ionkov, Dominique Martinet, v9fs
  Cc: linux-kernel

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 <linux_oss@crudebyte.com>

> Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
> ---
> 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;
>  }
> 



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-05-26  8:43 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-23 11:36 [PATCH] 9p: v9fs_fid_find: also lookup by inode if not found dentry Dominique Martinet
2024-05-23 20:25 ` kernel test robot
2024-05-23 20:46 ` kernel test robot
2024-05-23 21:00 ` [PATCH v2] " Dominique Martinet
2024-05-26  8:43   ` Christian Schoenebeck

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox