Received-SPF: pass (google.com: domain of linux-kernel+bounces-141418-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: <095c100c-b5dc-45ad-a5d2-86bedfcc1bfd@intel.com>
Date: Thu, 11 Apr 2024 10:40:37 -0700
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v1 22/31] x86/resctrl: Make resctrl_arch_pseudo_lock_fn()
 take a plr
To: Dave Martin <Dave.Martin@arm.com>
CC: James Morse <james.morse@arm.com>, <x86@kernel.org>,
	<linux-kernel@vger.kernel.org>, 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>
References: <20240321165106.31602-1-james.morse@arm.com>
 <20240321165106.31602-23-james.morse@arm.com>
 <6589ae7b-95bb-4490-83ac-95904da7649f@intel.com>
 <Zhf1244gRHVueI2u@e133380.arm.com>
Content-Language: en-US
From: Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <Zhf1244gRHVueI2u@e133380.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?dXV2Qnd1R29jaFlPSHhHdzRqVEJkSmNaOTFqOUZlaVR5bmxRNkh1OTFGMVlM?=
 =?utf-8?B?WmdzVjVUU1AxNEJ5NGJpdzJwQUZIbW9aTFZkamI5UWhlRTFtdHZQMUhwRnhP?=
 =?utf-8?B?T0hvbTRaYngvT3NTSjQ2OFJQelB4a2t1Mnl5ZmY2SFZieVlLS3NGYkJZNTVS?=
 =?utf-8?B?WlhkZHlKRnBBbmVzRWZHaURZOU4vd1dLT2ZLRVhSeGVBYVZKUXBqM3R6UU9j?=
 =?utf-8?B?N1BNOHYrUk9jVVM1VjRPRWFzbStvQ3dnNjJZMy81OGM2bDZLR1ZXVXVBc21y?=
 =?utf-8?B?OTNzU1gwbWJSWWlHUGVUSThpNnFXdlN0anZ5N28rRFFkZzY2aU5jbCtyemFS?=
 =?utf-8?B?dFQ1ckNwcjNaK2dKRlBrY1FmL2FZd1hCVVgzRWJWdWRoK3Q0WDlDSlJPWWt2?=
 =?utf-8?B?SGZxdzlDZm1VVHU1bnU3bWVFMC93TkdSM3hYZ3J3OUhKZzA4RVE5a25hbXNI?=
 =?utf-8?B?RzJDM0ZVUnNEOWMwZ2F5UlgxTitId0tkalZQRHY5dzYrOWxSMkR0cUNXem1q?=
 =?utf-8?B?SFN5L2hsdlFFczVORndnYkVWQ2J2TEl6TUVGWWRsTFBBMnVYVTRRbUo2T2dW?=
 =?utf-8?B?UkkrNUwzM1c2ZUNQdVhGaURybm41V1JtRC92UlZHNDhPTFJocnV0dmg2VXVK?=
 =?utf-8?B?ODlrUXViN29WNk9NUnExcTdrbTBLTnhza2FHNGl6Y2NjUE85eGh6dWF5emNk?=
 =?utf-8?B?TWhQMUhFYzdDWUgxSCtvckt4T09hd2pBQ2hnZlJ4RG1ESlZ3UThWV2RmVnhn?=
 =?utf-8?B?eGxHOWp6L2dWTytSSHU2MkY2TmJVVnRiYTVoTzQ3NEhFQlhYS3QwdlpuRmhP?=
 =?utf-8?B?WnBkK0t4UG82eXo4dGVITnNoNGtPaTB6aDdpcmExOWdjK0NUd3hOd0tYV2Zu?=
 =?utf-8?B?ZW9Xc1NicHFkMHBOSEVDOWJEY3F5Y01ERGVycHRoU1BBY1Rpd1ZUeXVQRktX?=
 =?utf-8?B?QnlNc0RnVzVzSTAxVGw0ZXJJTVBkTElXOHdtZGdpUklBMlhWZU4zRGdNSmdJ?=
 =?utf-8?B?b3ZWTEg1UCsxdFNYa1VENk9kYmNMM000WTd5Q1RxWGFDRHI0ZjNhWFM4cFpM?=
 =?utf-8?B?ay9lSkR6NkRsOEQ1Visxb2lmTTFkNlpDdkloSXZyVUhuNENxU3N1aTRYZ3I4?=
 =?utf-8?B?aFpOY2w1SjZQQUNkYktINDY2eFJpeURVbVBIZEpOdHgzaG4zQkJzQzRFWHFB?=
 =?utf-8?B?SmcwWEw0WGpxUnhWOXlEZGp3WVB3QjkvWVA2YTMrSUtVRHM5NThxY0NCTzdj?=
 =?utf-8?B?UGxnWlc5cEZxbWZyNG14TjM3NkV6Y0JmYmxoOG1FcEtnYnBoTkFLOG5RL3kz?=
 =?utf-8?B?dnNZbnZFTkF3SmZjbEw4NlVRRUtxSlptQWV6L25yQVlyN3paUlNFWVk0Z2tC?=
 =?utf-8?B?dVA4NWV3S1lsSmFlbE9vY3hKRytMLzJrQmdYZVpZR0JDbll5bFhuUzBPZmxv?=
 =?utf-8?B?emx3TXRMYjFMYXhYMEFqUTFvTS91K0NqT2Vhc2lpZVVENkRtbEV6Sm9xemFn?=
 =?utf-8?B?d0hSckJEaldJU1R1T0N5eU0vRTZocEUyN1NxT3B1OFpDcG1yejY3dFh3dDFk?=
 =?utf-8?B?Qjh0dDltUFU0ZDNGdnhyQVJVZGNocW9FQ1RYZE04anMwNlQ2TTN4dDRYaWkr?=
 =?utf-8?B?SE41bEcxSzJWQmZ2S01FWU9PRE9Na3dPeFdDeUpPNEZsdDRqOGZlR29MSDhl?=
 =?utf-8?B?VGFQamRMbThCL0xjR3hmd0pqQzJoTHhrOUFaTVlBL2FQa2s5Rk51aUw2VEQw?=
 =?utf-8?B?R25zbis0OS9EYjRHdDlSNHFOclhXRm1JZm1JZHBJNVNKNDVtNGpiZzVTUDYz?=
 =?utf-8?B?czdVVVdlSDZiMTY1a0xzUHBlLzRsUk5kdXd5Y0QxaTY4OEZ0dnpVUHYxQUVL?=
 =?utf-8?B?NWx6d1JTT2gvd00wWlZteVpVem9zMWM4dVZhUG1NNlVFaFJER2dUV0JhYUkw?=
 =?utf-8?B?aDh5M2xCRGV6cmpSeFBpcnVPcDhlaWwvOXA2WmFOQkhHYzlNTlZXbTJjMllZ?=
 =?utf-8?B?akk3dUcyUjF4QmU3Y0RCNWt2ZDNheFhLN0JHV0hGcVdpaGFhYUNxV0hYLzc0?=
 =?utf-8?B?cC9ucGYxZ0dZMEFKTkhmSDQ5VXBTeW0rRHpsZ29JdDZ5VGNkbUJGYWZaeFRz?=
 =?utf-8?B?enY4a2RYRWwrS1pTVlZoMGFyUDB5TmlFUFJwVnQxMWVIZzRJRWluaU9ZcG9X?=
 =?utf-8?B?S0E9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: f9ea4e15-26c3-49d8-5e70-08dc5a4e8118
X-MS-Exchange-CrossTenant-AuthSource: SJ2PR11MB7573.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Apr 2024 17:40:40.0754
 (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: xKnELePbnRHnolJOvC3TIqTUwzqYaY8pet2M+TkBnvFwyAeWy7oXrKM2ODW0A4XyP5wDxHzL+0tfGf6kQ6kvmRK4Clbr0ExqK8Z2Pb0ugGo=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR11MB7823
X-OriginatorOrg: intel.com

Hi Dave,

On 4/11/2024 7:38 AM, Dave Martin wrote:
> On Mon, Apr 08, 2024 at 08:24:35PM -0700, Reinette Chatre wrote:
>> Hi James,
>>
>> On 3/21/2024 9:50 AM, James Morse wrote:
>>> resctrl_arch_pseudo_lock_fn() has architecture specific behaviour,
>>> and takes a struct rdtgroup as an argument.
>>>
>>> After the filesystem code moves to /fs/, the definition of struct
>>> rdtgroup will not be available to the architecture code.
>>>
>>> The only reason resctrl_arch_pseudo_lock_fn() wants the rdtgroup is
>>> for the CLOSID. Embed that in the pseudo_lock_region as a hw_closid,
>>
>> Above creates expectation that the new member will be named hw_closid,
>> but that is not what the code does.
> 
> I'll flag this for review, but I'd guess that this can probably just be
> "closid".  I'll make a note to consider what needs to change to make
> things consistent between the patch and commit message.
> 
> James might have had other ideas, connected with the remapping done for
> CDP emulation causing the resctrl closid being different from the actual
> value used by the hardware, at least for MPAM (see my response on
> patch 24).  I don't fully understand how this works for x86 though.
> 
> So long as functionality is unaffected, and this patch is introducing no
> new confusion that wasn't there beforehand, the exact name may not
> matter too much(?)

closid sounds good. It may be a good match for what is expected to be in
general/fs code. 

> 
> Did you have other concerns here?

No.

Reinette