Messages in this thread | | | Date | Tue, 28 Nov 2023 21:29:15 +0900 | From | Masami Hiramatsu (Google) <> | Subject | Re: [PATCH] rethook: Use __rcu pointer for rethook::handler |
| |
On Tue, 28 Nov 2023 08:02:23 +0900 Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> On Fri, 24 Nov 2023 23:40:57 +0800 > kernel test robot <lkp@intel.com> wrote: > > > Hi Masami, > > > > kernel test robot noticed the following build warnings: > > > > [auto build test WARNING on linus/master] > > [also build test WARNING on v6.7-rc2 next-20231124] > > [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/Masami-Hiramatsu-Google/rethook-Use-__rcu-pointer-for-rethook-handler/20231124-090634 > > base: linus/master > > patch link: https://lore.kernel.org/r/170078778632.209874.7893551840863388753.stgit%40devnote2 > > patch subject: [PATCH] rethook: Use __rcu pointer for rethook::handler > > config: x86_64-randconfig-r113-20231124 (https://download.01.org/0day-ci/archive/20231124/202311241808.rv9ceuAh-lkp@intel.com/config) > > compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07) > > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231124/202311241808.rv9ceuAh-lkp@intel.com/reproduce) > > Let me fix this issue. It seems that sparse with function pointer > needs a special care.
Hmm, I think we need to ignore some warnings from sparse for function pointer.
struct rethook { rethook_handler_t __rcu handler; };
This doesn't work because the handler is not a pointer but a value type from the sparse point of view(?)
So if we change the rethook_handler_t as below, most of the sparse warnings are gone.
typedef void (__rcu *rethook_handler_t) (struct rethook_node *, void *, unsigned long, struct pt_regs *); ^^^^^^
However, I found that this causes another warnings on the init function call which requires rethook_handler_t as a parameter.
kernel/trace/fprobe.c:209:49: warning: incorrect type in argument 2 (different address spaces) kernel/trace/fprobe.c:209:49: expected void ( [noderef] __rcu *[usertype] handler )( ... ) kernel/trace/fprobe.c:209:49: got void ( * )( ... )
fp->rethook = rethook_alloc((void *)fp, fprobe_exit_handler, ^^^^^^^^^^^^^^^^^^^^^^^^^ sizeof(struct fprobe_rethook_node), size);
Why? because fprobe_exit_handler() has no "__rcu"! Of course we can fix this warning with force casting, e.g.
fp->rethook = rethook_alloc((void *)fp, (rethook_handler_t)fprobe_exit_handler,
But this is totally wrong because it disables the compiler's type check!
One possible solution is to use a raw function pointer type for the rethook_alloc() but that will be redundant or, just ignore the sparse warnings as Documentation/RCU/rcu_dereference.rst said.
(Another tricky hack is to use a union just for rcu_assign_pointer/rcu_derefernce)
Thank you,
> > Thank you, > > > > > 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/202311241808.rv9ceuAh-lkp@intel.com/ > > > > sparse warnings: (new ones prefixed by >>) > > >> kernel/trace/rethook.c:51:9: sparse: sparse: incompatible types in comparison expression (different address spaces): > > >> kernel/trace/rethook.c:51:9: sparse: void ( [noderef] __rcu * )( ... ) > > >> kernel/trace/rethook.c:51:9: sparse: void ( * )( ... ) > > kernel/trace/rethook.c:66:9: sparse: sparse: incompatible types in comparison expression (different address spaces): > > kernel/trace/rethook.c:66:9: sparse: void ( [noderef] __rcu * )( ... ) > > kernel/trace/rethook.c:66:9: sparse: void ( * )( ... ) > > kernel/trace/rethook.c:110:9: sparse: sparse: incompatible types in comparison expression (different address spaces): > > kernel/trace/rethook.c:110:9: sparse: void ( [noderef] __rcu * )( ... ) > > kernel/trace/rethook.c:110:9: sparse: void ( * )( ... ) > > kernel/trace/rethook.c:140:19: sparse: sparse: incompatible types in comparison expression (different address spaces): > > kernel/trace/rethook.c:140:19: sparse: void ( [noderef] __rcu * )( ... ) > > kernel/trace/rethook.c:140:19: sparse: void ( * )( ... ) > > kernel/trace/rethook.c:161:19: sparse: sparse: incompatible types in comparison expression (different address spaces): > > kernel/trace/rethook.c:161:19: sparse: void ( [noderef] __rcu * )( ... ) > > kernel/trace/rethook.c:161:19: sparse: void ( * )( ... ) > > kernel/trace/rethook.c:305:27: sparse: sparse: incompatible types in comparison expression (different address spaces): > > kernel/trace/rethook.c:305:27: sparse: void ( [noderef] __rcu * )( ... ) > > kernel/trace/rethook.c:305:27: sparse: void ( * )( ... ) > > > > vim +51 kernel/trace/rethook.c > > > > 40 > > 41 /** > > 42 * rethook_stop() - Stop using a rethook. > > 43 * @rh: the struct rethook to stop. > > 44 * > > 45 * Stop using a rethook to prepare for freeing it. If you want to wait for > > 46 * all running rethook handler before calling rethook_free(), you need to > > 47 * call this first and wait RCU, and call rethook_free(). > > 48 */ > > 49 void rethook_stop(struct rethook *rh) > > 50 { > > > 51 rcu_assign_pointer(rh->handler, NULL); > > 52 } > > 53 > > > > -- > > 0-DAY CI Kernel Test Service > > https://github.com/intel/lkp-tests/wiki > > > -- > Masami Hiramatsu (Google) <mhiramat@kernel.org>
-- Masami Hiramatsu (Google) <mhiramat@kernel.org>
| |