linux-kernel - Re: [PATCH v5 27/40] x86/resctrl: Move thread_throttle_mode

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <9ef7e709-7bc8-4bf5-b5b6-960e2eed5cfc@intel.com>
Date: Wed, 23 Oct 2024 15:59:38 -0700
From: Reinette Chatre <reinette.chatre@...el.com>
To: James Morse <james.morse@....com>, <x86@...nel.org>,
	<linux-kernel@...r.kernel.org>
CC: Fenghua Yu <fenghua.yu@...el.com>, Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>, H Peter Anvin
	<hpa@...or.com>, Babu Moger <Babu.Moger@....com>,
	<shameerali.kolothum.thodi@...wei.com>, D Scott Phillips OS
	<scott@...amperecomputing.com>, <carl@...amperecomputing.com>,
	<lcherian@...vell.com>, <bobo.shaobowang@...wei.com>,
	<tan.shaopeng@...itsu.com>, <baolin.wang@...ux.alibaba.com>, Jamie Iles
	<quic_jiles@...cinc.com>, Xin Hao <xhao@...ux.alibaba.com>,
	<peternewman@...gle.com>, <dfustini@...libre.com>, <amitsinght@...vell.com>,
	David Hildenbrand <david@...hat.com>, Rex Nie <rex.nie@...uarmicro.com>,
	"Dave Martin" <dave.martin@....com>, Shaopeng Tan
	<tan.shaopeng@...fujitsu.com>
Subject: Re: [PATCH v5 27/40] x86/resctrl: Move thread_throttle_mode_init() to
 be managed by resctrl

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@....com>
> Tested-by: Carl Worth <carl@...amperecomputing.com> # arm64
> Tested-by: Shaopeng Tan <tan.shaopeng@...fujitsu.com>
> Reviewed-by: Shaopeng Tan <tan.shaopeng@...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