bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Zhen Lei <thunder.leizhen@huawei.com>
To: Josh Poimboeuf <jpoimboe@kernel.org>,
	Jiri Kosina <jikos@kernel.org>, Miroslav Benes <mbenes@suse.cz>,
	Petr Mladek <pmladek@suse.com>,
	Joe Lawrence <joe.lawrence@redhat.com>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>,
	Martin KaFai Lau <martin.lau@linux.dev>,
	Song Liu <song@kernel.org>, Yonghong Song <yhs@fb.com>,
	John Fastabend <john.fastabend@gmail.com>,
	KP Singh <kpsingh@kernel.org>,
	Stanislav Fomichev <sdf@google.com>, Hao Luo <haoluo@google.com>,
	Jiri Olsa <jolsa@kernel.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>, <bpf@vger.kernel.org>,
	<linux-trace-kernel@vger.kernel.org>,
	<live-patching@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	Luis Chamberlain <mcgrof@kernel.org>,
	<linux-modules@vger.kernel.org>
Cc: Zhen Lei <thunder.leizhen@huawei.com>
Subject: [PATCH 2/3] bpf: Optimize get_modules_for_addrs()
Date: Fri, 30 Dec 2022 19:27:28 +0800	[thread overview]
Message-ID: <20221230112729.351-3-thunder.leizhen@huawei.com> (raw)
In-Reply-To: <20221230112729.351-1-thunder.leizhen@huawei.com>

Function __module_address() can quickly return the pointer of the module
to which an address belongs. We do not need to traverse the symbols of all
modules to check whether each address in addrs[] is the start address of
the corresponding symbol, because register_fprobe_ips() will do this check
later.

Assuming that there are m modules, each module has n symbols on average,
and the number of addresses 'addrs_cnt' is abbreviated as K. Then the time
complexity of the original method is O(K * log(K)) + O(m * n * log(K)),
and the time complexity of current method is O(K * (log(m) + M)), M <= m.
(m * n * log(K)) / (K * m) ==> n / log2(K). Even if n is 10 and K is 128,
the ratio is still greater than 1. Therefore, the new method will
generally have better performance.

Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
 kernel/trace/bpf_trace.c | 101 ++++++++++++++++-----------------------
 1 file changed, 40 insertions(+), 61 deletions(-)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 5f3be4bc16403a5..0ff9037098bd241 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -2684,69 +2684,55 @@ 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 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 get_modules_for_addrs(struct module ***out_mods, unsigned long *addrs, u32 addrs_cnt)
 {
-	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;
+	int i, j, err;
+	int mods_cnt = 0;
+	int mods_cap = 0;
+	struct module *mod;
+	struct module **mods = NULL;
 
-	if (args->mods && args->mods[args->mods_cnt - 1] == mod)
-		return 0;
+	for (i = 0; i < addrs_cnt; i++) {
+		mod = __module_address(addrs[i]);
+		if (!mod)
+			continue;
 
-	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 (!mods)
-			return -ENOMEM;
-		args->mods = mods;
-	}
+		/* check if we already have the module pointer stored */
+		for (j = 0; j < mods_cnt; j++) {
+			if (mods[j] == mod)
+				break;
+		}
+		if (j < mods_cnt)
+			continue;
 
-	if (!try_module_get(mod))
-		return -EINVAL;
+		if (mods_cnt == mods_cap) {
+			struct module **new_mods;
 
-	args->mods[args->mods_cnt] = mod;
-	args->mods_cnt++;
-	return 0;
-}
+			mods_cap = max(16, mods_cap * 3 / 2);
+			new_mods = krealloc_array(mods, mods_cap, sizeof(*mods), GFP_KERNEL);
+			if (!new_mods) {
+				err = -ENOMEM;
+				goto failed;
+			}
+			mods = new_mods;
+		}
 
-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;
+		if (!try_module_get(mod)) {
+			err = -EINVAL;
+			goto failed;
+		}
 
-	/* We return either err < 0 in case of error, ... */
-	err = module_kallsyms_on_each_symbol(NULL, module_callback, &args);
-	if (err) {
-		kprobe_multi_put_modules(args.mods, args.mods_cnt);
-		kfree(args.mods);
-		return err;
+		mods[mods_cnt] = mod;
+		mods_cnt++;
 	}
 
-	/* or number of modules found if everything is ok. */
-	*mods = args.mods;
-	return args.mods_cnt;
+	*out_mods = mods;
+	return mods_cnt;
+
+failed:
+	kprobe_multi_put_modules(mods, mods_cnt);
+	kfree(mods);
+	return err;
 }
 
 int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
@@ -2859,13 +2845,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);
-- 
2.25.1


  parent reply	other threads:[~2022-12-30 11:29 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-30 11:27 [PATCH 0/3] kallsyms: Optimize the search for module symbols by livepatch and bpf Zhen Lei
2022-12-30 11:27 ` [PATCH 1/3] livepatch: Improve the search performance of module_kallsyms_on_each_symbol() Zhen Lei
2023-01-04 15:36   ` Petr Mladek
2022-12-30 11:27 ` Zhen Lei [this message]
2023-01-04 16:25   ` [PATCH 2/3] bpf: Optimize get_modules_for_addrs() Petr Mladek
2023-01-04 17:07     ` Song Liu
2023-01-05  7:31       ` Leizhen (ThunderTown)
2023-01-05  9:05       ` Petr Mladek
2023-01-09  4:02         ` Leizhen (ThunderTown)
2023-01-05  7:48     ` Leizhen (ThunderTown)
2023-01-05  9:32     ` Petr Mladek
2023-01-09  4:10       ` Leizhen (ThunderTown)
2023-01-05 21:31     ` Jiri Olsa
2023-01-06  9:45       ` Jiri Olsa
2023-01-09  8:51         ` Leizhen (ThunderTown)
2023-01-09 13:48           ` Jiri Olsa
2023-01-09 15:11             ` Leizhen (ThunderTown)
2023-01-11  8:41               ` Leizhen (ThunderTown)
2023-01-11  9:53                 ` Jiri Olsa
2023-01-09  7:03       ` Leizhen (ThunderTown)
2022-12-30 11:27 ` [PATCH 3/3] kallsyms: Delete an unused parameter related to {module_}kallsyms_on_each_symbol() Zhen Lei

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221230112729.351-3-thunder.leizhen@huawei.com \
    --to=thunder.leizhen@huawei.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=haoluo@google.com \
    --cc=jikos@kernel.org \
    --cc=joe.lawrence@redhat.com \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=jpoimboe@kernel.org \
    --cc=kpsingh@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-modules@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=live-patching@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=martin.lau@linux.dev \
    --cc=mbenes@suse.cz \
    --cc=mcgrof@kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=pmladek@suse.com \
    --cc=rostedt@goodmis.org \
    --cc=sdf@google.com \
    --cc=song@kernel.org \
    --cc=yhs@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).