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: <70e6b0a3-a18e-ff10-1df0-3b6790692bda@intel.com>
Date:   Thu, 15 Dec 2022 09:17:15 -0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Firefox/102.0 Thunderbird/102.6.0
Subject: Re: [PATCH v9 06/13] x86/resctrl: Add __init attribute to
 rdt_get_mon_l3_config()
Content-Language: en-US
To:     Babu Moger <babu.moger@amd.com>, <corbet@lwn.net>,
        <tglx@linutronix.de>, <mingo@redhat.com>, <bp@alien8.de>
CC:     <fenghua.yu@intel.com>, <dave.hansen@linux.intel.com>,
        <x86@kernel.org>, <hpa@zytor.com>, <paulmck@kernel.org>,
        <akpm@linux-foundation.org>, <quic_neeraju@quicinc.com>,
        <rdunlap@infradead.org>, <damien.lemoal@opensource.wdc.com>,
        <songmuchun@bytedance.com>, <peterz@infradead.org>,
        <jpoimboe@kernel.org>, <pbonzini@redhat.com>,
        <chang.seok.bae@intel.com>, <pawan.kumar.gupta@linux.intel.com>,
        <jmattson@google.com>, <daniel.sneddon@linux.intel.com>,
        <sandipan.das@amd.com>, <tony.luck@intel.com>,
        <james.morse@arm.com>, <linux-doc@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <bagasdotme@gmail.com>,
        <eranian@google.com>, <christophe.leroy@csgroup.eu>,
        <jarkko@kernel.org>, <adrian.hunter@intel.com>,
        <quic_jiles@quicinc.com>, <peternewman@google.com>
References: <166990882621.17806.16780480657453071426.stgit@bmoger-ubuntu>
 <166990900117.17806.4506067708943298894.stgit@bmoger-ubuntu>
From:   Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <166990900117.17806.4506067708943298894.stgit@bmoger-ubuntu>
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?MlpOSXY0dlBjRzEwWU95SjFiNjVxdkFHZVdNbEZlZjkyazRmZWE0bEh2ZkJC?=
 =?utf-8?B?Z2l4NEJHY1IvTVZ1djZtYmRvcFI2dngwV0czYy9xU1RJUzByb3Z2d2xqQ3dG?=
 =?utf-8?B?YXBhWXhNeXZXN0Y1aFd4c3F1bmlEWUM5WnZBak9HYzN0WG9lWUlPNUZNOTdW?=
 =?utf-8?B?T3pMU3ppOGljY05nOWF2cE0vT0JnZ0lwWktaK056UlB0YTExNFZ6MUpLRC9E?=
 =?utf-8?B?bzNNZkQxRHdmWnc4YXczNUR5UDhJWkt3ZitRMld6REM5RlR5aFJvYWNsSFhx?=
 =?utf-8?B?RzhiQzhyWUZ3ODhyZ3pzTkFZY0kyM3hsbGwzMXVEMy8wWWdFQyt1VXdJN0Zr?=
 =?utf-8?B?K3VlVFlWL3FnTTBOeTFZTGpESDdaVy9CMGo0bUNwUDVxQ0FDUGNLVnBIM1dE?=
 =?utf-8?B?ZjF3VnZnMVh0Y0lFdlRqM2I4aTcrdmVkanpGdW52aDAyWlBQSEpxMDVlSkVD?=
 =?utf-8?B?QmZrVXo1TWRoZU83bTFzS00rMVdPa2p2dUxnNnNvaE5pUzFNOHpmNTVpdDln?=
 =?utf-8?B?TUhmY1ZHcHA3VjA4UkgvajJ6bjFyaUZwZ3VVblFhTG81K0dENFBhZlFtUTM4?=
 =?utf-8?B?L3hnb09MeFhCUEd1WmFpeGJRVkN1TXdnZWovMkx6aElDcHFqY0RTMHFON3Mr?=
 =?utf-8?B?Q3hDVW1uN1p4U1dscWNBd0xGMFFBUVZlellDdlVCTm1VSlJVWmVVTlRMbk9F?=
 =?utf-8?B?cnFjNEs2U1k3K0dJUEwrcFEvS3FtUjYza0FOZXY1NHVibkdjajYxbTA1OXNo?=
 =?utf-8?B?YXFRN2ZESFozaHNXc241UnJORlYxZUdpR2ZZbTJZSmtQc0NMKzNlcFAzaVdK?=
 =?utf-8?B?MUlMWGdtOXloU2xZWEdSbGtSVzNYTnNXRG9nM1Q1akF3S1pUWWpJOHZCYWpo?=
 =?utf-8?B?dTFYclo4UzAzOHJZZmhucWNMQ3c3SDJDN1Zta01kdmFNSFBjRlU1NGUzU2xr?=
 =?utf-8?B?bkxWZC9FS1FTR1ZTTDR5cW50R241UEtFWEFteHNTU25EOFppUUpyUHd3ak5F?=
 =?utf-8?B?SWFWY2tXc0E2N3JKd1lhYW9nT09GdGNUL2pnS3BrVGZiSUtaQk9OMGJ0eG81?=
 =?utf-8?B?ejQvdXhOdkNRNml6T1pMMVBiUzUyQXpMRGk1b0hwS3JCaGxTVlZTWTlrRlZo?=
 =?utf-8?B?Y3VSbGh5a3JLSUIyOXAzclJBNndZU0p3ZkxQNlZPdGtPZ2U5UWMwUGUzcXc5?=
 =?utf-8?B?U2F2RW4xb3ZnWkYzcGI1cTROR0Z5VzRZN3RGZmtPUFNyUTMvQ0VXOFZUMXlv?=
 =?utf-8?B?RlRvM0tqa1NiUlhFbEEyTFUvVGRyQURPNU9yTkxsalpXOGFNRjY1bUU3L2Jm?=
 =?utf-8?B?MFBQMWpFTmNTdFg4Mzdlejc4czhCc3VPVFhkTThGZy9DUUNwdjNGYVJ5ZEl0?=
 =?utf-8?B?RWZvSHJjaTVrUnZxYTRGMFJnZVN1MU5lbVVOditwQkJMUXV4aEpnOUZFeEtk?=
 =?utf-8?B?bzd4SWJhQ3QzeUxGTklHRzZzTFV3WGhlQUI0QnlBZk9QT3B2SzNIeUZqSk16?=
 =?utf-8?B?N0p2TllwN1QvMXRWMmRKRnd2MzVyMjJ5QlEraGYwV3JRL1V0K1lGMGVQazB6?=
 =?utf-8?B?a0RTNm8wQy9namxZR0FYZHQyTHZqZGh6RHUybEJsSDVwUzdMZytkSGFrMzJ4?=
 =?utf-8?B?T0NRSGR0VGt2OGRuY0FsRlAvTHdWcy9VWjRXRFdxbndqbmIrSVZDUWZXVzVl?=
 =?utf-8?B?Qms3MVNNajlMRDQ1SE9mOUNMY0hKMU1abm53c21RSk83akhYN1lZaytnb2RH?=
 =?utf-8?B?OWlZREg4Y2wyUkMyMnVzQVdOakNCN0Q3SElpUFVWeDZjWHlrUENhbFJXU2po?=
 =?utf-8?B?ZG1EcUpEaW8zRnZZOVZCS1ZwNHVEQ3RkeGk1N2Y1amdMeGo1ekM1d2h3ZmZK?=
 =?utf-8?B?MXQrZHZJTUpXSFUvRDFKQi9OZ0RHS0dnWFArTXhRTzdocnNGTzYreVlGS0NI?=
 =?utf-8?B?eWlXM0s4N0gvTzk5U01odlJEN0ZIREZyRWc2L05UQ2lKeVAxTFMrRzM3OVlZ?=
 =?utf-8?B?V2dGYit0YUM0K3hUZTZ4bUE2V0FGZHZRL1JPVlJXZVozaUtMT1prQVl0d1Yw?=
 =?utf-8?B?SXc0T0dBRURNRzB1SGEzdGwvMG1vMFg5NUlCdnRmTFpyWjlRK3JZbEpsQmxW?=
 =?utf-8?B?Nm9XVDVPMVNtSnE0ZEgwUDQ0ODZiTC9nUHczNlBLRVh0cktMN000d3VPaVJu?=
 =?utf-8?B?cFE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: c3cf88e4-71e7-48b9-604f-08dadec0391f
X-MS-Exchange-CrossTenant-AuthSource: CY4PR11MB1862.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Dec 2022 17:17:20.2290
 (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: TUzfSY5Qt5y+nmtbFNH8ZFb5l+QsRJaQgtEjBX9uwJkcdnPPphH2gzlmgiiNHjdvh0QTme4YW9DJi7FSJUudMm1cJ+cp2NS8Js5qVhWv9UA=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB7608
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-7.1 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_HI,SPF_HELO_NONE,SPF_NONE 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

Hi Babu,

On 12/1/2022 7:36 AM, Babu Moger wrote:
> The function rdt_get_mon_l3_config() needs to call rdt_cpu_has() to

No need to say "The function" ... by using () after a name it is clear
that it is a function.

To support this change it could perhaps be:
"In an upcoming change rdt_get_mon_l3_config() needs to call
rdt_cpu_has() to ..."

> query the monitor related features. It cannot be called right now
> because rdt_cpu_has() has the __init attribute but rdt_get_mon_l3_config()
> doesn't. So, add the __init attribute to rdt_get_mon_l3_config() to
> resolve it.

Please place the solution description in a new paragraph and drop the "So,".
The description could also be expanded to support this change. For example:

"Add the __init attribute to rdt_get_mon_l3_config() that is only called
by get_rdt_mon_resources() that already has the __init attribute. Also
make rdt_cpu_has() available to by rdt_get_mon_l3_config() via
the internal header file."


> 
> Also, make the function rdt_cpu_has() available outside core.c file.
> 
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
>  arch/x86/kernel/cpu/resctrl/core.c     |    2 +-
>  arch/x86/kernel/cpu/resctrl/internal.h |    1 +
>  arch/x86/kernel/cpu/resctrl/monitor.c  |    2 +-
>  3 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> index b4fc851f6489..030d3b409768 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -728,7 +728,7 @@ static int __init set_rdt_options(char *str)
>  }
>  __setup("rdt", set_rdt_options);
>  
> -static bool __init rdt_cpu_has(int flag)
> +bool __init rdt_cpu_has(int flag)
>  {
>  	bool ret = boot_cpu_has(flag);
>  	struct rdt_options *o;
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index fdbbf66312ec..7bbfc10094b6 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -512,6 +512,7 @@ void closid_free(int closid);
>  int alloc_rmid(void);
>  void free_rmid(u32 rmid);
>  int rdt_get_mon_l3_config(struct rdt_resource *r);
> +bool rdt_cpu_has(int flag);

Please also add __init attribute here by using the same style as the other functions
in this file that need __init.

>  void mon_event_count(void *info);
>  int rdtgroup_mondata_show(struct seq_file *m, void *arg);
>  void mon_event_read(struct rmid_read *rr, struct rdt_resource *r,
> diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
> index efe0c30d3a12..e33e8d8bd796 100644
> --- a/arch/x86/kernel/cpu/resctrl/monitor.c
> +++ b/arch/x86/kernel/cpu/resctrl/monitor.c
> @@ -746,7 +746,7 @@ static void l3_mon_evt_init(struct rdt_resource *r)
>  		list_add_tail(&mbm_local_event.list, &r->evt_list);
>  }
>  
> -int rdt_get_mon_l3_config(struct rdt_resource *r)
> +int __init rdt_get_mon_l3_config(struct rdt_resource *r)
>  {
>  	unsigned int mbm_offset = boot_cpu_data.x86_cache_mbm_width_offset;
>  	struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
> 
> 

Thank you

Reinette