linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Reinette Chatre <reinette.chatre@intel.com>
To: James Morse <james.morse@arm.com>, <x86@kernel.org>,
	<linux-kernel@vger.kernel.org>
Cc: Fenghua Yu <fenghua.yu@intel.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	H Peter Anvin <hpa@zytor.com>, Babu Moger <Babu.Moger@amd.com>,
	<shameerali.kolothum.thodi@huawei.com>,
	Jamie Iles <jamie@nuviainc.com>,
	"D Scott Phillips OS" <scott@os.amperecomputing.com>,
	<lcherian@marvell.com>, <bobo.shaobowang@huawei.com>
Subject: Re: [PATCH v1 05/20] x86/resctrl: Create mba_sc configuration in the rdt_domain
Date: Wed, 1 Sep 2021 14:22:36 -0700	[thread overview]
Message-ID: <638a0c73-ed1b-c4d6-f5f2-2af3c2e39a35@intel.com> (raw)
In-Reply-To: <20210729223610.29373-6-james.morse@arm.com>

Hi James,

On 7/29/2021 3:35 PM, James Morse wrote:
> To support resctrl's MBA software controller, the architecture must provide
> a second configuration array to hold the mbps_val from user-space.
> 
> This complicates the interface between the architecture code.

This sentence seems incomplete. I was expecting something like " 
complicates the interface between the architecture code and ..."

> 
> Make the filesystem parts of resctrl create an array for the mba_sc
> values when the struct resctrl_schema is created. The software controller
> can be changed to use this, allowing the architecture code to only
> consider the values configured in hardware.
> 
> Signed-off-by: James Morse <james.morse@arm.com>
> ---
>   arch/x86/kernel/cpu/resctrl/internal.h |  1 -
>   arch/x86/kernel/cpu/resctrl/rdtgroup.c | 69 ++++++++++++++++++++++++++
>   include/linux/resctrl.h                | 13 +++++
>   3 files changed, 82 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index e12b55f815bf..a7e2cbce29d5 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -36,7 +36,6 @@
>   #define MBM_OVERFLOW_INTERVAL		1000
>   #define MAX_MBA_BW			100u
>   #define MBA_IS_LINEAR			0x4
> -#define MBA_MAX_MBPS			U32_MAX
>   #define MAX_MBA_BW_AMD			0x800
>   #define MBM_CNTR_WIDTH_OFFSET_AMD	20
>   
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index cf0db0b7a5d0..185f9bb992d1 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -2030,6 +2030,60 @@ static int mkdir_mondata_all(struct kernfs_node *parent_kn,
>   			     struct rdtgroup *prgrp,
>   			     struct kernfs_node **mon_data_kn);
>   
> +static int mba_sc_domain_allocate(struct rdt_resource *res,
> +				  struct rdt_domain *d)
> +{
> +	u32 num_closid = closid_free_map_len;
> +	int cpu = cpumask_any(&d->cpu_mask);
> +	int i;
> +
> +	d->mba_sc = kcalloc_node(num_closid, sizeof(*d->mba_sc),
> +				 GFP_KERNEL, cpu_to_node(cpu));
> +	if (!d->mba_sc)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < num_closid; i++)
> +		d->mba_sc[i].mbps_val = MBA_MAX_MBPS;
> +
> +	return 0;
> +}
> +

I had the same initial reaction as Jamie and noted your answer to him. 
Considering the intricate flow here could you please add some comments 
to these functions that explains the calling flows in support of their 
safety?

...

>   /**
>    * struct rdt_domain - group of CPUs sharing a resctrl resource
>    * @list:		all instances of this resource
> @@ -53,6 +64,7 @@ struct resctrl_staged_config {
>    * @cqm_work_cpu:	worker CPU for CQM h/w counters
>    * @plr:		pseudo-locked region (if any) associated with domain
>    * @staged_config:	parsed configuration to be applied
> + * @mba_sc:	the mba software controller properties, indexed by closid
>    */
>   struct rdt_domain {
>   	struct list_head		list;
> @@ -67,6 +79,7 @@ struct rdt_domain {
>   	int				cqm_work_cpu;
>   	struct pseudo_lock_region	*plr;
>   	struct resctrl_staged_config	staged_config[CDP_NUM_TYPES];
> +	struct resctrl_mba_sc		*mba_sc;
>   };

Why is this additional abstraction needed? As I understand the usage 
struct resctrl_mba_sc would always only have the one member so why not 
have mbps_val within rdt_domain?

Reinette

  parent reply	other threads:[~2021-09-01 21:22 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-29 22:35 [PATCH v1 00/20] x86/resctrl: Make resctrl_arch_rmid_read() return values in bytes James Morse
2021-07-29 22:35 ` [PATCH v1 01/20] x86/resctrl: Kill off alloc_enabled James Morse
2021-08-11 12:12   ` Jamie Iles
2021-09-01 21:18   ` Reinette Chatre
2021-07-29 22:35 ` [PATCH v1 02/20] x86/resctrl: Merge mon_capable and mon_enabled James Morse
2021-08-11 12:15   ` Jamie Iles
2021-08-11 15:16     ` James Morse
2021-07-29 22:35 ` [PATCH v1 03/20] x86/resctrl: Add domain online callback for resctrl work James Morse
2021-09-01 21:19   ` Reinette Chatre
2021-09-17 16:57     ` James Morse
2021-07-29 22:35 ` [PATCH v1 04/20] x86/resctrl: Add domain offline " James Morse
2021-08-11 16:10   ` Jamie Iles
2021-09-01 21:21   ` Reinette Chatre
2021-07-29 22:35 ` [PATCH v1 05/20] x86/resctrl: Create mba_sc configuration in the rdt_domain James Morse
2021-08-11 16:32   ` Jamie Iles
2021-08-31 16:24     ` James Morse
2021-09-01 21:22   ` Reinette Chatre [this message]
2021-09-17 16:57     ` James Morse
2021-07-29 22:35 ` [PATCH v1 06/20] x86/resctrl: Switch over to the resctrl mbps_val list James Morse
2021-09-01 21:25   ` Reinette Chatre
2021-09-17 16:57     ` James Morse
2021-09-17 18:20       ` Reinette Chatre
2021-10-01 16:02         ` James Morse
2021-07-29 22:35 ` [PATCH v1 07/20] x86/resctrl: Remove architecture copy of mbps_val James Morse
2021-09-01 21:26   ` Reinette Chatre
2021-09-17 16:57     ` James Morse
2021-07-29 22:35 ` [PATCH v1 08/20] x86/resctrl: Remove set_mba_sc()s control array re-initialisation James Morse
2021-07-29 22:35 ` [PATCH v1 09/20] x86/resctrl: Abstract and use supports_mba_mbps() James Morse
2021-09-01 21:27   ` Reinette Chatre
2021-09-17 16:57     ` James Morse
2021-09-24  6:23   ` tan.shaopeng
2021-10-01 16:01     ` James Morse
2021-07-29 22:36 ` [PATCH v1 10/20] x86/resctrl: Allow update_mba_bw() to update controls directly James Morse
2021-09-01 21:28   ` Reinette Chatre
2021-07-29 22:36 ` [PATCH v1 11/20] x86/resctrl: Calculate bandwidth from the total bytes counter James Morse
2021-09-01 21:31   ` Reinette Chatre
2021-09-17 16:58     ` James Morse
2021-07-29 22:36 ` [PATCH v1 12/20] x86/recstrl: Add per-rmid arch private storage for overflow and chunks James Morse
2021-08-11 17:14   ` Jamie Iles
2021-08-31 16:25     ` James Morse
2021-07-29 22:36 ` [PATCH v1 13/20] x86/recstrl: Allow per-rmid arch private storage to be reset James Morse
2021-09-24  6:34   ` tan.shaopeng
2021-10-01 16:01     ` James Morse
2021-07-29 22:36 ` [PATCH v1 14/20] x86/resctrl: Abstract __rmid_read() James Morse
2021-07-29 22:36 ` [PATCH v1 15/20] x86/resctrl: Pass the required parameters into resctrl_arch_rmid_read() James Morse
2021-07-29 22:36 ` [PATCH v1 16/20] x86/resctrl: Move mbm_overflow_count() " James Morse
2021-07-29 22:36 ` [PATCH v1 17/20] x86/resctrl: Move get_corrected_mbm_count() " James Morse
2021-07-29 22:36 ` [PATCH v1 18/20] x86/resctrl: Rename and change the units of resctrl_cqm_threshold James Morse
2021-07-29 22:36 ` [PATCH v1 19/20] x86/resctrl: Add resctrl_rmid_realloc_limit to abstract x86's boot_cpu_data James Morse
2021-07-29 22:36 ` [PATCH v1 20/20] x86/resctrl: Make resctrl_arch_rmid_read() return values in bytes James Morse

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=638a0c73-ed1b-c4d6-f5f2-2af3c2e39a35@intel.com \
    --to=reinette.chatre@intel.com \
    --cc=Babu.Moger@amd.com \
    --cc=bobo.shaobowang@huawei.com \
    --cc=bp@alien8.de \
    --cc=fenghua.yu@intel.com \
    --cc=hpa@zytor.com \
    --cc=james.morse@arm.com \
    --cc=jamie@nuviainc.com \
    --cc=lcherian@marvell.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=scott@os.amperecomputing.com \
    --cc=shameerali.kolothum.thodi@huawei.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    --subject='Re: [PATCH v1 05/20] x86/resctrl: Create mba_sc configuration in the rdt_domain' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).