lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Z3bAMmIFKBYSG0Ix@e133380.arm.com>
Date: Thu, 2 Jan 2025 16:34:58 +0000
From: Dave Martin <Dave.Martin@....com>
To: Zeng Heng <zengheng4@...wei.com>
Cc: james.morse@....com, linux-kernel@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, jonathan.cameron@...wei.com,
	xiexiuqi@...wei.com,
	"Wangshaobo (bobo)" <bobo.shaobowang@...wei.com>
Subject: Re: [RFC PATCH mpam mpam/snapshot/v6.12-rc1 v3 4/5] arm_mpam:
 Automatically synchronize the configuration of all sub-monitoring groups

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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ