Received-SPF: pass (google.com: domain of linux-kernel+bounces-140660-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Thu, 11 Apr 2024 15:38:19 +0100
From: Dave Martin <Dave.Martin@arm.com>
To: Reinette Chatre <reinette.chatre@intel.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>
Subject: Re: [PATCH v1 22/31] x86/resctrl: Make resctrl_arch_pseudo_lock_fn()
 take a plr
Message-ID: <Zhf1244gRHVueI2u@e133380.arm.com>
References: <20240321165106.31602-1-james.morse@arm.com>
 <20240321165106.31602-23-james.morse@arm.com>
 <6589ae7b-95bb-4490-83ac-95904da7649f@intel.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <6589ae7b-95bb-4490-83ac-95904da7649f@intel.com>

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(?)

Did you have other concerns here?

Cheers
---Dave