Received-SPF: pass (google.com: domain of linux-kernel+bounces-125128-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <cc629f91-b4b5-4c9d-b47c-c40eddb03a07@intel.com>
Date: Fri, 29 Mar 2024 08:25:06 -0700
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] x86/resctrl: Fix uninitialized memory read when last CPU
 of domain goes offline
To: Ingo Molnar <mingo@kernel.org>
CC: <fenghua.yu@intel.com>, <bp@alien8.de>, <james.morse@arm.com>,
	<tony.luck@intel.com>, <peternewman@google.com>, <babu.moger@amd.com>,
	<tglx@linutronix.de>, <mingo@redhat.com>, <dave.hansen@linux.intel.com>,
	<x86@kernel.org>, <hpa@zytor.com>, <james.greenhalgh@arm.com>,
	<linux-kernel@vger.kernel.org>
References: <979cfd9522021aa6001f8995cd36fb56e1c9cd39.1711659804.git.reinette.chatre@intel.com>
 <ZgZnZgfDUWlhQQxW@gmail.com>
Content-Language: en-US
From: Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <ZgZnZgfDUWlhQQxW@gmail.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?ako3KzN3TEIyYUkxSHRxdFhZZzlxUk5td2Y5bFJSNEpKQ1N0NUFUNVVjQmt1?=
 =?utf-8?B?WnorU1RTb09sTXRjM2kyOStqbHBuaVE1WGVuVzZ2WTVCemZCcDFWaU5ROU1u?=
 =?utf-8?B?bjNoZldRZzE5MGNTb2prcXBtUklLeE0vclZJSGpLVVpPdHpJSFBSNFRmRUtu?=
 =?utf-8?B?WHI0ZG5KVVFNZDBEbWFaTVhsSVM4T1p1aHk1dGF4RnM2ZnFlbnRvMXJHQUpj?=
 =?utf-8?B?MVdUS0VNelBPUGRsWGJubjIxWDJxNlIzNDNwRDN0QzRSNmlwc2VDR2VVeWFD?=
 =?utf-8?B?RFJCZEErYkVYOHhPY3dGSHVCQ0cwdS9YYVEyOXNGY1dnKzh4ZTMvTXUvMWZy?=
 =?utf-8?B?enBrelpaMEwxUERDaFpSdTZ2L2RxSldCMkhKNWFsbnpSYnFpY0M2VmVKcFFm?=
 =?utf-8?B?ZE1PczhvQUpWMEdzV05CUmwxK0pFYVloMlZvMW9kNW9zeWppUlc0RlQzYjhU?=
 =?utf-8?B?eCtwUzBGa2dENDZjdGdUMnRCSUxwNzQ1RUU2RytRSmYzbThFZll2RDVaOEdn?=
 =?utf-8?B?aE10eEZub2NNUFdYMzYrN3dsT3RWcEVzSk1QRFpTRXR0MWRHYzRqbUdrUXFD?=
 =?utf-8?B?OGxGRGdTQTdiS0RHTjdyOHB0b1MvL01jTFBnR1ZXTktHNk91OTdONWErRGcv?=
 =?utf-8?B?Ri9ERHY1V3RYWmF0bUoxZU00cDdvc3IvTFVLMHpNRzFHb3JXUndVd3llSFJy?=
 =?utf-8?B?Y3VxUUFaeWlKOFVxZ1hSMG04RFh3WjVyZUU3MndCVWIrTzdVK0YrUkp4aEdW?=
 =?utf-8?B?bDdYeVhZN2c2YndNSk9nd0ZseEZlVFVoRDdzWll6S21hdkFsOXRpNXZWTjgx?=
 =?utf-8?B?TTRlTTNLRCtlM2VVRjlOZjNuOXBYc3dqaWc2MXFJNGJLRXp5TWhYU29JM0Zr?=
 =?utf-8?B?RXRuUEJmQXFhelNJTlY2WEkzSHZ0SjNsc0ZXblEvTy9hREJ4RDNxR0F2Ympn?=
 =?utf-8?B?U0Q3L2dmVVBBUW1SUXJWSDFtM2tvcldKYzUvQWVDcHFMOEl6WE9HVVNqTTJm?=
 =?utf-8?B?S2laaDhhSWZld0NrVnhTd05XbGQrZVkxd3ZjR3FQUng2b3BqdytMWk1TMm14?=
 =?utf-8?B?YVdnVWc0Sm15MnlFN2lGV2h0MU4ydFV6MmVLb3p5ZUFmSVhXWVBzQzRQMVRu?=
 =?utf-8?B?bEIyNjUreHpVeEwybkdTWWgxUkJMa1lrR3lWenlqUmxabHc1eVN2WWd3WGpj?=
 =?utf-8?B?TlRuN0FVRmdVbExSVW4xQ1M5YjhGc0NBVy91akRXK1dOclU1TVJMcGluNkdO?=
 =?utf-8?B?NnY2TVEwb2d1T1krdXc0YU92bTZ4RE9LWFZXUjdTdG5DbXp3ZFlQRmJ6bTAr?=
 =?utf-8?B?Mk5Yb2JoaW5tNjJsSHV6WklMTnJ0dm9MQ1ZGOTFHbDhoSCt0ZWxTMjBFcGp3?=
 =?utf-8?B?ME1BcEw1SXJFR2lXanNST0tXTzR6SDI5YkxNcjZnTGJrZHRHK3U3TGxqcndr?=
 =?utf-8?B?WFhMMGJnaS9jbDk1WjREd3F6OFlocDJWNGNCSWpNVHJMaUJqLzJkbzJsZi9J?=
 =?utf-8?B?N3RhSEtlUm1IYmlHMEk5K2pGWmQvUDkzcUZEcGlidDBJVldmWFdUa2laTFAw?=
 =?utf-8?B?SStYUnR4b2JoTTJSY1B0ZVBRVk9oMDU0TGlDVUVkWlFoTHVwRHFXTTBxWkF3?=
 =?utf-8?B?ejJjRWc2U2JkSDNLaHlNRHYweFN5K3ZINXpqMTNta29yemV2TWNhNHJHM0Q0?=
 =?utf-8?B?L1lGQ1VNYVZlTDdUODJZR2ZxSlpmNGg5YmJtdWNRdHhHbENIQ0VRWndFR0I4?=
 =?utf-8?B?NmUxakhyWVBaUCt4OXpTcXVlV3NsOXltNzBCK3ZkbnN5cVlhR1NZNDdOdjlt?=
 =?utf-8?B?aVBUb29ST2Y3cHhaU2xFN1lMZVd1YUhUMGRZbWJaVjFqVVVveEdWL2F5bDFO?=
 =?utf-8?B?K1k4ZU1OUzlxc0c0TXFtN1BzQ0FqelRyanhOZitYaFZIcVFNcEpXK2d4eHA0?=
 =?utf-8?B?R1Znc09zYjhkdzNZVSt5OFpCMkFLSVpCYUVlSHhzTzF1ejZCYWxBVS80YjJH?=
 =?utf-8?B?dmtxZWpzdGhkUldFa1FGV2FETE54QmhZYS9La0tQTE5SUFNoRjBEQXpBTjRK?=
 =?utf-8?B?ViszcjFjaW9NeVBoSXJIaEovOUsrNHZHV3hDcWdIdzU1ako2UzBVb05vUXc2?=
 =?utf-8?B?Yy9yOGRvUGVvVkRPelBOY2tSS1JDYVZ0cFFKN3dHSmFwWUhUM0xPWjA1aE5H?=
 =?utf-8?B?anc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: e4309225-70d0-454d-a4d9-08dc50046b0d
X-MS-Exchange-CrossTenant-AuthSource: SJ2PR11MB7573.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 Mar 2024 15:25:08.7616
 (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: dNEJUrFhiN2+9Ke2t3hq3waIcUiFOaoF7D1zUQYEa28MbXRGPwOG0j8aYIK0EhzKc3hAD6ZXcmdtboKU57f2lJVkDEviuIEeuCjhshf+28k=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR11MB7801
X-OriginatorOrg: intel.com

Hi Ingo,

On 3/29/2024 12:01 AM, Ingo Molnar wrote:
> 
> * Reinette Chatre <reinette.chatre@intel.com> wrote:
> 
>> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
>> index c99f26ebe7a6..4f9ef35626a7 100644
>> --- a/arch/x86/kernel/cpu/resctrl/internal.h
>> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
>> @@ -85,6 +85,10 @@ cpumask_any_housekeeping(const struct cpumask *mask, int exclude_cpu)
>>  	if (cpu < nr_cpu_ids && !tick_nohz_full_cpu(cpu))
>>  		return cpu;
>>  
>> +	/* Only continue if tick_nohz_full_mask has been initialized. */
>> +	if (!tick_nohz_full_enabled())
>> +		return cpu;
>> +
> 
> So we already have this a few lines up:
> 
>         if (!IS_ENABLED(CONFIG_NO_HZ_FULL))
>                 return cpu;
> 
> And we can combine the two checks into a single one, with the patch 
> below, right?

Right. Indeed. Doing so is most appropriate. Thank you very much.

> 
> Untested.

Tested-by: Reinette Chatre <reinette.chatre@intel.com>
Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>

> 
> Thanks,
> 
> 	Ingo
> 
> ==============>
> 
>  Signed-off-by: Ingo Molnar <mingo@kernel.org>
> 
> 
>  arch/x86/kernel/cpu/resctrl/internal.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index c99f26ebe7a6..1a8687f8073a 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -78,7 +78,8 @@ cpumask_any_housekeeping(const struct cpumask *mask, int exclude_cpu)
>  	else
>  		cpu = cpumask_any_but(mask, exclude_cpu);
>  
> -	if (!IS_ENABLED(CONFIG_NO_HZ_FULL))
> +	/* Only continue if tick_nohz_full_mask has been initialized. */
> +	if (!tick_nohz_full_enabled())
>  		return cpu;
>  
>  	/* If the CPU picked isn't marked nohz_full nothing more needs doing. */

Thank you very much.

Reinette