RE: [RFC PATCH 2/2] resctrl2: Arch x86 modules for most of the legacy control/monitor functions

From: Luck, Tony
Date: Wed Jul 05 2023 - 00:46:17 EST


>> + if (rr->event <= EV_LOC) {
>> + spin_lock_irqsave(&m->msr_lock, flags);
>
> Will there ultimately be any locking at the filesystem layer? I recall
> from feedback on my change adding a spinlock here[1] before that the
> filesystem-layer locking took care of this.

Yes. Working on that now. The spinlock will go away when everything is protected.
by resctrl_mutex.

> > + if (event <= EV_LOC)
> > + smp_call_function_any(&d->cpu_mask, __rdt_rmid_read, &rr, 1);
> > + else
> > + __rdt_rmid_read(&rr);
>
> I like that the driver is responsible for deciding where IPIs need to
> be sent, but it looks like the consequence is that RDT-level code
> wants to add in the child monitors' event counts once executing within
> the correct domain. The one-per-domain IPI assumption from the current
> resctrl code being wrong is probably harder to overcome than needing
> to figure out what additional RMIDs to read, but I'd really need to
> know the synchronization requirements for __rdt_rmid_read() to inspect
> the monitoring group hierarchy.

There are likely some gaps in current code. But I think they should be
fixable.

> Would you continue to promise that the FS structure won't change
> during a monitor read? To us, the biggest priority for
> parallelization is reading all the domain-group combinations in the
> system, because we have a lot of them and want the tightest possible
> snapshot of bandwidth usage, broken down by group.

The mbm_poll() code that makes sure that counters don't wrap is
doing all the expensive wrmsr(QM_EVTSEL);rdmsr(QM_COUNT)
once per second to give you the data you want. But existing resctrl
filesystem doesn't let you do a bulk read. I have some ideas on how
to provide something better. One question: do you really need that
snapshot to be system-wide? Or can you live with separate L3-scoped
snapshots that aren't tightly synchronized with each other?

-Tony