Re: [PATCH v3 0/2] Fix kallsyms with CONFIG_LTO_CLANG

[Song Liu <songliubraving@meta.com>]

Hi Luis,

> On Aug 12, 2024, at 9:57 AM, Luis Chamberlain <mcgrof@kernel.org> wrote:
> 
> On Mon, Aug 12, 2024 at 09:21:02AM -0700, Song Liu wrote:
>> Hi folks,
>> 
>> Do we have more concerns and/or suggestions with this set? If not,
>> what would be the next step for it?
> 
> I'm all for simplifying things, and this does just that, however,
> I'm not the one you need to convince, the folks who added the original
> hacks should provide their Reviewed-by / Tested-by not just for CONFIG_LTO_CLANG
> but also given this provides an alternative fix, don't we want to invert
> the order so we don't regress CONFIG_LTO_CLANG ? And shouldn't the patches
> also have their respective Fixes tag?

kallsyms has got quite a few changes/improvements in the past few years:

1. Sami added logic to trim LTO hash in 2021 [1];
2. Zhen added logic to sort kallsyms in 2022 [2];
3. Yonghong changed cleanup_symbol_name() in 2023 [3]. 

In this set, we are undoing 1 and 3, but we keep 2. Shall we point Fixes
tag to [1] or [3]? The patch won't apply to a kernel with only [1] 
(without [2] and [3]); while this set is not just fixing [3]. So I think
it is not accurate either way. OTOH, the combination of CONFIG_LTO_CLANG
and livepatching is probably not used by a lot of users, so I guess we 
are OK without Fixes tags? I personally don't have a strong preference 
either way. 

It is not necessary to invert the order of the two patches. Only applying
one of the two patches won't cause more issues than what we have today. 

Thanks,
Song


> 
> Provided the commit logs are extended with Fixes and order is maintained
> to be able to bisect correctly:
> 
> Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
> 
>  Luis


[1] 8b8e6b5d3b01 ("kallsyms: strip ThinLTO hashes from static functions")
[2] 60443c88f3a8 ("kallsyms: Improve the performance of kallsyms_lookup_name()")
[3] 8cc32a9bbf29 ("kallsyms: strip LTO-only suffixes from promoted global functions")