[PATCH v2] x86/resctrl: Annotate get_mem_config functions as __init

Nathan Chancellor posted 1 patch 2 months, 2 weeks ago
arch/x86/kernel/cpu/resctrl/core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH v2] x86/resctrl: Annotate get_mem_config functions as __init
Posted by Nathan Chancellor 2 months, 2 weeks ago
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.

While __rdt_get_mem_config_amd() does not exhibit a warning because it
does not call any __init code, it is a similar function that is only
called from __init code like __get_mem_config_intel(), so mark it __init
as well to keep the code symmetrical.

Link: https://github.com/llvm/llvm-project/commit/6b11573b8c5e3d36beee099dbe7347c2a007bf53 [1]
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
Changes in v2:
- Move position of __init within definition of __get_mem_config_intel()
  to better match coding style guidelines (Reinette).
- Apply __init to __rdt_get_mem_config_amd(), as it has the same issue
  by inspection (Reinette). Adjust commit message to reflect this
  change.
- Link to v1: https://lore.kernel.org/r/20240822-x86-restctrl-get_mem_config_intel-init-v1-1-8b0a68a8731a@kernel.org
---
 arch/x86/kernel/cpu/resctrl/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index 1930fce9dfe96d5c323cb9000fb06149916a5a3c..59961618a02374a5b1639baa7034d05867884640 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -199,7 +199,7 @@ static inline bool rdt_get_mb_table(struct rdt_resource *r)
 	return false;
 }
 
-static bool __get_mem_config_intel(struct rdt_resource *r)
+static __init bool __get_mem_config_intel(struct rdt_resource *r)
 {
 	struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
 	union cpuid_0x10_3_eax eax;
@@ -233,7 +233,7 @@ static bool __get_mem_config_intel(struct rdt_resource *r)
 	return true;
 }
 
-static bool __rdt_get_mem_config_amd(struct rdt_resource *r)
+static __init bool __rdt_get_mem_config_amd(struct rdt_resource *r)
 {
 	struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
 	u32 eax, ebx, ecx, edx, subleaf;

---
base-commit: 7424fc6b86c8980a87169e005f5cd4438d18efe6
change-id: 20240822-x86-restctrl-get_mem_config_intel-init-3af02a5130ba

Best regards,
-- 
Nathan Chancellor <nathan@kernel.org>
Re: [PATCH v2] x86/resctrl: Annotate get_mem_config functions as __init
Posted by Reinette Chatre 2 months, 1 week ago
Hi Nathan,

Just one nit in the subject ... this area has the custom to use "()" to
highlight that the name refers to a function, so rather:

	x86/resctrl: Annotate get_mem_config() functions as __init

On 9/13/24 4:27 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.
> 
> While __rdt_get_mem_config_amd() does not exhibit a warning because it
> does not call any __init code, it is a similar function that is only
> called from __init code like __get_mem_config_intel(), so mark it __init
> as well to keep the code symmetrical.
> 
> Link: https://github.com/llvm/llvm-project/commit/6b11573b8c5e3d36beee099dbe7347c2a007bf53 [1]
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> ---

Thank you very much.
With subject adjusted:

| Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>

Reinette
Re: [PATCH v2] x86/resctrl: Annotate get_mem_config functions as __init
Posted by Nathan Chancellor 2 months, 1 week ago
Hi Reinette,

On Mon, Sep 16, 2024 at 02:42:09PM -0700, Reinette Chatre wrote:
> Just one nit in the subject ... this area has the custom to use "()" to
> highlight that the name refers to a function, so rather:
> 
> 	x86/resctrl: Annotate get_mem_config() functions as __init

Thanks, I am aware of this custom since I do it in the commit message
but since get_mem_config() is a function in this file and it is not
exactly touched by this change, it did not really feel appropriate to
add the parentheses there.

I can send a v3 with this fixed if so desired or perhaps whoever is
going to apply this can just do it then?

> On 9/13/24 4:27 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.
> > 
> > While __rdt_get_mem_config_amd() does not exhibit a warning because it
> > does not call any __init code, it is a similar function that is only
> > called from __init code like __get_mem_config_intel(), so mark it __init
> > as well to keep the code symmetrical.
> > 
> > Link: https://github.com/llvm/llvm-project/commit/6b11573b8c5e3d36beee099dbe7347c2a007bf53 [1]
> > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> > ---
> 
> Thank you very much.
> With subject adjusted:
> 
> | Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>

Thanks a lot for the review!

Cheers,
Nathan
Re: [PATCH v2] x86/resctrl: Annotate get_mem_config functions as __init
Posted by Reinette Chatre 2 months, 1 week ago
Hi Nathan,

On 9/17/24 6:35 AM, Nathan Chancellor wrote:
> Hi Reinette,
> 
> On Mon, Sep 16, 2024 at 02:42:09PM -0700, Reinette Chatre wrote:
>> Just one nit in the subject ... this area has the custom to use "()" to
>> highlight that the name refers to a function, so rather:
>>
>> 	x86/resctrl: Annotate get_mem_config() functions as __init
> 
> Thanks, I am aware of this custom since I do it in the commit message
> but since get_mem_config() is a function in this file and it is not
> exactly touched by this change, it did not really feel appropriate to
> add the parentheses there.

hmmm ... I understand that () is used to highlight that the text is
referring to a function independent from how the function is impacted
by the change.

> 
> I can send a v3 with this fixed if so desired or perhaps whoever is
> going to apply this can just do it then?

Could you please send v3? That will make its inclusion smoother (pending
any other feedback from next level maintainers).

Thank you very much.

Reinette
Re: [PATCH v2] x86/resctrl: Annotate get_mem_config functions as __init
Posted by Nathan Chancellor 2 months, 1 week ago
On Tue, Sep 17, 2024 at 08:14:17AM -0700, Reinette Chatre wrote:
> Hi Nathan,
> 
> On 9/17/24 6:35 AM, Nathan Chancellor wrote:
> > Hi Reinette,
> > 
> > On Mon, Sep 16, 2024 at 02:42:09PM -0700, Reinette Chatre wrote:
> > > Just one nit in the subject ... this area has the custom to use "()" to
> > > highlight that the name refers to a function, so rather:
> > > 
> > > 	x86/resctrl: Annotate get_mem_config() functions as __init
> > 
> > Thanks, I am aware of this custom since I do it in the commit message
> > but since get_mem_config() is a function in this file and it is not
> > exactly touched by this change, it did not really feel appropriate to
> > add the parentheses there.
> 
> hmmm ... I understand that () is used to highlight that the text is
> referring to a function independent from how the function is impacted
> by the change.
> 
> > 
> > I can send a v3 with this fixed if so desired or perhaps whoever is
> > going to apply this can just do it then?
> 
> Could you please send v3? That will make its inclusion smoother (pending
> any other feedback from next level maintainers).

Alright sure, done: https://lore.kernel.org/20240917-x86-restctrl-get_mem_config_intel-init-v3-1-10d521256284@kernel.org/

Cheers,
Nathan