Re: [PATCH v1 3/6] x86/resctrl: Disallow mongroup rename on MPAM

[Peter Newman]

Hi Reinette,

On Sat, Apr 6, 2024 at 12:10 AM Peter Newman <peternewman@xxxxxxxxxx> wrote:
>
> Hi Reinette,
>
> On Thu, Apr 4, 2024 at 4:11 PM Reinette Chatre
> <reinette.chatre@xxxxxxxxx> wrote:
> >
> > Hi Peter,
> >
> > On 3/25/2024 10:27 AM, Peter Newman wrote:
> > > Moving a monitoring group to a different parent control assumes that the
> > > monitors will not be impacted. This is not the case on MPAM where the
> > > PMG is an extension of the PARTID.
> > >
> > > Detect this situation by requiring the change in CLOSID not to affect
> > > the result of resctrl_arch_rmid_idx_encode(), otherwise return
> > > -EOPNOTSUPP.
> > >
> >
> > Thanks for catching this. This seems out of place in this series. It sounds
> > more like an independent fix that should go in separately.
>
> I asserted in a comment in a patch later in the series that the
> mongroup parent pointer never changes on MPAM, then decided to follow
> up on whether it was actually true, so it's only here because this
> series depends on it. I'll post it again separately with the fix you
> requested below.

I'm preparing to finally re-post this patch, but another related
question came up...

It turns out the check added to mongroup rename in this patch is an
important property that the user needs in order to correctly interpret
the value returned by info/L3_MON/num_rmids.

I had told some Google users to attempt to move a monitoring group to
a new parent to determine whether the monitoring groups are
independent of their parent ctrl_mon groups. This approach proved
unwieldy when used on a system where the maximum number of allowed
groups has already been created. (In their problem case, a
newly-created process wanted to determine this property independently
of the older process which had originally mounted resctrl.)

Also, this information does not require an active rdtgroup pointer or
CLOSID/RMID. The following will also work:

 resctrl_arch_rmid_idx_encode(0, 0) != resctrl_arch_rmid_idx_encode(1, 0);

I propose adding a new info file returning the result of this
expression to be placed beside num_rmids. I would name it
"mon_id_includes_control_id", as it implies that the hardware
logically interprets the monitoring ID as concatenated with its parent
control ID. This tells the user whether num_rmids defines the number
of monitoring groups (and ctrl_mon groups) which can be created system
wide or whether it's one more than the number of monitoring groups
which can be created below every ctrl_mon group.

Without this information, I find it difficult to create test cases
with a maximal number of groups in a portable fashion.

Also, this would be a good opportunity to generalize "num_rmids" to
"num_mon_ids" and add a symlink between the two for backwards
compatibility.

Thoughts?

Thanks,
-Peter