Message-ID: <b85cc905-3ea2-9f73-a592-0ebdc4240c37@intel.com>
Date:   Thu, 2 Feb 2023 15:47:58 -0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Firefox/102.0 Thunderbird/102.6.1
Subject: Re: [PATCH v2 08/18] x86/resctrl: Queue mon_event_read() instead of
 sending an IPI
Content-Language: en-US
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>,
        D Scott Phillips OS <scott@os.amperecomputing.com>,
        <carl@os.amperecomputing.com>, <lcherian@marvell.com>,
        <bobo.shaobowang@huawei.com>, <tan.shaopeng@fujitsu.com>,
        <xingxin.hx@openanolis.org>, <baolin.wang@linux.alibaba.com>,
        Jamie Iles <quic_jiles@quicinc.com>,
        Xin Hao <xhao@linux.alibaba.com>, <peternewman@google.com>
References: <20230113175459.14825-1-james.morse@arm.com>
 <20230113175459.14825-9-james.morse@arm.com>
From:   Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <20230113175459.14825-9-james.morse@arm.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?Yis1UkRwekFDRnkxSm5oRER2bEJENWtHdVI1bVRoWk96ekx4Wjh2OVBwcGVM?=
 =?utf-8?B?TDdZQkNsTmt4M0FlNWVwSkNzZ3VXVG5vNGFpQUliTk9OUXNtL2pXTExoYWFI?=
 =?utf-8?B?S2JHRlNOYmcwdWJ2N3NMdlRJa2F1eldGdW9yeXAxWlNzSEFJODgyVHBlaFR2?=
 =?utf-8?B?ZkZrTnluR2xjK1hPR0hRRHRPeU1GSWxvR2dBN0hxYVh1ODhZQll0MTBVZGxI?=
 =?utf-8?B?WXY1aGZUVDMyL3E1VXh4K2tWOEF6YVVzRGVFU3hSM3poUFhpZkVXTUttMzlT?=
 =?utf-8?B?cTZZaWZwK2VORUxOeXNqS0dmaWNOakRKR2t6V3VHUTFpNEpwNVY1N2VMdU1H?=
 =?utf-8?B?c3AycWNmT3ZSSjJWcENod1pBeGFtTk83S3FqVDlhVnRadUh3M3dQQnZsNFVW?=
 =?utf-8?B?YlY3S1l1eDNlNWJENHhrMTJld3E0ZTdVYlRvKzlwWTNnTmVKN3JXZUJrbHRi?=
 =?utf-8?B?QytBSGZzSEFzTkQ1a2p4QjlZQnpHWUduWnRaamQzNWJ0Tk05cnlsRUF1NHVF?=
 =?utf-8?B?d080V1RRb0FiR0pSZVdIdSs1RitaR0JyL2hWekJpa1JBYlFjdXo2R2dKRVJo?=
 =?utf-8?B?am9RVjhYSDc5R1lpY3VyTWFUQnVuMHZlRjBNNTN3eGRlbEUxN3IwOXhqVm95?=
 =?utf-8?B?UERGOENianV3Z3Q1L2ZFMnd5Z3lxS3dUSkJWeGFkN3ljQWsvakZ2VWRUTEZx?=
 =?utf-8?B?TCtUT0pjbUZkN05DQTJUSFAyQlV1RUJJMys1aXpwWFJzYWRKNnZHZXR6Uklu?=
 =?utf-8?B?em5UTWxCdFVvZDJJU21DaCt6NFN5T29kUlFMUFJseCtacHY4dzJFVU0rTGJr?=
 =?utf-8?B?VnlJMkRGS0haaTVFdnFteVhkb3oxYzA5bWNGZThOV0x5d3ZHRzdyRlk2TlNM?=
 =?utf-8?B?dmJWWnZ5MXFJQnF6QktpT0RSOFJMYTdqUndzZG9Ccy9XVElKUUZkVkxvM01w?=
 =?utf-8?B?Zlgzc2Vubm52Qm1Ca2VWUERJSitmQ2hielRTOGVNbW5JRkdMcG5OL2pJS0xV?=
 =?utf-8?B?aERxS0p2ZU83c0dtaEw3cEllMHkxNFF1cVRQdFBFbXo4ZlozOER0cUp1THB4?=
 =?utf-8?B?SXJvcjBmcHVwbjh4VGVKaVo1V0IxTlVTeW93ZFkrVEYwV08wSzF2OWRJNmtZ?=
 =?utf-8?B?WENjOStTcDZCUTR1WS95UWk1TU1paHliQ2dtSnorc2l2RjlnN0k5ODNPTk1h?=
 =?utf-8?B?L0U3UTNpUSt4R0FYUURWMm0rK3VmMUN3Qis1YVI3TmlxRytwYTN6NkxmcGxh?=
 =?utf-8?B?UTg0WjZ6OTVzNW9JakFnS1NOT0hHR1lBbXc1eEx0OXhBYjduQjB6NEhhYmd4?=
 =?utf-8?B?cTJhWnZua0pWYmg2UzcrL0ZEdmJrcE5zamlEV1M0UFNlV1ovUjBNK2llZkVT?=
 =?utf-8?B?RXNmUDhtb3M2NmFUQ3VDT0RYem96bU80OFlqbUwyQ3h2ZFhzcyt2Nm1aZkJB?=
 =?utf-8?B?MWlQbTJYTkw4TWZqc2x3YXFXNmtudW5iRDg2UlN3WHVaTWlWdzRKWEFhc3dz?=
 =?utf-8?B?SnVOOTdxRnhQanltdzF5YVF0dWI2Y1NGR3B1dFN6ZEl4ZWJPYTJ6c2NKbGcz?=
 =?utf-8?B?YjRTM1lIWVFNSlQrT21yQ1l4OTlqUjVGVmsrcEgwUTV1eVc3WkIyNlNiT0Nm?=
 =?utf-8?B?UWMyZktPTS9nb3d1YlFqcW0rb0IySmo0Zmd1N25CRHV4Z0ZuWW5VdXRyQ0Ji?=
 =?utf-8?B?d0crc2ZZeHZCaVk4UHpnU1l4U01QcHBINTFsU2xlQVpneE9aSko1M3dtYXhn?=
 =?utf-8?B?RXB6OVFGbUdRb0ppY1ZRdGpkdmREWGlQUU4vdldBVGo0cUE5S1NEYjRaTHFq?=
 =?utf-8?B?eHlEemo4V3k4N2JaZFgzaGsyRlhoVzVlbDBHL0hyWkVjODBxbWUvY3BTS1p5?=
 =?utf-8?B?VjFCcGhWcWluNUlWVWljQUdLVjRjV0Y0SG1MZFZ4bS90eTN5YXAwa1J1cDRI?=
 =?utf-8?B?Mzdlc2RNbFp0cSt0SGVPbW52UUdHT0hTUE92KzNRenhNUEFqdUJBYXIxUDUz?=
 =?utf-8?B?MVlhS1Bma0NLaW56Sm8rMUkrZzhaMWJZUUQzQWI0YlphYjhDTDBFL2ZiVUF6?=
 =?utf-8?B?YzdRVXNvb2cwZlZNang0eFVoaFZPNUl3OUJpZENleGFMOHlCWVlVMDBiTjRR?=
 =?utf-8?B?aXhQSm9lTnp1WTBhQVVyTmZIMHIwUHNrR2I1Z29zeWhwOEVQSnVWVUV1R1cy?=
 =?utf-8?B?MXc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 0417dedc-a217-44ed-4467-08db0577ebab
X-MS-Exchange-CrossTenant-AuthSource: CY4PR11MB1862.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Feb 2023 23:48:01.8673
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: cXQqg8st57fBcZBn2zPeVisA+ujdN7oSdHPTzbGlMhzBY4eSq0g2aWekJdXwyA1RilITpUjtbAGx9S8XmApICctFiZYZueZkKbOUQ9t4ruI=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR11MB7295
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi James,

On 1/13/2023 9:54 AM, James Morse wrote:
> x86 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.
> 
> 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.
> 
> Change mon_event_read() to schedule mon_event_count() on a remote CPU and
> wait, instead of sending an IPI. 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.
> 
> Tested-by: Shaopeng Tan <tan.shaopeng@fujitsu.com>
> Signed-off-by: James Morse <james.morse@arm.com>
> ---
>  arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 7 +++++--
>  arch/x86/kernel/cpu/resctrl/internal.h    | 2 +-
>  arch/x86/kernel/cpu/resctrl/monitor.c     | 6 ++++--
>  3 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
> index 1df0e3262bca..4ee3da6dced7 100644
> --- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
> +++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
> @@ -532,8 +532,11 @@ void mon_event_read(struct rmid_read *rr, struct rdt_resource *r,
>  		    struct rdt_domain *d, struct rdtgroup *rdtgrp,
>  		    int evtid, int first)
>  {
> +	/* When picking a cpu from cpu_mask, ensure it can't race with cpuhp */

Please consistently use CPU instead of cpu.

> +	lockdep_assert_held(&rdtgroup_mutex);
> +
>  	/*
> -	 * 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;
> @@ -542,7 +545,7 @@ 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);
> +	smp_call_on_cpu(cpumask_any(&d->cpu_mask), mon_event_count, rr, false);

This would be problematic for the use cases where single tasks are run on
adaptive-tick CPUs. If an adaptive-tick CPU is chosen to run the function then
it may never run. Real-time environments are target usage of resctrl (with examples
in the documentation).

Reinette