Message-ID: <6edbfe7b-aec4-2b3c-2f85-42e418ab3d99@intel.com>
Date:   Wed, 19 Jul 2023 17:12:33 +0200
From:   Alexander Lobakin <aleksander.lobakin@...el.com>
To:     Nick Alcock <nick.alcock@...cle.com>,
        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>, <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

From: Nick Alcock <nick.alcock@...cle.com>
Date: Wed, 19 Jul 2023 12:12:06 +0100

> [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

Three efforts[0] :D Mine went unnoticed unfortunately, so I switched to
other projects then.
My idea was to give relative path from the kernel root to the objfile,
as we have a good bunch of non-unique "filename + symbol name" pairs.

>> 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.)
> 

[0]
https://lore.kernel.org/linux-kbuild/20220818115306.1109642-1-alexandr.lobakin@intel.com

Thanks,
Olek

Powered by blists - more mailing lists