Received-SPF: pass (google.com: domain of linux-kernel+bounces-136175-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <0cbc22c1-a35c-4ad8-8f3b-f7a7b034d944@intel.com>
Date: Mon, 8 Apr 2024 20:13:40 -0700
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v1 01/31] x86/resctrl: Fix allocation of cleanest CLOSID
 on platforms with no monitors
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>, <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>,
	David Hildenbrand <david@redhat.com>, Rex Nie <rex.nie@jaguarmicro.com>,
	"Dave Martin" <dave.martin@arm.com>
References: <20240321165106.31602-1-james.morse@arm.com>
 <20240321165106.31602-2-james.morse@arm.com>
Content-Language: en-US
From: Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <20240321165106.31602-2-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?Sld1dFFIWnlaU3AvbTVZTjh6R2FuZXFjL1l6bDlhN2szMHZiZy9QQ1dFNStS?=
 =?utf-8?B?UXZYcm85ZzBvVXI1M3gzeWRzQXlYbTI4by9leElTdGdkQkxTcUdITXBYL0p4?=
 =?utf-8?B?MDg2Qzc2MGZONGlvMkdoQ2l0OVNyaVdTbTMvd0NxenJvdDVhRnVpYlNPN002?=
 =?utf-8?B?TTBweURMazZmUndkWVdRZk5veW9rbWk4akZZbmwrc2NaRU5SQS9kQ2pVTkM5?=
 =?utf-8?B?RUs4dWlCaXB5ZkJzMSsweGJQbHZQWDVZUkVndlB6MlJrRlNuZUE2aFZjNEpX?=
 =?utf-8?B?MGxKK1V2d21hZ3VDNDNQNWUxMDVqQ2h0MmhZb3Bxakt5enNta2E4VUx2M1g1?=
 =?utf-8?B?QnhYZmxYVjA1ZEl1RjV1b0pIK1BnOTZ0R2x2Z04xbWgwdmNJeWZpVVFaK2VT?=
 =?utf-8?B?RFlxaDdjZDVkS1IvOWhZam90WkRLVExLd2hnRlpBRFoyWld4L2h6N25YM0Zx?=
 =?utf-8?B?d1NIQ3Y4cWFmbldSYWdSYVFNUmx6bmM0YmNqZjBTQmQ0Ym9jVWhhTE5aTXhY?=
 =?utf-8?B?eG1OT25FYzlZb3FtTXYyeVFaRlZoMlcwWnhqdndWMzZnMTZUc1FQSHRuR01a?=
 =?utf-8?B?dGVXME8xM1B6WGZpZjI5TTJpZ3ExcXFENkM5Mnp4NzB3d3pIN3lpcXdPTksy?=
 =?utf-8?B?akpMbHRSNWVaN3dZaUFhWU1VdGo4Z1NCUmgxT3hTWUJ0YnVrRi8rd29iaVJL?=
 =?utf-8?B?dFlWbzYzQ3B2YlJ3VGFOWXFXNzBzOGtzVDJIekZiQUFjMk1PQTA5bzhVWUo4?=
 =?utf-8?B?RnFvSzVzc1gwL2J0VVVHQUN6YWRvSjZ2d0hyTFlQWXJITVFhUkQxY2NqRTUx?=
 =?utf-8?B?aE84dHNSaWdDNFNITTQ3eXhJWkNUR3FEQXNnemdUaUVKUFEyOVdOakNGY2dG?=
 =?utf-8?B?L3pGVDVoekpuVXByNkd0RmxsQnF0ZE5mMm15d3N1SlN6N2tYdFBYY2RYeXBK?=
 =?utf-8?B?RnJoeGlHb2NmakcxV1MyK2pwSzdRV3BhY0cwbEtDRG04aWluYkVyQlJqRG1O?=
 =?utf-8?B?c213UnNuV1V0eC9PTGdUYm0xODlsVEJmREZrQnFsb3A4REpKQ1BnZ1liQVNi?=
 =?utf-8?B?UE83V0VZRmU1VmZvbG1jRlR4cW1pRFROTUFLeXY2aTVObHpvQVRrajZRdmxR?=
 =?utf-8?B?RmIvNS9LRk5OYzRnazlnUXJZZm1yOGNZYjFXcUtUVkpwdFpsaUFRV0FkeWZZ?=
 =?utf-8?B?eWhiNktYekJMbWxaeWdUR1lmaHZZVkZXeFlHSjJvbGw0TmJ3NkFXTGw1c25z?=
 =?utf-8?B?em9ZZWhJbUtuVXVyM280cTNCTlB6VWRNOXkzY0k2SmFiYWdMa3hrUVFlbW5D?=
 =?utf-8?B?UW9odHZPSGl3bFlYYnkxb2JKa2swZEt3ditPN1ZYRVpnS0gzbmorbTBjTmR1?=
 =?utf-8?B?dnJpWVR6dWl0azlMNUcwN056UTB2N0d2em9lRC8rd2t4Sm5SVHVrMFBnWHd2?=
 =?utf-8?B?OEZJZC9oTlFLMHBUaGdzRkYzYXU3YzI3c000WlpDNTUrQStRMmZCNEFWTmVG?=
 =?utf-8?B?RjdST0J2QmtYTEg0RXZqZUI0dHg0Q21RVlg3MjU0YXorODU4Z3ByVEI5aStw?=
 =?utf-8?B?QjVZYW1lWmU4WEhocHF3UWYya2YrempHS2d6Q3dkaHMvQVIraGtRNGxSVW1Q?=
 =?utf-8?B?cWw2M2U0UTFBdUE5eFpyMmNTSmJLaThHRG94VWV1K203aFhKenFNNkZ4UFkx?=
 =?utf-8?B?RjlTVGlPN2U4WHNFelRjWjdDaHJUNEpBdVI3NWpnNlhjZlhCbWtFMlJTaFI1?=
 =?utf-8?B?bjBadnZVZ0VUTU9Yd1A2aTN1NlZOODhvSVFWdW5GSFdBcHpIcHozS21SNW9i?=
 =?utf-8?B?NmdsNldXc1ZFSml6TVMwelZEb001a0ViYUFlTzNjU08xcVpCQzBFZ2VkMUN0?=
 =?utf-8?B?blJCanN2d0VuOCt2ZmloaTFXZGFMM3p3VjdMMFNUcUFCOUZBS05zWHZZR3pS?=
 =?utf-8?B?RHliL0hqYSsrOWE4MFRTZXZzczlnL2EzU213R0xPeU1pdTA2VkhMM29YZXZj?=
 =?utf-8?B?cWVEbHByMmlNRjhlNldUVi9vaVRJYmlQRFJHNUl4ZXRjT1FIWi83cEovcTNB?=
 =?utf-8?B?S1FjV0dHa21jMmVUY0I5OFJkRGJjM091ZUpYWjJCbWozaUlOc01wOHF5aG0v?=
 =?utf-8?B?YmFHWFczS0hiY25kamxOQnd4Y3FUcUtYcDh5SG5FV3VXUVl2bE9EV3NlSjhP?=
 =?utf-8?B?S3c9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 934535d9-d317-4a54-a603-08dc58430fe3
X-MS-Exchange-CrossTenant-AuthSource: SJ2PR11MB7573.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Apr 2024 03:13:43.3329
 (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: 2YUniORiL0+Pef3Y6IE9yVNZ2WlGtqAiIKI//usn87EWk4T4UCCdP5TcS1exJYBWWlAC7TuzQD8zsoITpeIu8E8rfOFWAL+E+AdczWOdSUU=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR11MB4783
X-OriginatorOrg: intel.com

Hi James,

On 3/21/2024 9:50 AM, James Morse wrote:
> commit 6eac36bb9eb0 ("x86/resctrl: Allocate the cleanest CLOSID by
> searching closid_num_dirty_rmid") added a Kconfig option that causes

Please see section about "Fixes" tags in section "Ordering of commit tags"
in Documentation/process/maintainer-tip.rst

> resctrl to search for the CLOSID with the fewest dirty cache lines when
> creating a new control group. This depends on the values read from the
> llc_occupancy counters.
> 
> This support missed that some platforms may not have these counters.
> This causes a NULL pointer dereference when creating a new control
> group as the array was not allocated by dom_data_init().
> 
> As this feature isn't necessary on platforms that don't have cache
> occupancy monitors, add this to the check that occurs when a new
> control group is allocated.
> 
> The existing code is not selected by any upstream platform, it makes
> no sense to backport this patch to stable.
> 
> Signed-off-by: James Morse <james.morse@arm.com>
> ---
>  arch/x86/kernel/cpu/resctrl/rdtgroup.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index 011e17efb1a6..1767c1affa60 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -149,7 +149,8 @@ static int closid_alloc(void)
>  
>  	lockdep_assert_held(&rdtgroup_mutex);
>  
> -	if (IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID)) {
> +	if (IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID) &&
> +	    is_llc_occupancy_enabled()) {
>  		cleanest_closid = resctrl_find_cleanest_closid();
>  		if (cleanest_closid < 0)
>  			return cleanest_closid;

Patch looks good.

Thank you

Reinette