linux-kernel - Re: [PATCH] x86/resctrl: Annotate __get_mem_config_intel() as _

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <b77ee442-77b5-48cd-89dd-3c6c487d767b@intel.com>
Date: Fri, 13 Sep 2024 14:40:04 -0700
From: Reinette Chatre <reinette.chatre@...el.com>
To: Nathan Chancellor <nathan@...nel.org>
CC: Fenghua Yu <fenghua.yu@...el.com>, <x86@...nel.org>, Thomas Gleixner
	<tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>, Borislav Petkov
	<bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>,
	<linux-kernel@...r.kernel.org>, <llvm@...ts.linux.dev>,
	<patches@...ts.linux.dev>
Subject: Re: [PATCH] x86/resctrl: Annotate __get_mem_config_intel() as __init

Hi Nathan,

On 9/13/24 12:41 PM, Nathan Chancellor wrote:
> Hi Reinette,
> 
> On Thu, Sep 12, 2024 at 03:33:09PM -0700, Reinette Chatre wrote:
>> Apologies for the delay.
> 
> No worries, this is not super high priority (except when the section
> mismatch warning is elevated to an error but that does not happen in too
> many real world configurations).
> 
>> On 8/22/24 5:12 PM, Nathan Chancellor wrote:
>>> After a recent LLVM change [1] that deduces __cold on functions that
>>> only call cold code (such as __init functions), there is a section
>>> mismatch warning from __get_mem_config_intel(), which got moved to
>>> .text.unlikely. as a result of that optimization:
>>>
>>>     WARNING: modpost: vmlinux: section mismatch in reference: __get_mem_config_intel+0x77 (section: .text.unlikely.) -> thread_throttle_mode_init (section: .init.text)
>>>
>>> Mark __get_mem_config_intel() as __init as well since it is only called
>>> from __init code, which clears up the warning.
>>
>> It looks to me as though __rdt_get_mem_config_amd() may need the same __init
>> treatment
> 
> It certainly looks like __init would be appropriate for
> __rdt_get_mem_config_amd(), although there is no current risk of a
> modpost warning like __get_mem_config_intel() because it does not call
> any __init functions, which is really what triggered this warning.

Ah I see ... I missed the part played by thread_throttle_mode_init().

> 
>> it is not clear to me why __get_mem_config_intel() would trigger
>> such warning, but not __rdt_get_mem_config_amd()?
> 
> Based on my understanding of the LLVM change linked below my comment
> here, __get_mem_config_intel() gets implicitly marked as __cold because
> it unconditionally calls thread_throttle_mode_init(), which is __cold
> through __init. If __get_mem_config_intel() does not get inlined into
> its caller (which could happen if a compiler decides not to optimize
> __cold code), that call to thread_throttle_mode_init() will appear to
> come from the .text section, even though it will really be from
> .init.text because __get_mem_config_intel() is only called from __init
> functions.
> 
> __rdt_get_mem_config_amd() does not call any cold functions so it avoids
> this problem altogether.

Thank you very much for the detailed explanation. Much appreciated.

> 
> I can send a v2 with __init added to __rdt_get_mem_config_amd() if you
> want, along with the style update you mention below. Just let me know
> what you prefer based on my comments above.

Could you please add __init to __rdt_get_mem_config_amd() also? I do understand
that it does not produce a warning today but __rdt_get_mem_config_amd() too is
only called from __init code. To me this already indicates that __init is
appropriate and ensuring its storage class is accurate protects against triggering
this warning in the future.

Thank you very much.

Reinette