RE: [PATCH v4 0/7] x86/resctrl: Miscellaneous resctrl features

Moger, Babu
2023-05-05T15:43:38Z

[AMD Official Use Only - General]

Hi Reinette,

> -----Original Message-----
> From: Reinette Chatre <reinette.chatre@intel.com>
> Sent: Thursday, May 4, 2023 1:54 PM
> To: Moger, Babu <Babu.Moger@amd.com>; corbet@lwn.net;
> tglx@linutronix.de; mingo@redhat.com; bp@alien8.de
> Cc: fenghua.yu@intel.com; dave.hansen@linux.intel.com; x86@kernel.org;
> hpa@zytor.com; paulmck@kernel.org; akpm@linux-foundation.org;
> quic_neeraju@quicinc.com; rdunlap@infradead.org;
> damien.lemoal@opensource.wdc.com; songmuchun@bytedance.com;
> peterz@infradead.org; jpoimboe@kernel.org; pbonzini@redhat.com;
> chang.seok.bae@intel.com; pawan.kumar.gupta@linux.intel.com;
> jmattson@google.com; daniel.sneddon@linux.intel.com; Das1, Sandipan
> <Sandipan.Das@amd.com>; tony.luck@intel.com; james.morse@arm.com;
> linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org;
> bagasdotme@gmail.com; eranian@google.com; christophe.leroy@csgroup.eu;
> jarkko@kernel.org; adrian.hunter@intel.com; quic_jiles@quicinc.com;
> peternewman@google.com
> Subject: Re: [PATCH v4 0/7] x86/resctrl: Miscellaneous resctrl features
>
> Hi Babu,
>
> On 4/17/2023 4:33 PM, Babu Moger wrote:
> > These series adds support few minor features.
> > 1. Support assigning multiple tasks to control/mon groups in one command.
> > 2. Add debug mount option for resctrl interface.
> > 3. Add RMID and CLOSID in resctrl interface when mounted with debug
> option.
> > 4. While doing these above changes, found that rftype flags needed some
> cleanup.
> > They were named inconsistently. Re-arranged them much more cleanly
> now.
> > Hope it can help future additions.
> >
> > ---
> > v4: Changes since v3
> > Addressed comments from Reinette and others.
> > Removed newline requirement when adding tasks.
> > Dropped one of the changes on flags. Kept the flag names mostly same.
> > Changed the names of closid and rmid to ctrl_hw_id and mon_hw_id
> respectively.
> > James had some concerns about adding these files. But I thing it is big
> problem.
> > Please comment back if we can do better.
>
> From what I understand the primary concern was the naming of the files, which
> you address in this version.
>
> A second point I saw was a request for insight into why user space may need
> this (James recommended obfuscation when value is only shared between
> kernel interfaces).
> You did answer this in your response and since there was no follow-up I
> currently assume that this has been answered.
>
> Unless we hear otherwise from James I thus believe that his feedback is
> addressed.

Ok. Sounds good.

>
> > Tried to address Reinette's comment on patch 7. But due to current code
> design
> > I could not do it exact way. But changed it little bit to make it easy debug
> > file additions in the future.
>
> Could you please elaborate? It actually looks like you may be headed there next
> according to:
> https://lore.kernel.org/lkml/933d8ae2-d8b7-7436-5918-
> f639405c9ecb@amd.com/

Sorry, I was talking about this comment.
https://lore.kernel.org/lkml/fef12c9e-7d6f-bcd4-f92e-e776eb9e673b@intel.com/

I tried to address it here.
https://lore.kernel.org/lkml/168177451010.1758847.568218491528297451.stgit@bmoger-ubuntu/

This may not be the exact way you mentioned. Reason is, adding the flags before rdtgroup_add_files cannot be done. It does not update the resctrl root filesystem.
These files have to added by calling rdtgroup_add_file and kernfs_activate in rdt_enable_ctx.
Thanks
Babu