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