Commit 00b8f39f authored by Alexei Starovoitov's avatar Alexei Starovoitov
Browse files

Merge branch 'kallsyms: Optimize the search for module symbols by livepatch and bpf' 

Jiri Olsa says:

====================

hi,
sending new version of [1] patchset posted originally by Zhen Lei.
It contains 2 changes that improove search performance for livepatch
and bpf.

v3 changes:
  - fixed off by 1 issue, simplified condition, added acks [Song]
  - added module attach as subtest [Andrii]

v2 changes:
  - reworked the bpf change and meassured the performance
  - adding new selftest to benchmark kprobe multi module attachment
  - skipping patch 3 as requested by Zhen Lei
  - added Reviewed-by for patch 1 [Petr Mladek]

thanks,
jirka

[1] https://lore.kernel.org/bpf/20221230112729.351-1-thunder.leizhen@huawei.com/


---
Jiri Olsa (2):
      selftests/bpf: Add serial_test_kprobe_multi_bench_attach_kernel/module tests
      bpf: Change modules resolving for kprobe multi link
====================

Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
parents 92afc532 6a5f2d6e

include/linux/module.h

+4 −2
Original line number Diff line number Diff line
@@ -879,11 +879,13 @@ static inline bool module_sig_ok(struct module *module) 
#endif	/* CONFIG_MODULE_SIG */



#if defined(CONFIG_MODULES) && defined(CONFIG_KALLSYMS)

int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,

int module_kallsyms_on_each_symbol(const char *modname,

				   int (*fn)(void *, const char *,

					     struct module *, unsigned long),

				   void *data);

#else

static inline int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,

static inline int module_kallsyms_on_each_symbol(const char *modname,

						 int (*fn)(void *, const char *,

						 struct module *, unsigned long),

						 void *data)

{

kernel/livepatch/core.c

+1 −9
Original line number Diff line number Diff line
@@ -118,7 +118,6 @@ static struct klp_object *klp_find_object(struct klp_patch *patch, 
}



struct klp_find_arg {

	const char *objname;

	const char *name;

	unsigned long addr;

	unsigned long count;
@@ -148,15 +147,9 @@ static int klp_find_callback(void *data, const char *name, 
{

	struct klp_find_arg *args = data;



	if ((mod && !args->objname) || (!mod && args->objname))

		return 0;



	if (strcmp(args->name, name))

		return 0;



	if (args->objname && strcmp(args->objname, mod->name))

		return 0;



	return klp_match_callback(data, addr);

}
@@ -164,7 +157,6 @@ static int klp_find_object_symbol(const char *objname, const char *name, 
				  unsigned long sympos, unsigned long *addr)

{

	struct klp_find_arg args = {

		.objname = objname,

		.name = name,

		.addr = 0,

		.count = 0,
@@ -172,7 +164,7 @@ static int klp_find_object_symbol(const char *objname, const char *name, 
	};



	if (objname)

		module_kallsyms_on_each_symbol(klp_find_callback, &args);

		module_kallsyms_on_each_symbol(objname, klp_find_callback, &args);

	else

		kallsyms_on_each_match_symbol(klp_match_callback, name, &args);

kernel/module/kallsyms.c

+12 −1
Original line number Diff line number Diff line
@@ -494,7 +494,8 @@ unsigned long module_kallsyms_lookup_name(const char *name) 
	return ret;

}



int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,

int module_kallsyms_on_each_symbol(const char *modname,

				   int (*fn)(void *, const char *,

					     struct module *, unsigned long),

				   void *data)

{
@@ -509,6 +510,9 @@ int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *, 
		if (mod->state == MODULE_STATE_UNFORMED)

			continue;



		if (modname && strcmp(modname, mod->name))

			continue;



		/* Use rcu_dereference_sched() to remain compliant with the sparse tool */

		preempt_disable();

		kallsyms = rcu_dereference_sched(mod->kallsyms);
@@ -525,6 +529,13 @@ int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *, 
			if (ret != 0)

				goto out;

		}



		/*

		 * The given module is found, the subsequent modules do not

		 * need to be compared.

		 */

		if (modname)

			break;

	}

out:

	mutex_unlock(&module_mutex);

kernel/trace/bpf_trace.c

+47 −46
Original line number Diff line number Diff line
@@ -2682,69 +2682,77 @@ static void symbols_swap_r(void *a, void *b, int size, const void *priv) 
	}

}



struct module_addr_args {

	unsigned long *addrs;

	u32 addrs_cnt;

struct modules_array {

	struct module **mods;

	int mods_cnt;

	int mods_cap;

};



static int module_callback(void *data, const char *name,

			   struct module *mod, unsigned long addr)

static int add_module(struct modules_array *arr, struct module *mod)

{

	struct module_addr_args *args = data;

	struct module **mods;



	/* We iterate all modules symbols and for each we:

	 * - search for it in provided addresses array

	 * - if found we check if we already have the module pointer stored

	 *   (we iterate modules sequentially, so we can check just the last

	 *   module pointer)

	 * - take module reference and store it

	 */

	if (!bsearch(&addr, args->addrs, args->addrs_cnt, sizeof(addr),

		       bpf_kprobe_multi_addrs_cmp))

		return 0;



	if (args->mods && args->mods[args->mods_cnt - 1] == mod)

		return 0;



	if (args->mods_cnt == args->mods_cap) {

		args->mods_cap = max(16, args->mods_cap * 3 / 2);

		mods = krealloc_array(args->mods, args->mods_cap, sizeof(*mods), GFP_KERNEL);

	if (arr->mods_cnt == arr->mods_cap) {

		arr->mods_cap = max(16, arr->mods_cap * 3 / 2);

		mods = krealloc_array(arr->mods, arr->mods_cap, sizeof(*mods), GFP_KERNEL);

		if (!mods)

			return -ENOMEM;

		args->mods = mods;

		arr->mods = mods;

	}



	if (!try_module_get(mod))

		return -EINVAL;



	args->mods[args->mods_cnt] = mod;

	args->mods_cnt++;

	arr->mods[arr->mods_cnt] = mod;

	arr->mods_cnt++;

	return 0;

}



static bool has_module(struct modules_array *arr, struct module *mod)

{

	int i;



	for (i = arr->mods_cnt - 1; i >= 0; i--) {

		if (arr->mods[i] == mod)

			return true;

	}

	return false;

}



static int get_modules_for_addrs(struct module ***mods, unsigned long *addrs, u32 addrs_cnt)

{

	struct module_addr_args args = {

		.addrs     = addrs,

		.addrs_cnt = addrs_cnt,

	};

	int err;

	struct modules_array arr = {};

	u32 i, err = 0;



	for (i = 0; i < addrs_cnt; i++) {

		struct module *mod;



		preempt_disable();

		mod = __module_address(addrs[i]);

		/* Either no module or we it's already stored  */

		if (!mod || has_module(&arr, mod)) {

			preempt_enable();

			continue;

		}

		if (!try_module_get(mod))

			err = -EINVAL;

		preempt_enable();

		if (err)

			break;

		err = add_module(&arr, mod);

		if (err) {

			module_put(mod);

			break;

		}

	}



	/* We return either err < 0 in case of error, ... */

	err = module_kallsyms_on_each_symbol(module_callback, &args);

	if (err) {

		kprobe_multi_put_modules(args.mods, args.mods_cnt);

		kfree(args.mods);

		kprobe_multi_put_modules(arr.mods, arr.mods_cnt);

		kfree(arr.mods);

		return err;

	}



	/* or number of modules found if everything is ok. */

	*mods = args.mods;

	return args.mods_cnt;

	*mods = arr.mods;

	return arr.mods_cnt;

}



int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
@@ -2857,13 +2865,6 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr 
		       bpf_kprobe_multi_cookie_cmp,

		       bpf_kprobe_multi_cookie_swap,

		       link);

	} else {

		/*

		 * We need to sort addrs array even if there are no cookies

		 * provided, to allow bsearch in get_modules_for_addrs.

		 */

		sort(addrs, cnt, sizeof(*addrs),

		       bpf_kprobe_multi_addrs_cmp, NULL);

	}



	err = get_modules_for_addrs(&link->mods, addrs, cnt);

kernel/trace/ftrace.c

+1 −1
Original line number Diff line number Diff line
@@ -8324,7 +8324,7 @@ int ftrace_lookup_symbols(const char **sorted_syms, size_t cnt, unsigned long *a 
	found_all = kallsyms_on_each_symbol(kallsyms_callback, &args);

	if (found_all)

		return 0;

	found_all = module_kallsyms_on_each_symbol(kallsyms_callback, &args);

	found_all = module_kallsyms_on_each_symbol(NULL, kallsyms_callback, &args);

	return found_all ? 0 : -ESRCH;

}