Re: [PATCH v5 27/40] x86/resctrl: Move thread_throttle_mode_init() to be managed by resctrl

[Reinette Chatre <reinette.chatre@intel.com>]

Hi James,

On 10/4/24 11:03 AM, James Morse wrote:
> thread_throttle_mode_init() is called from the architecture specific code
> to make the 'thread_throttle_mode' file visible. The architecture specific
> code has already set the membw.throttle_mode in the rdt_resource.
> 
> This doesn't need to be specific to the architecture, the throttle_mode
> can be used by resctrl to determine if the 'thread_throttle_mode' file
> should be visible.
> 
> Call thread_throttle_mode_init() from resctrl_setup(), check the
> membw.throttle_mode on the MBA resource. This avoids publishing an
> extra function between the architecture and filesystem code.
> 
> Signed-off-by: James Morse <james.morse@arm.com>
> Tested-by: Carl Worth <carl@os.amperecomputing.com> # arm64
> Tested-by: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com>
> Reviewed-by: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com>
> ---
>  arch/x86/kernel/cpu/resctrl/core.c     | 1 -
>  arch/x86/kernel/cpu/resctrl/internal.h | 1 -
>  arch/x86/kernel/cpu/resctrl/rdtgroup.c | 9 ++++++++-
>  3 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> index b5ad1ed2a4de..0da7314195af 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -228,7 +228,6 @@ static bool __get_mem_config_intel(struct rdt_resource *r)
>  		r->membw.throttle_mode = THREAD_THROTTLE_PER_THREAD;
>  	else
>  		r->membw.throttle_mode = THREAD_THROTTLE_MAX;
> -	thread_throttle_mode_init();
>  
>  	r->alloc_capable = true;
>  
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index 9c08efb0e198..30de95e59129 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -495,7 +495,6 @@ void cqm_handle_limbo(struct work_struct *work);
>  bool has_busy_rmid(struct rdt_mon_domain *d);
>  void __check_limbo(struct rdt_mon_domain *d, bool force_free);
>  void rdt_domain_reconfigure_cdp(struct rdt_resource *r);
> -void __init thread_throttle_mode_init(void);
>  void __init mbm_config_rftype_init(const char *config);
>  void rdt_staged_configs_clear(void);
>  bool closid_allocated(unsigned int closid);
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index 3f10e6897daa..596f5f087834 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -2048,10 +2048,15 @@ static struct rftype *rdtgroup_get_rftype_by_name(const char *name)
>  	return NULL;
>  }
>  
> -void __init thread_throttle_mode_init(void)
> +static void __init thread_throttle_mode_init(void)
>  {
> +	struct rdt_resource *r = resctrl_arch_get_resource(RDT_RESOURCE_MBA);
>  	struct rftype *rft;
>  
> +	if (!r->alloc_capable ||
> +	    r->membw.throttle_mode == THREAD_THROTTLE_UNDEFINED)
> +		return;
> +

The goal from the changelog is to make "thread_throttle_mode_init()" not be specific
to an architecture. It does so by checking the value of rdt_resource->resctrl_membw->membw_throttle_mode.
I thus expect that as part of being non-architectural it should check this for all
resources that initialize resctrl_membw, this includes RDT_RESOURCE_SMBA.

Reinette