Received-SPF: pass (google.com: domain of linux-kernel+bounces-145874-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <36bdfb3c-2828-4188-88b1-b9d01b2a114f@amd.com>
Date: Mon, 15 Apr 2024 15:27:42 -0500
User-Agent: Mozilla Thunderbird
From: "Moger, Babu" <babu.moger@amd.com>
Subject: Re: [PATCH v1 01/31] x86/resctrl: Fix allocation of cleanest CLOSID
 on platforms with no monitors
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,
 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
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?MFZZL1RPOWoxamZGT3psRlZnd2NiSk5SQzUwN0NhckxmNDBLa0VIdFY0RUF6?=
 =?utf-8?B?ZGs4VWowQlZQREtQdXVEem1vUk4rcnI5VHZqMFk3RVRRQU01WDNkdFFLcGdy?=
 =?utf-8?B?NjBML050NjFaR25oZzJOcHFkRWJPeXNjZ3l4bkkwVHZsY2lTQXcycC9Ua1h4?=
 =?utf-8?B?dk16VC9FbXcyaTF5VHE5eGZUNlowYTRXYnBLT290cUFucUp2UURkUktoQkFN?=
 =?utf-8?B?SjE5K2pGazE2T29qYUpsdStqem5DUU1zS0hlMHI3WUVWNGxqaVY4aW9qK0JN?=
 =?utf-8?B?MEpKcnNrdmxNaVdUOGhZblFxRmkrQytIWW1TZGZiQXJFYVhrWDlkd05tT093?=
 =?utf-8?B?S2dwbjNDNEZxRUxta3YwbWV1aFR3c0JoTllmblZjZW4ycWg4ZmdxNEdkN2hr?=
 =?utf-8?B?RVVQUERPbUE0NXpBOEs1R3p2a1J6M0w3anpYdUdsK1pYM2hmN2JudnJxU3Iv?=
 =?utf-8?B?SjlVVlo2Zm9qNGxTU0oycW9jcXdEeTIvNGJWcVJIT3hzT1FCUTluenJzZWVT?=
 =?utf-8?B?ZnlsN1ZRN1QzdTZ0NVFtY0ZOM1NZT1BQYzR6NjdnWFpHRnArODBEQWp6eTlK?=
 =?utf-8?B?dm9yendvOVRjR3h6K3M4b01mR2RxQnpGSWM3MTdqYjhJbHlsdDVQWDNJNmxB?=
 =?utf-8?B?cFRVWFNFdll5dGNMZXJIZ09BTVh2UEZ6Z3pIbUNxTXBMWXBqaVM3Y2R3QnJJ?=
 =?utf-8?B?RGpqQXlsSUVUVmxwTmJDaHU4RnJneUxtY2YrUGM1S2MxbndreDcxR3F6S05i?=
 =?utf-8?B?WHI3NmtJTVIyU3IwbCtVTWJhK0xEN2NWZ1dpOUNDNmlLN2ZEb2Z6SW8xUUdC?=
 =?utf-8?B?Z2FnQy92azRkUjZVR1dxZ1huaEIzWVZ5VVNCWnRtaG50bVQ0UXFqOFhJaEpD?=
 =?utf-8?B?blBBRGhFaEQ1Ukc1YnVCZERBYkdRRXVNVXlWNEJtaTdyOEN0VGFGaC8wNGVU?=
 =?utf-8?B?Q3dxWHRrOWhabDZ1Q2R3Rjd4dm9VRmJQdTdPZFYrUGdHRk5VQlpTblF0c0Jw?=
 =?utf-8?B?bEVaU3RiVXN2YTdPL1VCQ056Vk1VdHNISGFiYjNKcWNraXpybXhKa0N4ZDc3?=
 =?utf-8?B?RDUzNS9aemxmNnlFR2ZCMG84VkhjWEwxOTBNODZzaWxJZEhpcHd5RHRmaTRi?=
 =?utf-8?B?aEJDSEJnNXVEQ0lKZEs4a1N1MTBvcnl2MTg2dGFQSjFBSVUvTElnbXd3MExB?=
 =?utf-8?B?ck5mSkhKd0VtdnVkTTF4Rm5tdm1kdlVwNDRLOEZOYVp4dWFzQ3BRenNDTTds?=
 =?utf-8?B?b042N0h0L0VFZE1ObVM2TEF6ckNMdGh4MVhaMCtGbWlZTU1hTFRRRGloVXJP?=
 =?utf-8?B?aXdFVUt3U1BpeWhyYnM5ZmU5VHNKSmFzYjZVcitNQ0xTM1RBSGo5Q0llYSsw?=
 =?utf-8?B?YXRxaDhOOTJVYzkwaGg0Y09FUnpmcS9LMEJ3WUlHVzBCYlFkS29HWENwdGFL?=
 =?utf-8?B?bWFBazdyU0FicmJ2Sko0UGpCVUYwUzRtVEpYQVp2bk0rczRRcXFiVTRFMXZh?=
 =?utf-8?B?VEtIMEQ4SlFXT3FielZiNFZmODQ0NEpTaStCZXhCdnJYWk9haTZYalorclNZ?=
 =?utf-8?B?eWlNVUJBLy9weHI2bWROY2Y0Zk1vZXRLcDI0eWFhdDdmcjZld1cvU3BoclhB?=
 =?utf-8?B?ajVvR3hWUDltalFhZEMyZVU4ZjhaSmJFejZEQy9rbmt3eCs5ajdSRlVWZHc5?=
 =?utf-8?B?T2h4RmdDWnYvL29GVC9zVzRNZGI2TVJkWVYrKzY0aXlWcmtCZFlDZ3N2R21U?=
 =?utf-8?B?K2wyWGd5NVhwQUN4OGtqRmpXb212d0hSRXdLK2gxNUJJL2kxQk1MZGszR3ZV?=
 =?utf-8?B?aHZRczhGVzc0SnJEZVcxQnkyMURibFVNWmpOZm9jNjdFc2E2NDloZjJHakg5?=
 =?utf-8?B?ZnNqQzBsYUhKYWh5VWRtUUh6c3gxRFpwbloyN0NsR3p1cEN1eVdYamNEb1pR?=
 =?utf-8?B?OWw0NVJGTXdRSlVTTldCN3FEVCtzNlZRVDhNS2V4aG1WQVJnSWFkRkpGSE9E?=
 =?utf-8?B?QzlxTnVIdS84bktBR3dJZlVHOGZkTTVvNkh4ZmMvU3pOUENka29QK1FGN0xQ?=
 =?utf-8?B?a0IzYmxmMThxV2JJWEVseUREQ0hHMVhSYVl6UlVMQUN5WmkyMkg2RnJCVHZu?=
 =?utf-8?Q?aL/s=3D?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 564e803b-8965-424f-b81b-08dc5d8a82d2
X-MS-Exchange-CrossTenant-AuthSource: MW3PR12MB4553.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Apr 2024 20:27:46.2943
 (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: Y9XJ8YplQOjlXAKOqAwYDJa5P0eswjT+cj8yq2e3kxj+Ht0M526iRVPs/1GwI5e+
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR12MB8859


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

This is not true. The Kconfig option is never added. It is added later in
the series. There is also comment
on this https://lore.kernel.org/lkml/ZhecyLQsGZ9Iv8wU@gmail.com/


Shouldn't the Kconfig option added first before doing this change?

> 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;

-- 
Thanks
Babu Moger