All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stanislaw Gruszka <stf_xl@wp.pl>
To: Petr Pavlu <petr.pavlu@suse.com>
Cc: linux-modules@vger.kernel.org,
	Sami Tolvanen <samitolvanen@google.com>,
	Luis Chamberlain <mcgrof@kernel.org>,
	linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org,
	live-patching@vger.kernel.org, Daniel Gomez <da.gomez@kernel.org>,
	Aaron Tomlin <atomlin@atomlin.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	Jordan Rome <linux@jordanrome.com>,
	Viktor Malik <vmalik@redhat.com>
Subject: Re: [PATCH] module/kallsyms: sort function symbols and use binary search
Date: Wed, 25 Mar 2026 09:26:48 +0100	[thread overview]
Message-ID: <20260325082648.GA18968@wp.pl> (raw)
In-Reply-To: <282574df-7689-4677-929b-b844e7201bd5@suse.com>

On Tue, Mar 24, 2026 at 05:00:19PM +0100, Petr Pavlu wrote:
> On 3/24/26 1:53 PM, Stanislaw Gruszka wrote:
> > Hi,
> > 
> > On Mon, Mar 23, 2026 at 02:06:43PM +0100, Petr Pavlu wrote:
> >> On 3/17/26 12:04 PM, Stanislaw Gruszka wrote:
> >>> Module symbol lookup via find_kallsyms_symbol() performs a linear scan
> >>> over the entire symtab when resolving an address. The number of symbols
> >>> in module symtabs has grown over the years, largely due to additional
> >>> metadata in non-standard sections, making this lookup very slow.
> >>>
> >>> Improve this by separating function symbols during module load, placing
> >>> them at the beginning of the symtab, sorting them by address, and using
> >>> binary search when resolving addresses in module text.
> >>
> >> Doesn't considering only function symbols break the expected behavior
> >> with CONFIG_KALLSYMS_ALL=y. For instance, when using kdb, is it still
> >> able to see all symbols in a module? The module loader should be remain
> >> consistent with the main kallsyms code regarding which symbols can be
> >> looked up.
> > 
> > We already have a CONFIG_KALLSYMS_ALL=y inconsistency between kernel and 
> > module symbol lookup, independent of this patch. find_kallsyms_symbol()
> > restricts the search to MOD_TEXT (or MOD_INIT_TEXT) address ranges, so
> > it cannot resolve data or rodata symbols.
> 
> My understanding is that find_kallsyms_symbol() can identify all symbols
> in a module by their addresses. However, the issue I see with
> MOD_TEXT/MOD_INIT_TEXT is that the function may incorrectly calculate
> the size of symbols that are not within these ranges, which is a bug
> that should be fixed.

You are right, I misinterpreted the code:

	if (within_module_init(addr, mod))
		mod_mem = &mod->mem[MOD_INIT_TEXT];
	else
		mod_mem = &mod->mem[MOD_TEXT];

	nextval = (unsigned long)mod_mem->base + mod_mem->size;

	bestval = kallsyms_symbol_value(&kallsyms->symtab[best]);

For best = 0, bestval is also 0 as it comes from the ELF null symbol.

> A test using kdb confirms that non-text symbols can be found by their
> addresses. The following shows the current behavior with 7.0-rc5 when
> printing a module parameter in mlx4_en:
> 
> [1]kdb> mds __param_arr_num_vfs
> 0xffffffffc1209f20 0000000100000003   ........
> 0xffffffffc1209f28 ffffffffc0fbf07c [mlx4_core]num_vfs_argc  
> 0xffffffffc1209f30 ffffffff8844bba0 param_ops_byte  
> 0xffffffffc1209f38 ffffffffc0fbf080 [mlx4_core]num_vfs  
> 0xffffffffc1209f40 000000785f69736d   msi_x...
> 0xffffffffc1209f48 656c5f6775626564   debug_le
> 0xffffffffc1209f50 00000000006c6576   vel.....
> 0xffffffffc1209f58 0000000000000000   ........
> 
> .. and the behavior with the proposed patch:
> 
> [1]kdb> mds __param_arr_num_vfs
> 0xffffffffc1077f20 0000000100000003   ........
> 0xffffffffc1077f28 ffffffffc104707c   |p......
> 0xffffffffc1077f30 ffffffffb4a4bba0 param_ops_byte  
> 0xffffffffc1077f38 ffffffffc1047080   .p......
> 0xffffffffc1077f40 000000785f69736d   msi_x...
> 0xffffffffc1077f48 656c5f6775626564   debug_le
> 0xffffffffc1077f50 00000000006c6576   vel.....
> 0xffffffffc1077f58 0000000000000000   ........

Thanks for testing and pointing this out. Patch indeed breaks
the CONFIG_KALLSYMS_ALL case. 

I think, possible fix would be to track the relevant sections in 
__layout_sections() and use defined symbols from those sections,
instead of just function symbols. 

Regards
Stanislaw

  reply	other threads:[~2026-03-25  8:27 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-17 11:04 [PATCH] module/kallsyms: sort function symbols and use binary search Stanislaw Gruszka
2026-03-23 13:06 ` Petr Pavlu
2026-03-24 12:53   ` Stanislaw Gruszka
2026-03-24 16:00     ` Petr Pavlu
2026-03-25  8:26       ` Stanislaw Gruszka [this message]
2026-03-25 10:02         ` Stanislaw Gruszka

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=20260325082648.GA18968@wp.pl \
    --to=stf_xl@wp.pl \
    --cc=atomlin@atomlin.com \
    --cc=da.gomez@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-modules@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=linux@jordanrome.com \
    --cc=live-patching@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=petr.pavlu@suse.com \
    --cc=rostedt@goodmis.org \
    --cc=samitolvanen@google.com \
    --cc=vmalik@redhat.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.