Received-SPF: pass (google.com: domain of linux-kernel+bounces-15926-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <cf72ab6a-83b7-4609-8dec-7756f8ae5808@amd.com>
Date: Wed, 3 Jan 2024 13:43:18 -0600
User-Agent: Mozilla Thunderbird
From: "Moger, Babu" <babu.moger@amd.com>
Subject: Re: [PATCH v8 13/24] x86/resctrl: Queue mon_event_read() instead of
 sending an IPI
Reply-To: babu.moger@amd.com
To: James Morse <james.morse@arm.com>, x86@kernel.org,
 linux-kernel@vger.kernel.org
Cc: Fenghua Yu <fenghua.yu@intel.com>,
 Reinette Chatre <reinette.chatre@intel.com>,
 Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>,
 Borislav Petkov <bp@alien8.de>, H Peter Anvin <hpa@zytor.com>,
 shameerali.kolothum.thodi@huawei.com,
 D Scott Phillips OS <scott@os.amperecomputing.com>,
 carl@os.amperecomputing.com, lcherian@marvell.com,
 bobo.shaobowang@huawei.com, tan.shaopeng@fujitsu.com,
 baolin.wang@linux.alibaba.com, Jamie Iles <quic_jiles@quicinc.com>,
 Xin Hao <xhao@linux.alibaba.com>, peternewman@google.com,
 dfustini@baylibre.com, amitsinght@marvell.com
References: <20231215174343.13872-1-james.morse@arm.com>
 <20231215174343.13872-14-james.morse@arm.com>
Content-Language: en-US
In-Reply-To: <20231215174343.13872-14-james.morse@arm.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?WWtTT1B1UDZVTTFvRUdxMnYrdUN0RStKQ1puWTNTUG5CWHY3UlN3eFpzMEVV?=
 =?utf-8?B?Y3doa0tYdGcyUllsOE9MSW5pRHZCc2ZjdXZXN0xqRjBMK0RJWDNKeG5HeFZM?=
 =?utf-8?B?OGVtTm5xa0Zqd24xVmtXdmJ6dDJLRjNEMTI4Zmx3TG9lSjlZdGdMMnA1R2lN?=
 =?utf-8?B?VXNNYSs1L3MwQUdjVVJ2MnBLZDVaVmk5UUpMRWI2dG5VaGI0eWVhZExiWk9T?=
 =?utf-8?B?UGl0QW5ydEhISjBoUFg2cDAxQk5qMHdua1FoNVkybjg2amJUbXg0SHpUckEw?=
 =?utf-8?B?SmNMMW44R2RVVTRPUVhNb2tsQTEyL1o2T2VzdkdsQkkvWW40dlFwZ2Z4MEdJ?=
 =?utf-8?B?b1lRTXlxUi8vZFFUK0VKeDdjNmdvN3Y2NTZOZXZNWE1pUnpjQ1VlcXNLelY4?=
 =?utf-8?B?OXBYR1k3ak1hY3IvNDBPTGQwOGZZZE1tSmtnOTJLT2tsL3ExRnRCblVxK0NZ?=
 =?utf-8?B?akpUZWIyY1FkK1R6QVhvSGFVM3BkTmdhSXFFTmJidGE3eEV4Q0ltemFaclRY?=
 =?utf-8?B?VktIUy80RUQxTlk1bnRpNkdHendZK2pHUHlWMkdEa3FWdlBkcEwrZEszTW9B?=
 =?utf-8?B?VTI1N3lScDI3SDlGNWpUeVp1RlVVY0NvSVh6ZWpWZ254S0NkTERwTkpnWGhT?=
 =?utf-8?B?a0tKb3E2QmpHK2VKdXFSOU9GeEtGeXljOGk5RnNLbXhPWjRmekdjaWNDY29V?=
 =?utf-8?B?M21HS1RoclBMZnB6Nmd1dUVreFNiMzQwckZ0Ry9tWTZhTHA2M2F0SjZxMnZN?=
 =?utf-8?B?c3pHSEROa3RlTzBZbzNGcy80cFcwSlJndG9ReWtpU1g2U3lVWVRkQmVPbWZZ?=
 =?utf-8?B?YXZtd0dlaDVCdnhzR2M2MEhPdUg3MFZCQzVZVlA2dDZ2Q1prT0JGKzQ3Umdx?=
 =?utf-8?B?dWx2cVVGazVndGFvYUQ5UDFaWTJhVWZXQVZVdzBIeTBlV0ZkalArR2lvM0ZK?=
 =?utf-8?B?Y1QyNkVSMjhZZmR4RDNLWW1Rcmw3ek8xZFdrV2tOTEMrZnNkbEtSem40YnlN?=
 =?utf-8?B?N2c1UkNncUVyaHVadzM2Q1lab05haE4yaVFhUzZZeUY1UEE5MEVHcFNNYk5a?=
 =?utf-8?B?VjZnMVdNUWNTOW9qcFJrUzJNM256NUE1ZzlSVTdOKzA2REdVN1lLaExGNENT?=
 =?utf-8?B?dThWcmlLRXVjTmE0RXNaT0ErUXh6Y1dLdmxDb2lCbldFa2VRcHBZTm53TGNG?=
 =?utf-8?B?QlFYVkk1S05KQmoxWWxXYVlnck1kc0RBTXg2UEVoZ2tQYi94ZEV6Q3pNdzhi?=
 =?utf-8?B?MW5OdjhNdVVFM3Znbmp4dkxPaDlaa2RDWEFVc1RFd3FTcGZNQ3huMFc4USt6?=
 =?utf-8?B?RUlzTEpoM3ZVM0hyTllZdlZFV0RDUGwvckxLNS9UbmNESWJwNkFoQTlrV1Jq?=
 =?utf-8?B?QTZPQ1ZkK0hORWFmMkk2WmZFbGNVdHlaR1JFRHNWQVFOOTU5M1UycHdVdFlZ?=
 =?utf-8?B?TGJlcjBuTDhYZEF5QU91S2hXK1dyS3VHRWhWWFE4RGF4cG05eGpUWkFWR25S?=
 =?utf-8?B?VU9JSkU1aWpoaVNpVTRpOUZwSm0zeW5vcDVrUUZLbWp5V245dU1KbmhGaFM0?=
 =?utf-8?B?c0R3UG9VWjFFRGxvbVFKTWErTGVpaXNsRkZCbE1oL1JQam9pb00waGNpZUpw?=
 =?utf-8?B?V05Yb2gxN3ZBZkk0QkcvUXhTTEFMdVp3SFFrY2kvUDdBeXZrYzU4dG9BUU1t?=
 =?utf-8?B?bVRuckttbSthVTNxamYzNWFLL1FlSzd5TkFwcEI4eFYvVUhtQXhkZ2hvRDk0?=
 =?utf-8?B?YWU3VFJqMlIwQW5PUXcvQjk3Ti94MXNYYWw2Nlh5R2Q3K3VCczFWRHFQTTF0?=
 =?utf-8?B?MEVTQ0d5ZnUxUnpJTVlEUFR4MEdsWVFRSmZ5TEkrbE4xT2Z6MktHVlJ3Vkc1?=
 =?utf-8?B?akoraFFtbFBFdVBrOVlvTzJPYklDOWp5a0E3ckdIRllQYk9aSjBlQnBzdmcy?=
 =?utf-8?B?RFFyYlZ4K3hyL1hUVVZodHQxMEc4TWU3ZDY4UmZFMXZiajJPT2JuNGsydmZl?=
 =?utf-8?B?bFZoZXdtZHl5aW42aG5qYUFabExGc3YwVUVMdW00NThTb0hFM0lrRWpHNjN3?=
 =?utf-8?B?YjZCT0NKTkFMMVZaZmJSTjA0cVVuQjdwdG1VOXZBNkcvQVVyQzdYOVlPWncz?=
 =?utf-8?Q?nB6M=3D?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 31283422-a61e-4d7a-8d8d-08dc0c943d86
X-MS-Exchange-CrossTenant-AuthSource: MW3PR12MB4553.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Jan 2024 19:43:20.7972
 (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: wIUE25UQkDrWHDpAznYKPrULXCpiY+1eGRecdoJ1s+YY9REZKsL42ZqBRuvoFm/y
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS7PR12MB5813


On 12/15/23 11:43, James Morse wrote:
> Intel is blessed with an abundance of monitors, one per RMID, that can be
> read from any CPU in the domain. MPAMs monitors reside in the MMIO MSC,
> the number implemented is up to the manufacturer. This means when there are
> fewer monitors than needed, they need to be allocated and freed.
> 
> MPAM's CSU monitors are used to back the 'llc_occupancy' monitor file. The
> CSU counter is allowed to return 'not ready' for a small number of
> micro-seconds after programming. To allow one CSU hardware monitor to be
> used for multiple control or monitor groups, the CPU accessing the
> monitor needs to be able to block when configuring and reading the
> counter.
> 
> Worse, the domain may be broken up into slices, and the MMIO accesses
> for each slice may need performing from different CPUs.
> 
> These two details mean MPAMs monitor code needs to be able to sleep, and
> IPI another CPU in the domain to read from a resource that has been sliced.
> 
> mon_event_read() already invokes mon_event_count() via IPI, which means
> this isn't possible. On systems using nohz-full, some CPUs need to be
> interrupted to run kernel work as they otherwise stay in user-space
> running realtime workloads. Interrupting these CPUs should be avoided,
> and scheduling work on them may never complete.
> 
> Change mon_event_read() to pick a housekeeping CPU, (one that is not using
> nohz_full) and schedule mon_event_count() and wait. If all the CPUs
> in a domain are using nohz-full, then an IPI is used as the fallback.
> 
> This function is only used in response to a user-space filesystem request
> (not the timing sensitive overflow code).
> 
> This allows MPAM to hide the slice behaviour from resctrl, and to keep
> the monitor-allocation in monitor.c. When the IPI fallback is used on
> machines where MPAM needs to make an access on multiple CPUs, the counter
> read will always fail.
> 
> Signed-off-by: James Morse <james.morse@arm.com>
> Tested-by: Shaopeng Tan <tan.shaopeng@fujitsu.com>
> Tested-by: Peter Newman <peternewman@google.com>
> Tested-by: Babu Moger <babu.moger@amd.com>
> Reviewed-by: Shaopeng Tan <tan.shaopeng@fujitsu.com>
> Reviewed-by: Peter Newman <peternewman@google.com>
> Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>

Reviewed-by: Babu Moger <babu.moger@amd.com>

> ---
> Changes since v2:
>  * Use cpumask_any_housekeeping() and fallback to an IPI if needed.
> 
> Changes since v3:
>  * Actually include the IPI fallback code.
> 
> Changes since v4:
>  * Tinkered with existing capitalisation.
> 
> Changes since v5:
>  * Added a newline.
> 
> Changes since v6:
>  * Moved lockdep annotations to a later patch.
> ---
>  arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 26 +++++++++++++++++++++--
>  arch/x86/kernel/cpu/resctrl/monitor.c     |  2 +-
>  2 files changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
> index beccb0e87ba7..d07f99245851 100644
> --- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
> +++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
> @@ -19,6 +19,8 @@
>  #include <linux/kernfs.h>
>  #include <linux/seq_file.h>
>  #include <linux/slab.h>
> +#include <linux/tick.h>
> +
>  #include "internal.h"
>  
>  /*
> @@ -522,12 +524,21 @@ int rdtgroup_schemata_show(struct kernfs_open_file *of,
>  	return ret;
>  }
>  
> +static int smp_mon_event_count(void *arg)
> +{
> +	mon_event_count(arg);
> +
> +	return 0;
> +}
> +
>  void mon_event_read(struct rmid_read *rr, struct rdt_resource *r,
>  		    struct rdt_domain *d, struct rdtgroup *rdtgrp,
>  		    int evtid, int first)
>  {
> +	int cpu;
> +
>  	/*
> -	 * setup the parameters to send to the IPI to read the data.
> +	 * Setup the parameters to pass to mon_event_count() to read the data.
>  	 */
>  	rr->rgrp = rdtgrp;
>  	rr->evtid = evtid;
> @@ -536,7 +547,18 @@ void mon_event_read(struct rmid_read *rr, struct rdt_resource *r,
>  	rr->val = 0;
>  	rr->first = first;
>  
> -	smp_call_function_any(&d->cpu_mask, mon_event_count, rr, 1);
> +	cpu = cpumask_any_housekeeping(&d->cpu_mask);
> +
> +	/*
> +	 * cpumask_any_housekeeping() prefers housekeeping CPUs, but
> +	 * are all the CPUs nohz_full? If yes, pick a CPU to IPI.
> +	 * MPAM's resctrl_arch_rmid_read() is unable to read the
> +	 * counters on some platforms if its called in irq context.
> +	 */
> +	if (tick_nohz_full_cpu(cpu))
> +		smp_call_function_any(&d->cpu_mask, mon_event_count, rr, 1);
> +	else
> +		smp_call_on_cpu(cpu, smp_mon_event_count, rr, false);
>  }
>  
>  int rdtgroup_mondata_show(struct seq_file *m, void *arg)
> diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
> index 8737a9d6caef..7e81268137b0 100644
> --- a/arch/x86/kernel/cpu/resctrl/monitor.c
> +++ b/arch/x86/kernel/cpu/resctrl/monitor.c
> @@ -588,7 +588,7 @@ static void mbm_bw_count(u32 closid, u32 rmid, struct rmid_read *rr)
>  }
>  
>  /*
> - * This is called via IPI to read the CQM/MBM counters
> + * This is scheduled by mon_event_read() to read the CQM/MBM counters
>   * on a domain.
>   */
>  void mon_event_count(void *info)

-- 
Thanks
Babu Moger