Message-ID: <874jm088ah.fsf@esperi.org.uk>
Date:   Wed, 19 Jul 2023 12:12:06 +0100
From:   Nick Alcock <nick.alcock@...cle.com>
To:     Alessandro Carminati <alessandro.carminati@...il.com>
Cc:     Luis Chamberlain <mcgrof@...nel.org>,
        Masahiro Yamada <masahiroy@...nel.org>,
        Nathan Chancellor <nathan@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Nicolas Schier <nicolas@...sle.eu>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Daniel Bristot de Oliveira <bristot@...nel.org>,
        Viktor Malik <vmalik@...hat.com>, linux-kernel@...r.kernel.org,
        linux-kbuild@...r.kernel.org, linux-trace-kernel@...r.kernel.org,
        Nick Alcock <nick.alcock@...cle.com>, eugene.loh@...cle.com,
        kris.van.hees@...cle.com, live-patching@...r.kernel.org
Subject: Re: [PATCH v2] scripts/link-vmlinux.sh: Add alias to duplicate
 symbols for kallsyms

[Reply concocted by hand out of lore.kernel.org archives because this
 entire thread has seemingly been mis-sorted somewhere into the stygian
 blackness of my mail spool and I can't find it anywhere: apologies if I
 messed anything up]

On Mon, Jul 17, 2023 at 10:52:40AM -0400, Steven Rostedt wrote:
>> Honestly, I think the "_alias_<some-random-number>" is useless. It doesn't
>> give you any clue to what function you are actually attaching to. 
>
> Agreed.

Agreed, and not *only* because I have a patch series that I think does
better. I'm also a consumer of this stuff (that's why I wrote the patch)
and I'm afraid I couldn't do anything with a bunch of random numbers.
What user interface are we supposed to show people? All it tells you is
that two symbols are different, not which is which or where they come
from.

This seems to be useful for the one case of "look up a symbol out of a
trace log and then filter by it or re-trace using it": no other use case
seems possible, because a number is just so little information. I'm also
hoping that people can say *before they run a trace* "I know I'm looking
at a problem with cgroups, I want to look at blah@...oup/cgroup.o"
without having to know beforehand that this is blah__alias_19404.

Actually, you probably *can* get something else out of plain numbers --
by looking at the aliased symbol name you got back, then looking at
/proc/kallmodsyms to see what other symbols it's textually close to,
guessing what object file they're in from their names and then grepping
around for what module that might be related to. i.e. doing by
eye-and-grep what my patch series is doing automatically.


A further problem: doing this using aliases in particular requires
modifying the tracers to ensure that they always report the aliased
symbol, not the non-aliased one at the same address. As it is, I suspect
a lot of them will either report whichever symbol comes first, or pick
one at random, or even try to disambiguate them themselves a second
time, leading to blah__alias_19404@...ething (double disambiguation) or
in some cases they'll simply hang. You might be lucky and see them pick
the longest aliased name, or whicheer one comes last, I suppose.

I think all the hanging tracers have been fixed in the last couple of
years but they certainly *used* to exist. This is of course a bug in
those tools, but it is easily overlookable when only a few symbols are
aliases: less so after this patch. Nonetheless, it is unarguable that
this series probably requires auditing (all?) kallsyms users, at least
to be sure they don't misbehave. (Mine doesn't need any of that, but
only because I'm using a new /proc file for that exact reason. People
migrating to it would also need adjustment.)

I know I've got too many concerns in one mail by now, but I'm *also* a
bit concerned about the space requirements of this: kallsyms's name
compression won't help compress all those textual numbers much. It's
certainly higher space requirements than my approach: it's just more
hidden because it's lumped into the existing kallsyms tables.


Alessandro: I'll have a look through your patch: after dumping on it
like this I think I have to. Alternate approaches are always worth
looking at :)

>> There's
>> been other approaches that show module and/or file names. I know there's
> > still some issues with getting those accepted, but I much rather have them
> > than this!
> > 
> > See: https://lore.kernel.org/all/20221205163157.269335-1-nick.alcock@oracle.com/
> 
> Yes, please coordinate with Nick and review each other's work, now we
> have two separate efforts with different reasons but hopefully we'll
> come back with one unified solution.

Yes indeed! I am getting back to this very soon (a few days), after a
prolonged hiatus getting USDT probing working in DTrace V2. Sorry about
that. (I suspect you wanted a break from my cover letters too!)

I think what my series really needs is someone capable of writing less
convoluted English, because my cover letter skills are still not
brilliant.

> Please Cc live-patching also, as they had suggested before just to
> provide the file filename + line number, that'll make it even more
> valuable.

That would be amazing! Nothing obvious occurs to me though.

My series could easily give objfile + address offset for every symbol
with little more overhead than we have now (we'd need to encode more
partial objfile names, that's all, low tens of KiB) -- the problem is
translating that address offset into a line number without a cripplingly
expensive storage-per-symbol lookup table. I mean if storing the size of
each symbol is too expensive (and it is, costing hundreds of KiB),
storing an offset->line number mapping probably would be too!

I'll think on it, maybe a trick to avoid representing every offset
individually will occur to me. (If any occurs to anyone else, I'd be
fascinated.)

-- 
NULL && (void)

Powered by blists - more mailing lists