Message-ID: <20250102150356.1372a947@gandalf.local.home>
Date: Thu, 2 Jan 2025 15:03:56 -0500
From: Steven Rostedt <rostedt@...dmis.org>
To: Peter Zijlstra <peterz@...radead.org>
Cc: linux-kernel@...r.kernel.org, linux-trace-kernel@...r.kernel.org,
 linux-kbuild@...r.kernel.org, bpf <bpf@...r.kernel.org>, Masami Hiramatsu
 <mhiramat@...nel.org>, Mark Rutland <mark.rutland@....com>, Mathieu
 Desnoyers <mathieu.desnoyers@...icios.com>, Andrew Morton
 <akpm@...ux-foundation.org>, Linus Torvalds
 <torvalds@...ux-foundation.org>, Masahiro Yamada <masahiroy@...nel.org>,
 Nathan Chancellor <nathan@...nel.org>, Nicolas Schier <nicolas@...sle.eu>,
 Zheng Yejian <zhengyejian1@...wei.com>, Martin Kelly
 <martin.kelly@...wdstrike.com>, Christophe Leroy
 <christophe.leroy@...roup.eu>, Josh Poimboeuf <jpoimboe@...hat.com>
Subject: Re: [PATCH 14/14] scripts/sorttable: ftrace: Do not add weak
 functions to available_filter_functions

On Thu, 2 Jan 2025 14:55:01 -0500
Steven Rostedt <rostedt@...dmis.org> wrote:

> On Thu, 2 Jan 2025 20:48:14 +0100
> Peter Zijlstra <peterz@...radead.org> wrote:
> 
> > *sigh*.. can we please just either add the 'hole' symbols in symtab, or
> > fix symtab to have entry size?
> > 
> > You're just fixing your one problem and leaving everybody else that has
> > extra data inside the dead weak things up a creek :/
> > 
> > Eg. if might make sense to also ignore alternative / static_branch /
> > static_call patching for such 'dead' code. Yes, that's not an immediate
> > problem atm, but just fixing __mcount_loc seems very short sighted.  
> 
> Read my reply to the email that I forgot to add to the cover letter (but
> mention in the last patch). Fixing kallsyms does not remove the place
> holders in the available_filter_functions. This has nothing to do with
> kallsyms. I need to remove the fentry/mcount references in the mcount_loc
> section.
> 
> The kallsyms is a completely different issue.

Maybe I misunderstood you, if you are not talking about kallsyms, but for
static calls or anything else that references weak functions.

The reference is not a problem I'm trying to address. The problem with
mcount_loc, is that it is used to create the ftrace_table that is exposed
to user space, and I can't remove entries once they are added.

To set filter functions you echo names into set_ftrace_filter. If you want
to enabled 5000 filters, that can take over a minute complete. That's
because echoing in names to set_ftrace_filter is an O(n^2) operation. It
has to search every address, call kallsyms on the address then compare it
to every function passed in. If you have 40,000 functions total, and pass
in 5,000 functions, that's 40,000 * 5,000 compares!

Since tooling is what does add these large number of filters, a shortcut
was added. If a number written into set_ftrace_filter, it doesn't do a
kallsyms lookup, it will enable the nth function in
available_filter_functions. This turns into a O(1) operation.

libtracefs() will read the available_filter_functions, figure out what to
enable from that, and then write the indexes of all the functions it wants
to enable. This is a much faster operation then echoing the names one at a
time.

This is where the weak functions becomes an issue. If I just ignore them,
and do not add a place holder in the mcount section. Then the index will be
off, and will break.

When the issue first came about, I simply ignored the weak functions, but
then my libtracefs self tests started to fail.

So yes, this is just fixing mcount_loc, but I believe it's the only one
that has a user interface issue.

-- Steve

Powered by blists - more mailing lists