Re: [RFC PATCH mpam mpam/snapshot/v6.12-rc1 v3 4/5] arm_mpam: Automatically synchronize the configuration of all sub-monitoring groups

[Dave Martin <Dave.Martin@arm.com>]

Hi,

On Fri, Dec 20, 2024 at 05:36:23PM +0800, Zeng Heng wrote:
> 
> 
> On 2024/12/13 0:18, Dave Martin wrote:
> > Hi,
> > 
> > > @@ -3072,9 +3080,20 @@ struct mpam_write_config_arg {
> > >   static int __write_config(void *arg)
> > >   {
> > > +	int closid_num = resctrl_arch_get_num_closid(NULL);
> > >   	struct mpam_write_config_arg *c = arg;
> > > +	u32 reqpartid, req_idx;
> > > +
> > > +	WARN_ON(c->partid >= closid_num);
> > > -	mpam_reprogram_ris_partid(c->ris, c->partid, &c->comp->cfg[c->partid]);
> > > +	/* Synchronize the configuration to each sub-monitoring group. */
> > > +	for (req_idx = 0; req_idx < get_num_reqpartid_per_closid();
> > > +	     req_idx++) {
> > > +		reqpartid = req_idx * closid_num + c->partid;
> > > +
> > > +		mpam_reprogram_ris_partid(c->ris, reqpartid,
> > > +					 &c->comp->cfg[c->partid]);
> > > +	}
> > >   	return 0;
> > >   }
> > 
> > I haven't decided whether this iteration belongs here or in
> > mpam_resctrl.c.
> > 
> > Your approach looks like it should work; I do it in
> > resctrl_arch_update_one() instead [1], but I think the approaches are
> > pretty much equivalent -- but let me know if you have any thoughts on
> > it.
> > 
> 
> Yes, the actual functions of these two locations are essentially the
> same. However, at the __write_config position, we can reduce the
> repeated judgments of cfg[partid] in mpam_update_config() and also
> decrease the times of smp_call remote invocations.
> 
> What about your option towards it?

I think either can be done.  I was aiming to keep things as simple as
possible for now, and contain all the mapping logic in mpam_resctrl.c.

I think that with my version of the code, changing the
mpam_apply_config() interface to accept a PARTID range instead of a
single PARTID might be a natural way to do this.

This probably does make sense, in order to avoid excessive SMP cross-
calling; I will have a go and see whether this works.


(Note, there is likely to be redundant cross-calling already, which is
one reason why I did not pay close attention to this issue.  If we
could batch updates separately per group of CPUs then we could reduce
the number of cross-calls, though care would be needed if CPUs can be
hotplugged while processing a batch of updates.  I think that a change
to the resctrl core interface might be necessary if we want the arch
code to be able to queue and schedule updates in this way; resctrl
currently assumes that each update is applied immediately.)

Cheers
---Dave