On Wed, Jan 4, 2023 at 8:25 AM Petr Mladek [off-list ref] wrote:

On Fri 2022-12-30 19:27:28, Zhen Lei wrote:

quoted

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 <redacted>
---
 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]);

This must be called under module_mutex to make sure that the module
would not disappear.

module_mutex is not available outside kernel/module/. The common
practice is to disable preempt before calling __module_address().
CONFIG_LOCKDEP should catch this.

Thanks,
Song

[...]