On Mon 2022-07-18 15:42:25, kernel test robot wrote: > Hi Song, > > I love your patch! Perhaps something to improve: > > [auto build test WARNING on bpf-next/master] > > url: https://github.com/intel-lab-lkp/linux/commits/Song-Liu/ftrace-host-klp-and-bpf-trampoline-together/20220718-135652 > base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master > config: x86_64-randconfig-a004 (https://download.01.org/0day-ci/archive/20220718/202207181552.VuKfz9zg-lkp@xxxxxxxxx/config) > compiler: gcc-11 (Debian 11.3.0-3) 11.3.0 > reproduce (this is a W=1 build): > # https://github.com/intel-lab-lkp/linux/commit/9ef1ec8cb818d8ca70887c8c123f2d579384a6c6 > git remote add linux-review https://github.com/intel-lab-lkp/linux > git fetch --no-tags linux-review Song-Liu/ftrace-host-klp-and-bpf-trampoline-together/20220718-135652 > git checkout 9ef1ec8cb818d8ca70887c8c123f2d579384a6c6 > # save the config file > mkdir build_dir && cp config build_dir/.config > make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash kernel/trace/ > > If you fix the issue, kindly add following tag where applicable > Reported-by: kernel test robot <lkp@xxxxxxxxx> > > All warnings (new ones prefixed by >>): > > kernel/trace/ftrace.c: In function 'register_ftrace_function': > >> kernel/trace/ftrace.c:8197:14: warning: variable 'direct_mutex_locked' set but not used [-Wunused-but-set-variable] > 8197 | bool direct_mutex_locked = false; > | ^~~~~~~~~~~~~~~~~~~ > > > vim +/direct_mutex_locked +8197 kernel/trace/ftrace.c > > 8182 > 8183 /** > 8184 * register_ftrace_function - register a function for profiling > 8185 * @ops: ops structure that holds the function for profiling. > 8186 * > 8187 * Register a function to be called by all functions in the > 8188 * kernel. > 8189 * > 8190 * Note: @ops->func and all the functions it calls must be labeled > 8191 * with "notrace", otherwise it will go into a > 8192 * recursive loop. > 8193 */ > 8194 int register_ftrace_function(struct ftrace_ops *ops) > 8195 __releases(&direct_mutex) > 8196 { > > 8197 bool direct_mutex_locked = false; > 8198 int ret; > 8199 > 8200 ftrace_ops_init(ops); > 8201 > 8202 ret = prepare_direct_functions_for_ipmodify(ops); > 8203 if (ret < 0) > 8204 return ret; > 8205 else if (ret == 1) > 8206 direct_mutex_locked = true; Honestly, this is another horrible trick. Would it be possible to call prepare_direct_functions_for_ipmodify() with direct_mutex already taken? I mean something like: mutex_lock(&direct_mutex); ret = prepare_direct_functions_for_ipmodify(ops); if (ret) goto out: mutex_lock(&ftrace_lock); ret = ftrace_startup(ops, 0); mutex_unlock(&ftrace_lock); out: mutex_unlock(&direct_mutex); return ret; > 8208 mutex_lock(&ftrace_lock); > 8209 > 8210 ret = ftrace_startup(ops, 0); > 8211 > 8212 mutex_unlock(&ftrace_lock); > 8213 Would be possible to handle tr->mutex the same way to avoid the trylock? I mean to take it in advance before direct_mutex? Best Regards, Petr
