Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) client-ip=2620:137:e000::3:5;
Message-ID: <2504daf3-16fa-9474-e9c5-cd6aa99d4776@amd.com>
Date:   Tue, 24 Oct 2023 18:20:17 -0500
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.0
Subject: Re: [PATCH] x86/resctrl: mba_MBps: Fall back to total b/w if local
 b/w unavailable
To:     Tony Luck <tony.luck@intel.com>, Fenghua Yu <fenghua.yu@intel.com>,
        Reinette Chatre <reinette.chatre@intel.com>,
        Peter Newman <peternewman@google.com>,
        Jonathan Corbet <corbet@lwn.net>,
        Shuah Khan <skhan@linuxfoundation.org>,
        "x86@kernel.org" <x86@kernel.org>
Cc:     Shaopeng Tan <tan.shaopeng@fujitsu.com>,
        James Morse <james.morse@arm.com>,
        Jamie Iles <quic_jiles@quicinc.com>,
        Randy Dunlap <rdunlap@infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
        "patches@lists.linux.dev" <patches@lists.linux.dev>
References: <20231024181600.8270-1-tony.luck@intel.com>
Content-Language: en-US
From:   "Moger, Babu" <bmoger@amd.com>
In-Reply-To: <20231024181600.8270-1-tony.luck@intel.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?RWJONlpYOE5JNUNBOGdjZ3BxRXpMdkxmVCtpaktXODAxVDlaRm9NVXA1WUdD?=
 =?utf-8?B?OG9Na2Nia1dRRWQ2YzYzWUlyUjhRdmdGSGQ2MDFuWGRJMEluOU0walZOTFla?=
 =?utf-8?B?c1M4SkRLcWdIUTRQOW9MdktrWGtlSE5obWhUWm1nVzJxSGxjd2M4S3NDTE8z?=
 =?utf-8?B?VVRlUlBCbExYVGhsdmhYc3V3Tm15Rjd0NnJMNkJ0UytqeHZUdSszdWpPK1cy?=
 =?utf-8?B?RkMxWWUvdnpnYXZNWDAzVHNwZjMvSml5QmNtSEl3MnlXM2E4d1hpTDg3VDV1?=
 =?utf-8?B?SFhkbFRFcU5LYmVlMW00NW9wZXZZR1RjdldCbUtRaUtINDl5QThseWNqa3l3?=
 =?utf-8?B?WWpuUnlVbkJIZzBlVGdiQS9BRU9nSFZXclFDY01Bd20xUFRWbWZSWkFwckhk?=
 =?utf-8?B?QmNIclNGT3RSbS9tbHZ3UmdBckdGeW9TdldDS09QZTBuYmNXMWQvbDk4alJs?=
 =?utf-8?B?YWJOYzlyM2lucWlmbFRyQ1ErYlRRWUdYdjhDcmc3VlZlOUtKUG5WclFHak5J?=
 =?utf-8?B?QlEyclFCaDlLc251cjJLaDNOSHRMV0pjZXNhODdVVThVUEEwRTBiSUIzRkEx?=
 =?utf-8?B?dEVxSzh3VWlOWjJxVk5uVDA0eE1Xc3Z2bnQwb2xNR2dsc0dRb214MXFGYmpK?=
 =?utf-8?B?dkMraGQ1dlU1YzlYWmROWTNHeEExZnRMUXZjZ3VKMHMyRlk0QndFNDFsZ2xh?=
 =?utf-8?B?QnhZRXFxYk12bFlvMDZNUEhtdTdlWllUamR5U3lwNkowa1NMZ0daWTlHeHhR?=
 =?utf-8?B?UzN0TWIxcnVYVGFBUXlHRHkvNHdmaEZCaUowVnRtYWVURGc2T3o3c2wxT3U5?=
 =?utf-8?B?cXlOV28vVElYb0ZTR2tEVUdla0NmcE81QzE5SXVFaHh3WUhHZ3lyc1V0bGMw?=
 =?utf-8?B?R2xuMGZ5dXBLdjFxby9IS0VLTEp4cUMvUXRKa1c3RTNrRDMyeVFyQ3VCTm1w?=
 =?utf-8?B?VGJnZStja2JDMElNY0tyc2hpbzlIN2NZZHJKT0xzeGcydm1XU3BmS1JxVjJQ?=
 =?utf-8?B?TC9TVi80KytsL3ZXZ0FUL3hMRGQ3c2JQZ2JyeG4raUpZUUZQanJ1RHhCM3ZD?=
 =?utf-8?B?VldoZ3JzaVVja2ZRRlpwcDdMUEJYME5GV2tKME5pYjR4NWt2VGdaWkFKUTd0?=
 =?utf-8?B?clpLR0N1QU8vNU13Z0pQUkRJQ2FNUENDd2JVZUJ1TllSN3RVSERCOHp4ZmRH?=
 =?utf-8?B?bTZyQ2phNTVwMko0N0ZlaW5OZVFtS2RMQjI4a29TYVFsVDZlRDNzUzFVVHdr?=
 =?utf-8?B?bUpZTWF0L3dtejJ4dldvN0UvcjVqejIxWWJkMnNPOHZINlRYZ1VXSGVTaU8y?=
 =?utf-8?B?Z3FDYysvbFZpNzllOWxxUGtPRVhxR21EZUR2REVYekJJTkVucGNFMmpwQ3g1?=
 =?utf-8?B?WXUxdHpEMmJaZjlpQjBnZTZDUjNTWXBENm5tOGducVd5WlF0TDRwS3p3MnIy?=
 =?utf-8?B?US9iWFBlYXNhK0tzWjZ6dlpLVGxxbkpmTXYwNkF2U3VZRmRTN3VWQ1hDV3k3?=
 =?utf-8?B?K0F4c2NuVUdwTUFwWU8vTi95T0NWNjNuVVkxZFJ0V1hFdUdWL1ZPMGkrMm5o?=
 =?utf-8?B?RUhTL0Z1WkhqZmlqbFJhYlNMYm84TEsvUEZKVzFBcitQMi9lQWRHbDVudnFH?=
 =?utf-8?B?cVU5b0FxTWdZUXhiRkpRYzlJVDE2SVlkd2hjOGxpclBTdHBrcFpKako1VVBs?=
 =?utf-8?B?VVlDeFJ6VlFsYVY3bmtXRkcvU3R2SDdQZmVQRmtFOGFNTVZkUDZRc09Reklw?=
 =?utf-8?B?aVJMdEM3Rk5DVTMzamd2UVkrWm9wcHM5eWpvU1k1YXE2ZGQ3ZTY5T29mVnY2?=
 =?utf-8?B?VXFBOXpyMUtzSTF0T2N5NFNkVEMwSEdvbmVXWVZxSnV2VmdJTXhFNTdtcG9m?=
 =?utf-8?B?QlhKc2ZiU0lHR3ZNWkdLd0E1MERWNFU3bVROK2JmVS9ydGRMUUJ0NjZZNkNo?=
 =?utf-8?B?dGxPM01OSTFyVjFYaHM2S3ZESDgzWi9pOXhyeHZmM3prbER0bGltUy82OVRt?=
 =?utf-8?B?VUF6c3hwdFpYWHZJUENXNGpPS3U0bFdiS2NWbTUyS3RmQ2hnMVJPeTlUaVVx?=
 =?utf-8?B?TEkxVzAzZStkOHl3YUZvMDh6Uy8vWnJha1VjVmo1ZVROUEZCbW9xYndsNC9D?=
 =?utf-8?Q?ejFk=3D?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c400ac07-f29a-4433-0117-08dbd4e7ca78
X-MS-Exchange-CrossTenant-AuthSource: MW3PR12MB4553.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 24 Oct 2023 23:20:20.3762
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 7ytAAkkEqasuCZFiyV5vXv+WzWDHx4EvNEcDyi2uLHPkfHUlm1bsMiVrOSHbsXtI
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR12MB6696
X-Spam-Status: No, score=-4.2 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS
	autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on groat.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (groat.vger.email [0.0.0.0]); Tue, 24 Oct 2023 16:20:43 -0700 (PDT)

Hi Tony,

> -----Original Message-----
> From: Tony Luck <tony.luck@intel.com>
> Sent: Tuesday, October 24, 2023 1:16 PM
> To: Fenghua Yu <fenghua.yu@intel.com>; Reinette Chatre
> <reinette.chatre@intel.com>; Peter Newman <peternewman@google.com>;
> Jonathan Corbet <corbet@lwn.net>; Shuah Khan <skhan@linuxfoundation.org>;
> x86@kernel.org
> Cc: Shaopeng Tan <tan.shaopeng@fujitsu.com>; James Morse
> <james.morse@arm.com>; Jamie Iles <quic_jiles@quicinc.com>; Moger, Babu
> <Babu.Moger@amd.com>; Randy Dunlap <rdunlap@infradead.org>; linux-
> kernel@vger.kernel.org; linux-doc@vger.kernel.org; patches@lists.linux.dev;
> Tony Luck <tony.luck@intel.com>
> Subject: [PATCH] x86/resctrl: mba_MBps: Fall back to total b/w if local b/w
> unavailable
> 
> On Intel the various resource director technology (RDT) features are all
> orthogonal and independently enumerated. Thus it is possible to have a system
> that  provides "total" memory bandwidth measurements without providing
> "local" bandwidth measurements.
> 
> If local bandwidth measurement is not available, do not give up on providing
> the "mba_MBps" feedback option completely, make the code fall back to using
> total bandwidth.


Is this customer requirement ?
What do you mean by " If local bandwidth measurement is not available" ?
Is the hardware supports only total bandwidth and not local?

It can get real ugly if we try to handle one special case.

thanks
Babu

> 
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---
>  arch/x86/kernel/cpu/resctrl/monitor.c  | 34 ++++++++++++++++----------
> arch/x86/kernel/cpu/resctrl/rdtgroup.c |  2 +-
>  2 files changed, 22 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c
> b/arch/x86/kernel/cpu/resctrl/monitor.c
> index f136ac046851..3b9531cce807 100644
> --- a/arch/x86/kernel/cpu/resctrl/monitor.c
> +++ b/arch/x86/kernel/cpu/resctrl/monitor.c
> @@ -418,6 +418,14 @@ static int __mon_event_count(u32 rmid, struct
> rmid_read *rr)
>  	return 0;
>  }
> 
> +static struct mbm_state *get_mbm_data(struct rdt_domain *dom_mbm, int
> +rmid) {
> +	if (is_mbm_local_enabled())
> +		return &dom_mbm->mbm_local[rmid];
> +
> +	return &dom_mbm->mbm_total[rmid];
> +}
> +
>  /*
>   * mbm_bw_count() - Update bw count from values previously read by
>   *		    __mon_event_count().
> @@ -431,7 +439,7 @@ static int __mon_event_count(u32 rmid, struct
> rmid_read *rr)
>   */
>  static void mbm_bw_count(u32 rmid, struct rmid_read *rr)  {
> -	struct mbm_state *m = &rr->d->mbm_local[rmid];
> +	struct mbm_state *m = get_mbm_data(rr->d, rmid);
>  	u64 cur_bw, bytes, cur_bytes;
> 
>  	cur_bytes = rr->val;
> @@ -526,14 +534,14 @@ static void update_mba_bw(struct rdtgroup *rgrp,
> struct rdt_domain *dom_mbm)
>  	struct list_head *head;
>  	struct rdtgroup *entry;
> 
> -	if (!is_mbm_local_enabled())
> +	if (!is_mbm_enabled())
>  		return;
> 
>  	r_mba = &rdt_resources_all[RDT_RESOURCE_MBA].r_resctrl;
> 
>  	closid = rgrp->closid;
>  	rmid = rgrp->mon.rmid;
> -	pmbm_data = &dom_mbm->mbm_local[rmid];
> +	pmbm_data = get_mbm_data(dom_mbm, rmid);
> 
>  	dom_mba = get_domain_from_cpu(smp_processor_id(), r_mba);
>  	if (!dom_mba) {
> @@ -553,7 +561,7 @@ static void update_mba_bw(struct rdtgroup *rgrp,
> struct rdt_domain *dom_mbm)
>  	 */
>  	head = &rgrp->mon.crdtgrp_list;
>  	list_for_each_entry(entry, head, mon.crdtgrp_list) {
> -		cmbm_data = &dom_mbm->mbm_local[entry->mon.rmid];
> +		cmbm_data = get_mbm_data(dom_mbm, entry->mon.rmid);
>  		cur_bw += cmbm_data->prev_bw;
>  		delta_bw += cmbm_data->delta_bw;
>  	}
> @@ -595,7 +603,7 @@ static void update_mba_bw(struct rdtgroup *rgrp,
> struct rdt_domain *dom_mbm)
>  	 */
>  	pmbm_data->delta_comp = true;
>  	list_for_each_entry(entry, head, mon.crdtgrp_list) {
> -		cmbm_data = &dom_mbm->mbm_local[entry->mon.rmid];
> +		cmbm_data = get_mbm_data(dom_mbm, entry->mon.rmid);
>  		cmbm_data->delta_comp = true;
>  	}
>  }
> @@ -621,15 +629,15 @@ static void mbm_update(struct rdt_resource *r, struct
> rdt_domain *d, int rmid)
>  		rr.evtid = QOS_L3_MBM_LOCAL_EVENT_ID;
>  		rr.val = 0;
>  		__mon_event_count(rmid, &rr);
> -
> -		/*
> -		 * Call the MBA software controller only for the
> -		 * control groups and when user has enabled
> -		 * the software controller explicitly.
> -		 */
> -		if (is_mba_sc(NULL))
> -			mbm_bw_count(rmid, &rr);
>  	}
> +
> +	/*
> +	 * Call the MBA software controller only for the
> +	 * control groups and when user has enabled
> +	 * the software controller explicitly.
> +	 */
> +	if (is_mba_sc(NULL))
> +		mbm_bw_count(rmid, &rr);
>  }
> 
>  /*
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index 69a1de92384a..0c4f8a1b8df0 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -2294,7 +2294,7 @@ static bool supports_mba_mbps(void)  {
>  	struct rdt_resource *r =
> &rdt_resources_all[RDT_RESOURCE_MBA].r_resctrl;
> 
> -	return (is_mbm_local_enabled() &&
> +	return (is_mbm_enabled() &&
>  		r->alloc_capable && is_mba_linear());  }
> 
> --
> 2.41.0