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

[Moger, Babu]

[AMD Official Use Only - General]

Hi Reinette,

> -----Original Message-----
> From: Reinette Chatre <reinette.chatre@xxxxxxxxx>
> Sent: Thursday, May 4, 2023 1:54 PM
> To: Moger, Babu <Babu.Moger@xxxxxxx>; corbet@xxxxxxx;
> tglx@xxxxxxxxxxxxx; mingo@xxxxxxxxxx; bp@xxxxxxxxx
> Cc: fenghua.yu@xxxxxxxxx; dave.hansen@xxxxxxxxxxxxxxx; x86@xxxxxxxxxx;
> hpa@xxxxxxxxx; paulmck@xxxxxxxxxx; akpm@xxxxxxxxxxxxxxxxxxxx;
> quic_neeraju@xxxxxxxxxxx; rdunlap@xxxxxxxxxxxxx;
> damien.lemoal@xxxxxxxxxxxxxxxxxx; songmuchun@xxxxxxxxxxxxx;
> peterz@xxxxxxxxxxxxx; jpoimboe@xxxxxxxxxx; pbonzini@xxxxxxxxxx;
> chang.seok.bae@xxxxxxxxx; pawan.kumar.gupta@xxxxxxxxxxxxxxx;
> jmattson@xxxxxxxxxx; daniel.sneddon@xxxxxxxxxxxxxxx; Das1, Sandipan
> <Sandipan.Das@xxxxxxx>; tony.luck@xxxxxxxxx; james.morse@xxxxxxx;
> linux-doc@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> bagasdotme@xxxxxxxxx; eranian@xxxxxxxxxx; christophe.leroy@xxxxxxxxxx;
> jarkko@xxxxxxxxxx; adrian.hunter@xxxxxxxxx; quic_jiles@xxxxxxxxxxx;
> peternewman@xxxxxxxxxx
> 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@xxxxxxx/

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

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