Re: [PATCH v3 bpf-next 2/4] ftrace: allow IPMODIFY and DIRECT ops on the same function

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> On Jul 17, 2022, at 8:36 PM, kernel test robot <lkp@xxxxxxxxx> wrote:
> 
> Hi Song,
> 
> I love your patch! Yet something to improve:
> 
> [auto build test ERROR on bpf-next/master]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Song-Liu/ftrace-host-klp-and-bpf-trampoline-together/20220718-081533
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
> config: x86_64-randconfig-a001 (https://download.01.org/0day-ci/archive/20220718/202207181140.tqIgD0Jp-lkp@xxxxxxxxx/config )
> compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project d74b88c69dc2644bd0dc5d64e2d7413a0d4040e5)
> reproduce (this is a W=1 build):
>        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross  -O ~/bin/make.cross
>        chmod +x ~/bin/make.cross
>        # https://github.com/intel-lab-lkp/linux/commit/1535f287d288f9b7540ec50f56da1fe437ac6512
>        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-081533
>        git checkout 1535f287d288f9b7540ec50f56da1fe437ac6512
>        # save the config file
>        mkdir build_dir && cp config build_dir/.config
>        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash
> 
> If you fix the issue, kindly add following tag where applicable
> Reported-by: kernel test robot <lkp@xxxxxxxxx>
> 
> All errors (new ones prefixed by >>):
> 
>>> kernel/trace/ftrace.c:8082:14: error: use of undeclared identifier 'direct_mutex'; did you mean 'event_mutex'?
>           mutex_lock(&direct_mutex);
>                       ^~~~~~~~~~~~
>                       event_mutex

Folded the the fix in. I guess this should also fail on v2, but 
the kernel test robot didn't seem to find it. 

Song

>   include/linux/mutex.h:187:44: note: expanded from macro 'mutex_lock'
>   #define mutex_lock(lock) mutex_lock_nested(lock, 0)
>                                              ^
>   kernel/trace/trace.h:1523:21: note: 'event_mutex' declared here
>   extern struct mutex event_mutex;
>                       ^
>>> kernel/trace/ftrace.c:8084:14: error: no member named 'func_hash' in 'struct ftrace_ops'
>           hash = ops->func_hash->filter_hash;
>                  ~~~  ^
>>> kernel/trace/ftrace.c:8095:9: error: call to undeclared function 'ops_references_ip'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
>                                   if (ops_references_ip(op, ip)) {
>                                       ^
>>> kernel/trace/ftrace.c:8103:14: error: no member named 'ops_func' in 'struct ftrace_ops'
>                                   if (!op->ops_func) {
>                                        ~~  ^
>   kernel/trace/ftrace.c:8107:15: error: no member named 'ops_func' in 'struct ftrace_ops'
>                                   ret = op->ops_func(op, FTRACE_OPS_CMD_ENABLE_SHARE_IPMODIFY_PEER);
>                                         ~~  ^
>   kernel/trace/ftrace.c:8122:16: error: use of undeclared identifier 'direct_mutex'; did you mean 'event_mutex'?
>           mutex_unlock(&direct_mutex);
>                         ^~~~~~~~~~~~
>                         event_mutex
>   kernel/trace/trace.h:1523:21: note: 'event_mutex' declared here
>   extern struct mutex event_mutex;
>                       ^
>   kernel/trace/ftrace.c:8158:17: error: use of undeclared identifier 'direct_mutex'; did you mean 'event_mutex'?
>                   mutex_unlock(&direct_mutex);
>                                 ^~~~~~~~~~~~
>                                 event_mutex
>   kernel/trace/trace.h:1523:21: note: 'event_mutex' declared here
>   extern struct mutex event_mutex;
>                       ^
>   kernel/trace/ftrace.c:8178:14: error: use of undeclared identifier 'direct_mutex'; did you mean 'event_mutex'?
>           mutex_lock(&direct_mutex);
>                       ^~~~~~~~~~~~
>                       event_mutex
>   include/linux/mutex.h:187:44: note: expanded from macro 'mutex_lock'
>   #define mutex_lock(lock) mutex_lock_nested(lock, 0)
>                                              ^
>   kernel/trace/trace.h:1523:21: note: 'event_mutex' declared here
>   extern struct mutex event_mutex;
>                       ^
>   kernel/trace/ftrace.c:8180:14: error: no member named 'func_hash' in 'struct ftrace_ops'
>           hash = ops->func_hash->filter_hash;
>                  ~~~  ^
>   kernel/trace/ftrace.c:8191:9: error: call to undeclared function 'ops_references_ip'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
>                                   if (ops_references_ip(op, ip)) {
>                                       ^
>   kernel/trace/ftrace.c:8199:24: error: no member named 'ops_func' in 'struct ftrace_ops'
>                           if (found_op && op->ops_func)
>                                           ~~  ^
>   kernel/trace/ftrace.c:8200:9: error: no member named 'ops_func' in 'struct ftrace_ops'
>                                   op->ops_func(op, FTRACE_OPS_CMD_DISABLE_SHARE_IPMODIFY_PEER);
>                                   ~~  ^
>   kernel/trace/ftrace.c:8203:16: error: use of undeclared identifier 'direct_mutex'; did you mean 'event_mutex'?
>           mutex_unlock(&direct_mutex);
>                         ^~~~~~~~~~~~
>                         event_mutex
>   kernel/trace/trace.h:1523:21: note: 'event_mutex' declared here
>   extern struct mutex event_mutex;
>                       ^
>   13 errors generated.
> 
> 
> vim +8082 kernel/trace/ftrace.c
> 
>  8051	
>  8052	/*
>  8053	 * When registering ftrace_ops with IPMODIFY, it is necessary to make sure
>  8054	 * it doesn't conflict with any direct ftrace_ops. If there is existing
>  8055	 * direct ftrace_ops on a kernel function being patched, call
>  8056	 * FTRACE_OPS_CMD_ENABLE_SHARE_IPMODIFY_PEER on it to enable sharing.
>  8057	 *
>  8058	 * @ops:     ftrace_ops being registered.
>  8059	 *
>  8060	 * Returns:
>  8061	 *         0 - @ops does not have IPMODIFY or @ops itself is DIRECT, no
>  8062	 *             change needed;
>  8063	 *         1 - @ops has IPMODIFY, hold direct_mutex;
>  8064	 *         -EBUSY - currently registered DIRECT ftrace_ops cannot share the
>  8065	 *                  same function with IPMODIFY, abort the register.
>  8066	 *         -EAGAIN - cannot make changes to currently registered DIRECT
>  8067	 *                   ftrace_ops due to rare race conditions. Should retry
>  8068	 *                   later. This is needed to avoid potential deadlocks
>  8069	 *                   on the DIRECT ftrace_ops side.
>  8070	 */
>  8071	static int prepare_direct_functions_for_ipmodify(struct ftrace_ops *ops)
>  8072		__acquires(&direct_mutex)
>  8073	{
>  8074		struct ftrace_func_entry *entry;
>  8075		struct ftrace_hash *hash;
>  8076		struct ftrace_ops *op;
>  8077		int size, i, ret;
>  8078	
>  8079		if (!(ops->flags & FTRACE_OPS_FL_IPMODIFY))
>  8080			return 0;
>  8081	
>> 8082		mutex_lock(&direct_mutex);
>  8083	
>> 8084		hash = ops->func_hash->filter_hash;
>  8085		size = 1 << hash->size_bits;
>  8086		for (i = 0; i < size; i++) {
>  8087			hlist_for_each_entry(entry, &hash->buckets[i], hlist) {
>  8088				unsigned long ip = entry->ip;
>  8089				bool found_op = false;
>  8090	
>  8091				mutex_lock(&ftrace_lock);
>  8092				do_for_each_ftrace_op(op, ftrace_ops_list) {
>  8093					if (!(op->flags & FTRACE_OPS_FL_DIRECT))
>  8094						continue;
>> 8095					if (ops_references_ip(op, ip)) {
>  8096						found_op = true;
>  8097						break;
>  8098					}
>  8099				} while_for_each_ftrace_op(op);
>  8100				mutex_unlock(&ftrace_lock);
>  8101	
>  8102				if (found_op) {
>> 8103					if (!op->ops_func) {
>  8104						ret = -EBUSY;
>  8105						goto err_out;
>  8106					}
>  8107					ret = op->ops_func(op, FTRACE_OPS_CMD_ENABLE_SHARE_IPMODIFY_PEER);
>  8108					if (ret)
>  8109						goto err_out;
>  8110				}
>  8111			}
>  8112		}
>  8113	
>  8114		/*
>  8115		 * Didn't find any overlap with direct ftrace_ops, or the direct
>  8116		 * function can share with ipmodify. Hold direct_mutex to make sure
>  8117		 * this doesn't change until we are done.
>  8118		 */
>  8119		return 1;
>  8120	
>  8121	err_out:
>  8122		mutex_unlock(&direct_mutex);
>  8123		return ret;
>  8124	}
>  8125	
> 
> -- 
> 0-DAY CI Kernel Test Service
> https://01.org/lkp 




[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux