Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <6edbfe7b-aec4-2b3c-2f85-42e418ab3d99@intel.com>
Date:   Wed, 19 Jul 2023 17:12:33 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.12.0
Subject: Re: [PATCH v2] scripts/link-vmlinux.sh: Add alias to duplicate
 symbols for kallsyms
Content-Language: en-US
To:     Nick Alcock <nick.alcock@oracle.com>,
        Alessandro Carminati <alessandro.carminati@gmail.com>
CC:     Luis Chamberlain <mcgrof@kernel.org>,
        Masahiro Yamada <masahiroy@kernel.org>,
        Nathan Chancellor <nathan@kernel.org>,
        "Nick Desaulniers" <ndesaulniers@google.com>,
        Nicolas Schier <nicolas@fjasle.eu>,
        Masami Hiramatsu <mhiramat@kernel.org>,
        Daniel Bristot de Oliveira <bristot@kernel.org>,
        Viktor Malik <vmalik@redhat.com>,
        <linux-kernel@vger.kernel.org>, <linux-kbuild@vger.kernel.org>,
        <linux-trace-kernel@vger.kernel.org>, <eugene.loh@oracle.com>,
        <kris.van.hees@oracle.com>, <live-patching@vger.kernel.org>
References: <ZLVxUQiC5iF+xTPQ@bombadil.infradead.org>
 <20230714150326.1152359-1-alessandro.carminati@gmail.com>
 <20230717105240.3d986331@gandalf.local.home> <874jm088ah.fsf@esperi.org.uk>
From:   Alexander Lobakin <aleksander.lobakin@intel.com>
In-Reply-To: <874jm088ah.fsf@esperi.org.uk>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?ekxQZHE3T1Y4L3J6aXFtaXFpM09SL2tFc2FSMURkTDJwbDZsa0RsQWZ2S2s1?=
 =?utf-8?B?WGdzei9CRFRQSUJwYWNtTkw0VjM1M2VobnlDMGRiSFhsNWRKNEZ3bXBqWXV1?=
 =?utf-8?B?SGFhM2tGdjhlc2NaSTQ2VVNrNHliOTVMaGN3QTNORHFKRm5lS0Ywa2ZnRk9p?=
 =?utf-8?B?WmxWeHgyS3JkQnZJSFRsdTJGQWJBS0cvK3BUMHh5LytNOWZZVU1KMzJERHkx?=
 =?utf-8?B?WnhLZ1JsNXJ6MmJCRjR4VmxLOEdBbEpMVlhrZVlsblFWOGhCNm1YRHZITnQz?=
 =?utf-8?B?WXhYR3o0dWpjdy85VmVQbnNhd0ZqZWlueTFlbksxN2d0c1NRUXhSeWhUZk1W?=
 =?utf-8?B?anBwT0FpVkUyam4rODdXaEdpQXFtU3BBZXo2cDQremIvcEdwR3NEZ1RwNCtO?=
 =?utf-8?B?SnVtYXc5bkhZcUhyZnc1YlJ6b0RPNTFNd0ZpY3V4cDE5S094NzFkT2pDR1d4?=
 =?utf-8?B?eW9mMkNmTHpFazhvU3ZUMnBOTFQ5VU01cXRmbXB1STFmMUZNU2RBTHlMMWVQ?=
 =?utf-8?B?Q09IUFp0YWw3OUc0UWg0Q2VvRE9vOVl4VHM1eU82THlzYk9VdlBaeGZHSHZN?=
 =?utf-8?B?Q015Uk8rOUVyM0tWSjJKbzlKb3NQdC9Cb0ZWbFVJRGdpQzFXWHc0VFRyRTN2?=
 =?utf-8?B?Q1EzRWJPUk42azl6NjB5ZkRwNzJvT1JudUw5dkVmd3BpMXd2UUk5ZEQyKzJq?=
 =?utf-8?B?UTJRcHNOcXFjK2x5dm9uL0MycDRTaERTcVp0dVJ1TllyMVZYM2E5d3hidmw5?=
 =?utf-8?B?MUhEaXpFTm5BaTdTQml5bVlBTFFWVmZhQVpManhxbUtrQVNXM1hBZ29OZlFR?=
 =?utf-8?B?bVZTK0RBeXlGVzFHZWRmNnBTVFdUNHkvN3dyWnQ2OTV0aytaT0dHQ1FFdlFW?=
 =?utf-8?B?WWxOYU8yZFIxZzU0cGdlVDJhc0dKZlRMUW9hOGo5c0pOTjdUZnRwZXEzZFV5?=
 =?utf-8?B?VWdoU05LYW9YZldUUVJiZncycnM5emtlbE5WNzRQZllpVEJtQnBaSHEvaXBZ?=
 =?utf-8?B?NnpVZ2FnQ1lncmdZR2JuYkZ0QldpSU1MZGg3enh1YUhCamdKNTdKMlJQaFJa?=
 =?utf-8?B?VXIwUGU2dms3MWtpOWxjYnRDSmtteDZuRFBYSHQ0QU5OWm5ubE1qNWpGVWY4?=
 =?utf-8?B?ajVWVUJFN1FiTkdrKzh5VjhmVnE0cWcyUHM2bTF1c3ZwWXVwbWZRK3QwekFH?=
 =?utf-8?B?VERMWVVCNFFIaWM1TTVlUFUvZXVtdW90NlVpQmI2cCtzTmNGendSU1Iybk50?=
 =?utf-8?B?SUg1aVovM2tIQ0ViaThUL0tudkNVM0toUFY3VUIyQ2FHaDdiRWtvT0FOQWZj?=
 =?utf-8?B?d3pja0hkbDg4c2lmRVZNem45K2lVNm9yQ05rT0lGTGtDSVBnQlY1SmFjSVpQ?=
 =?utf-8?B?cnRrUVQweWx1UENZeGhoK1QrQi9tV0VkWUdrS3NIMVo1SXRLQ0xDL29vYzRs?=
 =?utf-8?B?T2hBWVBuY3pIUDduUmVHc2FSSGNvNzRlMnlNbGhZSzdtY0lSRnNXR1EwQml4?=
 =?utf-8?B?cEZDb2ZDR1NoYXgwY3I5RUk5UTVQdEFOZGpSaHV1bklZa0hYUFNYRVFHajJ0?=
 =?utf-8?B?Y2JFcjJ1RkhsVml4WTF2ek03cUM4ZWFFNEp1QU9rQkEvTlJoeG5RazNUeVEr?=
 =?utf-8?B?bXBUOEdnSEZFREwyVTdUa3QwS2YxOGd4YXpRNDdvR1hIV1o2Vzh6K2NIdXMw?=
 =?utf-8?B?Q3pJMUhsYklkdE9GSjFqZWdocGhXWHdod054UUJoRWtuVjdNMkJ5Y1NzbStu?=
 =?utf-8?B?MDM0Tmxhc3h4a0M2RXhRNkpGT3dLc1RoQ21nclkydkR3TnpxdGY3bjM1NHBO?=
 =?utf-8?B?SHRoQjBMNi85K3NWVUt5YmorTE4wVU5VOGkrWE16WWlNTENaL0FXaFdiTUdS?=
 =?utf-8?B?aWVDZEhBUmJBcWw3dnlzZExyclpxcjVHNEMwREtPN3Nvc010NFZRM2Rna2c4?=
 =?utf-8?B?RWd6clhLdzI1VWl1emhOZE4yVEVpNVhZMk5Ua2JramNBdEdnRndrbzRvNElY?=
 =?utf-8?B?dldDb09YdUljdFFBWUk5SmpleVRDc05NbTNkTlN4T2RobUIyVWJBaW9IM3Ju?=
 =?utf-8?B?RjNpd0xHeVVudVhibmN3bUhlb1oxTWRCc0prK24rb2FwQ3NKNkVBaSthWmlZ?=
 =?utf-8?B?L09mK1ZXYWg4c0x1VjNtVkp2c1J5V04velU0K1d4bnpoVHg5c1hiWXFBNkxI?=
 =?utf-8?B?Wnc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 15ef76f3-f6a0-4350-8105-08db886ad596
X-MS-Exchange-CrossTenant-AuthSource: DM6PR11MB3625.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Jul 2023 15:14:23.8441
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: bG6ED3m7bU2QjLInWUoOwaDTSXyMvrXtMcg4YFn1VC7aaqX88YSNKvZ5ndKwU6tQUxhHt2j7m4lrIoXq3eJhUfd+R6ftx2YdL+k/9vZXfTE=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW3PR11MB4523
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-2.2 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_BLOCKED,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,
        T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=ham autolearn_force=no
        version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

From: Nick Alcock <nick.alcock@oracle.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@cgroup/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@something (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