Messages in this thread | | | Subject | Re: [PATCH] x86/resctrl: Preserve CDP enable over cpuhp | From | James Morse <> | Date | Fri, 14 Feb 2020 18:12:43 +0000 |
| |
Hi Reinette,
On 13/02/2020 19:45, Reinette Chatre wrote: > On 2/13/2020 9:42 AM, James Morse wrote: >> On 12/02/2020 22:53, Reinette Chatre wrote: >>> On 2/12/2020 10:53 AM, James Morse wrote: >>>> mounted, and that cpus remember their CDP-enabled state over cpu >>>> hotplug. >>>> >>>> This goes wrong when resctrl's CDP-enabled state changes while all >>>> the cpus in a domain are offline. >>>> >>>> When a domain comes online, enable (or disable!) CDP to match resctrl's >>>> current setting.
>> ... I think you're describing adding:
[...]
>> to rdtgroup.c and using that from core.c? > > If I understand this correctly the CDP configuration will be done twice > for each CDP resource, and four times for each CDP resource on a system > supporting both L2 and L3 CDP. I think it is possible to do > configuration once for each. Also take care on systems that support MBA > that would not be caught by the first if statement. A system supporting > MBA and CDP may thus attempt the configuration even more. It should be > possible to use the resource parameter for a positive test and then just > let the other resources fall through? Considering this, what do you > think of something like below? > > void rdt_domain_reconfigure_cdp(struct rdt_resource *r) > { > if (!r->alloc_capable) > return; > > if (r == &rdt_resources_all[RDT_RESOURCE_L2DATA]) > l2_qos_cfg_update(&r->alloc_enabled); > > if (r == &rdt_resources_all[RDT_RESOURCE_L3DATA]) > l3_qos_cfg_update(&r->alloc_enabled); > }
Sold!
(the !r->alloc_capable are already filtered out by the caller, but checking is the least-surprise option)
I'll send a v2 shortly with your suggested-by. I'd like to keep the lockdep annotations as the MPAM tree tries to stop the arch code taking the rdtgroup_mutex. Those patches changing these annotations makes it nice and clear what is going on.
Thanks,
James
| |