Received-SPF: pass (google.com: domain of linux-kernel+bounces-105006-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <27076e14-66b5-4fb7-8a9a-4d5d59bbe837@intel.com>
Date: Fri, 15 Mar 2024 17:20:13 -0700
User-Agent: Mozilla Thunderbird
Subject: Re: 32bit resctrl? (was Re: [PATCH v2] fs/resctrl: fix domid loss
 precision issue)
Content-Language: en-US
To: Thomas Gleixner <tglx@linutronix.de>, Rex Nie <rex.nie@jaguarmicro.com>,
	<james.morse@arm.com>, "x86@kernel.org" <x86@kernel.org>, "Luck, Tony"
	<tony.luck@intel.com>, Babu Moger <babu.moger@amd.com>, Peter Newman
	<peternewman@google.com>, Borislav Petkov <bp@alien8.de>
CC: <fenghua.yu@intel.com>, <ilpo.jarvinen@linux.intel.com>,
	<linux-kernel@vger.kernel.org>
References: <20240312075349.977-1-rex.nie@jaguarmicro.com>
 <fed6affb-c7f4-4992-8646-8f5a52c33966@intel.com> <87jzm3jckw.ffs@tglx>
From: Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <87jzm3jckw.ffs@tglx>
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?VEJnQUpFS0haMGRSWHZJRmp6MlB6NXRsOFdDWnZxUG1WS0NYSnNsZVIrQWFU?=
 =?utf-8?B?VXJHWHdXc1h5N0EyWFN0YkU3aWNLUTAzL1VTY2diS2g3Z0I0dUdVRFNtSDYv?=
 =?utf-8?B?aTNtWFVydjhxWWc3ZmloSUJwMGZFNzdwUGF2eHJGd2wzRmlQb1RjS0UwUW1O?=
 =?utf-8?B?T1ZKTXNoY1RKTmhvcmUwU2ZWdFZHeGx4cVR5VnN5NTlqUEg1UFJlNFlMRVU4?=
 =?utf-8?B?VE1adVF2WlErWkpWajJhUlJyODlicVoyeGZMVmZBSmEvRkJCbm5scVV4T2xM?=
 =?utf-8?B?eURza1hocksweERGbnNUV0tFN2xPbVA2QU1kN3BTK096ZzdKTFdpaHJpSVpG?=
 =?utf-8?B?SDFTRmdRemZBK3A4d1Y1b1ZEMTdKUy9qVnh5SEdRZlk3c3BzUUl4TGxaNUp6?=
 =?utf-8?B?YmxMc08yQWI1N0ZIQ2JjRHF0TlE3NStOajlZUEtWY3EvY255cjFtZmRZY3h4?=
 =?utf-8?B?eVovSHVaeVYvbDNQUkUwZUxISXFpSFRCMWwvU3E0b1ZmaXlOMUFGeDM5Y3Q2?=
 =?utf-8?B?ZDI2bkRVY3J4aXNJVXFCQUp6SDROUjRaVDBIZWZPOHRNMGhtbFBDdGE1RWZs?=
 =?utf-8?B?blVZbVVJMnpISFY5cUdPOTUrcjZKK3dWaENZQjFDcXQ1MHpXTXZJSm1WSzNk?=
 =?utf-8?B?Z2pxbzdsZEFuNGEwU0hjTVFWUzBkQnZnNGxaY3NSUXNmeUdKdXZpUDFxNjVw?=
 =?utf-8?B?VC90dXNqV0djUmhKc1VKdGRZUjVqZit1NlVBQnVHVTh4d0VRa2cvYlA0SGs2?=
 =?utf-8?B?dWluMk5mVjA5R3o4bEdWTFRiRU1JN0hxWkg0YmxIdTVpNEowcXlpbGRZR1ht?=
 =?utf-8?B?VUdxNnVyTlhhdlE4SEpSYVlwLy9scGo5Z2puSmtPRXJCc0hIYXZsK1VmZG1k?=
 =?utf-8?B?T1haM3hPdW53RzhubjZDTFdYNEtwVGZWR1JIOUx6V3l3QjVMTmJjSUtEa1k5?=
 =?utf-8?B?S1ArTEp1VFZwcjZuQ1VYRmpWTGxoTWpZSWx3UnNjNUlGdENweFUwanJPUDU4?=
 =?utf-8?B?TkdOZVhZNGdaanhnSmJmUFpLVmZyeVROQWdxemE4bTM5MUJUNEZhTUUrWDkx?=
 =?utf-8?B?L2JnMWNhUWtDa1FRamRLZzZBVCtra3RVSWdqREgxVFBoYUoyREdFU1B0K2dz?=
 =?utf-8?B?UXN4ZVZtQjJhaEl3Wjk2dm04MmVOeXJZY1VpVStXSUJrNTZrZ284emhnbW9j?=
 =?utf-8?B?WnA3T0lwSURodHp2L0g3STd5Sk5GTWJSRHROdm9KSnJzSGF0aDlibzUyaGxF?=
 =?utf-8?B?cURFVTM4T3p4QTk0NlVaRHpWeXdVVVhzVEs4ZEttL3pVSHFpS2dBWmYyb1NR?=
 =?utf-8?B?RzRrOWdzQkpQRW84NVFlamdHbjlCYVZxT0I2UE5EWG8vNkJtV0QwRHBmY3FH?=
 =?utf-8?B?a09uTDJXMGhSWjNxbjhLNEdvYXMyL2NqdmRRYjFBUEJvOExPaXJCd0RQeml5?=
 =?utf-8?B?QnJTdXV1RGJNV1M0a2RVWXUzZmQrNlhrUnpLSlNtbUYzMDY5dTFqbWYwT0Yy?=
 =?utf-8?B?QzRTS21JSnV1SVYxOEljb0NwelkyMVJ2TjJZTFRhSncrcVVzQ2Y2SVhTZzNB?=
 =?utf-8?B?MUhGUFVHY3lvY1MyRStBZElEVm5RaWhZSVNvUWRNdTAvZ2I5VmFXV0dTWklO?=
 =?utf-8?B?dURNcE5nNiswN2JkTFVLSHFnYTdtSjNCU1M0a2lzZjdBTE00WlFYTFZPTDZx?=
 =?utf-8?B?Smt5MnNRZFJVemgwRml1aTFjWTJ4OWd1U0NIU3RRdW44MHZTT1pHa0p5bFlv?=
 =?utf-8?B?S2p4a0pZUmU3elNDWGVpaHpqOUFJaCtVOTN5YXpJaU5aYTViTThWWENqd00w?=
 =?utf-8?B?K0ZrNEpDSS9Yckx1U0xsc3hzOUZFcEkwTEVkZ1JDN3F5V05rOWxZVjlETVN0?=
 =?utf-8?B?SEQyOVRjRUptaTczQjRFVVhqM1ZtdSswVS9qSnlucjBPYmhCaVJPUi9mTU9n?=
 =?utf-8?B?OWhDd3RBVmRUckdGZkM0N3lWRlRjcGdjNGpTaUNMcGh5azQ4ZUs5Y09vd2Yx?=
 =?utf-8?B?OTAxOXpFa21hRFdIU0g3OTZ0Ym5tU2JPdVdVTHBNNnkvaVVuRVpIbXdrK3FM?=
 =?utf-8?B?WUppQVJpcnVWeGFYbkNRYXp4cjZDRVNWVVVyellRUkFOdmlQT2ptMlB0MUJi?=
 =?utf-8?B?NkM5VWlPUXBXeUpkTFdGandBOC9ISUlUUFo3WkhOaEliRzk3SzFqeEo1NUZH?=
 =?utf-8?B?RXc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 64e14671-285c-48b7-e0d4-08dc454edad4
X-MS-Exchange-CrossTenant-AuthSource: SJ2PR11MB7573.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Mar 2024 00:20:16.2188
 (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: 7raasj0/UaUjOPd97M5E9XeMIN1P0gqHHuzg4ueClFEKf7ofYp0iD3wLzaLunzuo+8Aef9GiRASpPPrKlwylZQUHiPKDx2SiovDtErJKNec=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB5970
X-OriginatorOrg: intel.com

Hi Thomas,

On 3/15/2024 4:32 PM, Thomas Gleixner wrote:
> On Thu, Mar 14 2024 at 08:25, Reinette Chatre wrote:
>>> On some platforms(e.g.,x86), the max cache_id is the amount of L3 caches,
>>> so it is not in the range of 0x3fff. But some platforms use higher
>>> cache_id, e.g., arm uses cache_id as locator for cache MSC. This will
>>> cause below issue if cache_id > 0x3fff likes:
>>> /sys/fs/resctrl/mon_groups/p1/mon_data/mon_L3_1048564 # cat llc_occupancy
>>> cat: read error: No such file or directory
>>>
>>> This is the call trace when cat llc_occupancy:
>>> rdtgroup_mondata_show()
>>> 	domid = md.u.domid
>>> 	d = resctrl_arch_find_domain(r, domid)
>>>
>>> d is null here because of lossing precision
>>>
>>> Signed-off-by: Rex Nie <rex.nie@jaguarmicro.com>
>>> ---
>>>  fs/resctrl/internal.h | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/fs/resctrl/internal.h b/fs/resctrl/internal.h
>>> index 7a6f46b4edd0..096317610949 100644
>>> --- a/fs/resctrl/internal.h
>>> +++ b/fs/resctrl/internal.h
>>> @@ -94,7 +94,7 @@ union mon_data_bits {
>>>  	struct {
>>>  		unsigned int rid		: 10;
>>>  		enum resctrl_event_id evtid	: 8;
>>> -		unsigned int domid		: 14;
>>> +		u32				domid;
>>>  	} u;
>>>  };
>>>  
>>
>> resctrl currently supports 32bit builds. Fixing this issue* in this way
>> would first require that resctrl (the architecture independent fs part)
>> depend on X86_64. Is this a change that everybody will be comfortable
>> with?
> 
> Why? Making mon_data_bits::u larger in the way it has been done does not
> have any dependency on 32 or 64 builds unless I'm missing something.

I should have expanded the diff. The expanded view of current code below
gives more insight into how a pointer is used to store data:

union mon_data_bits {
	void *priv;
	struct {
		unsigned int rid		: 10;
		enum resctrl_event_id evtid	: 8;
		unsigned int domid		: 14;
	} u;
}

> 
>> (Of course, there are other solutions available to address the issue mentioned
>> in this patch that do not require depending on X86_64, but I would like
>> to take this moment to understand the sentiment surrounding continuing support
>> for 32bit resctrl.)
> 
> Caring about 32biit resctrl on x86 is a pointless exercise.
> 

Thank you Thomas. This code is what will soon be moved into the architecture
agnostic "resctrl filesystem". Are there expectations from more generic
interfaces like this regarding 32-bit/64-bit? "resctrl filesystem" is on a
path to support more architectures (x86, Arm, and RISC-V) and I am not familiar
with the architectures and platforms to know what the impact of such a change
would be nor what existing usages there may be for 32-bit builds.

Reinette