答复: [PATCH] fs/resctrl: Uniform data type of component_id/domid/id/cache_id

[Rex Nie]

HI James Morse,
	Thanks for your reply. Please check my inline reply.
BTW, can I know the progress/roadmap of mpam driver upstream?
Best regards
Rex

> -----邮件原件-----
> 发件人: James Morse <james.morse@xxxxxxx>
> 发送时间: 2024年3月12日 1:55
> 收件人: Rex Nie <rex.nie@xxxxxxxxxxxxxxx>
> 抄送: fenghua.yu@xxxxxxxxx; reinette.chatre@xxxxxxxxx;
> rohit.mathew@xxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> 主题: Re: [PATCH] fs/resctrl: Uniform data type of
> component_id/domid/id/cache_id
> 
> Hi Rex Nie,
> 
> (for those following along at home - this is a patch against the MPAM tree, not
> mainline)
> 
> On 11/03/2024 08:18, Rex Nie wrote:
> > This patch uniform data type of component_id/domid/id/cache_id to
> > u32 to avoid type confusion. According to ACPI for mpam, cache id is
> > used as locator for cache MSC. Reference to RD_PPTT_CACHE_ID
> > definition from edk2-platforms, u32 is enough for cache_id.
> >
> >
> 	(
> \
> > 	  (((PackageId) & 0xF) << 20) | (((ClusterId) & 0xFF) << 12) | \
> > 	  (((CoreId) & 0xFF) << 4) | ((CacheType) & 0xF)               \
> > 	)
> 
> Aha, this is where those numbers are coming from! Thanks for digging that
> out.
> 
> 
> > refs:
> > 1. ACPI for mpam:
> > https://developer.arm.com/documentation/den0065/latest/
> > 2. RD_PPTT_CACHE_ID from edk2-platforms:
> > https://github.com/tianocore/edk2-platforms/blob/master/Platform/ARM/S
> > giPkg/Include/SgiAcpiHeader.h#L202
> 
> Just to check - you don't see any side effects from doing this, its just cleaner.
> I agree - today this is only an int because that's what it is in struct rdt_domain.
> 
> 
> > diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h index
> > dd34523469a5..b00a89addf91 100644
> > --- a/include/linux/resctrl.h
> > +++ b/include/linux/resctrl.h
> > @@ -108,7 +108,7 @@ struct resctrl_staged_config {
> >   */
> >  struct rdt_domain {
> >  	struct list_head		list;
> > -	int				id;
> > +	u32						id;
> >  	struct cpumask			cpu_mask;
> >  	unsigned long			*rmid_busy_llc;
> >  	struct mbm_state		*mbm_total;
> 
> 
> We should probably only make this change if we clean this up in restrl, not just
> the MPAM driver.
> 
I agree. The cleanup should in top of resctrl, considering different platforms
> 
> I'll pick the MPAM bits of this up for the MPAM tree. This will eventually get
> merged with the patch that adds the original code as there is no point
> preserving the history of code that isn't merged yet. I'll add you to 'CC' of those
> patches. (The joke is 'CC' also stands for Celebrate Contribution!)
> 
Thanks!
> 
> Thanks!
> 
> James