Re: [PATCH v4 1/7] x86/resctrl: Create separate domains for control and monitoring

From: Tony Luck
Date: Mon Aug 28 2023 - 20:32:05 EST


Trimming to the core of the discussion.

> > >> if (r->alloc_capable && domain_setup_ctrlval(r, d)) {
> > >> domain_free(hw_dom);
> > >> return;
> > >> }
> > >>
> > >> if (r->mon_capable && arch_domain_mbm_alloc(r->num_rmid, hw_dom)) {
> > >> domain_free(hw_dom);
> > >> return;
> > >> }

In the current code the control and monitor functions share a domain
structure. So enabling one without the other would lead to a whole
slew of complications. Various other bits of code would need to change
to make run-time checks that the control/monitor pieces of the shared
domain structure had been completely set up. So the existing code takes
the easy path out and says "if I can't have both, I'll take have
neither".

> > >> The above is the other item that I've been trying to discuss
> > >> with you. Note that existing resctrl will not initialize monitoring if
> > >> control could not be initialized.
> > >> Compare with this submission:
> > >>
> > >> if (r->alloc_capable)
> > >> domain_add_cpu_ctrl(cpu, r);
> > >> if (r->mon_capable)
> > >> domain_add_cpu_mon(cpu, r);

With the updates I'm making, there are separate domain structures for
control and monitor. They don't have to be tied together. It's possible
for initialization to fail on one, but the other remain completely
functional (without adding "is this completely setup" checks to
other code).

The question is what should the code do? Should it allow a partial
success (now that is possible)? Or should it maintain legacy behavior
and block use of both control and monitor for domain if either fails
to allocate necessary memory to operate?

I'm generally in favor of legacy semantics. But it seems weird to make
the code deliberately worse to preserve this particular behavior.
Especially as it adds complexity to the code for a case that in practice
is never going to happen.

-Tony