Hi, Tony,
On 1/26/23 10:41, Tony Luck wrote:
> Scope of monitoring may be scoped at L3 cache granularity (legacy) or
> at the node level (systems with Sub NUMA Cluster enabled).
>
> Save the struct rdt_resource pointer that was used to initialize
> the monitor sections of code and use that value instead of the
> hard-coded RDT_RESOURCE_L3.
>
> No functional change.
>
> Signed-off-by: Tony Luck <tony.luck@xxxxxxxxx>
> ---
> arch/x86/kernel/cpu/resctrl/monitor.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
> index 77538abeb72a..d05bbd4f6b2d 100644
> --- a/arch/x86/kernel/cpu/resctrl/monitor.c
> +++ b/arch/x86/kernel/cpu/resctrl/monitor.c
> @@ -30,6 +30,8 @@ struct rmid_entry {
> struct list_head list;
> };
>
> +static struct rdt_resource *mon_resource;
> +
> /**
> * @rmid_free_lru A least recently used list of free RMIDs
> * These RMIDs are guaranteed to have an occupancy less than the
> @@ -251,7 +253,7 @@ int resctrl_arch_rmid_read(struct rdt_resource *r, struct rdt_domain *d,
> */
> void __check_limbo(struct rdt_domain *d, bool force_free)
> {
> - struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_L3].r_resctrl;
> + struct rdt_resource *r = mon_resource;
> struct rmid_entry *entry;
> u32 crmid = 1, nrmid;
> bool rmid_dirty;
> @@ -316,7 +318,7 @@ int alloc_rmid(void)
>
> static void add_rmid_to_limbo(struct rmid_entry *entry)
> {
> - struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_L3].r_resctrl;
> + struct rdt_resource *r = mon_resource;
> struct rdt_domain *d;
> int cpu, err;
> u64 val = 0;
> @@ -633,7 +635,7 @@ void cqm_handle_limbo(struct work_struct *work)
>
> mutex_lock(&rdtgroup_mutex);
>
> - r = &rdt_resources_all[RDT_RESOURCE_L3].r_resctrl;
> + r = mon_resource;
> d = container_of(work, struct rdt_domain, cqm_limbo.work);
>
> __check_limbo(d, false);
> @@ -669,7 +671,7 @@ void mbm_handle_overflow(struct work_struct *work)
> if (!static_branch_likely(&rdt_mon_enable_key))
> goto out_unlock;
>
> - r = &rdt_resources_all[RDT_RESOURCE_L3].r_resctrl;
> + r = mon_resource;
> d = container_of(work, struct rdt_domain, mbm_over.work);
>
> list_for_each_entry(prgrp, &rdt_all_groups, rdtgroup_list) {
> @@ -747,9 +749,11 @@ static struct mon_evt mbm_local_event = {
> /*
> * Initialize the event list for the resource.
> *
> - * Note that MBM events are also part of RDT_RESOURCE_L3 resource
> + * Note that MBM events can either be part of RDT_RESOURCE_L3 resource
> * because as per the SDM the total and local memory bandwidth
> - * are enumerated as part of L3 monitoring.
> + * are enumerated as part of L3 monitoring, or they may be per NUMA
> + * node on systems with sub-NUMA cluster enabled and are then in the
> + * RDT_RESOURCE_NODE resource.
"RDT_RESOURCE_NODE" is not defined yet. It will be defined in patch #3.
Maybe better to move this comment change to patch #3 to avoid confusion
on RDT_RESOURCE_NODE.
Further, the current comment calls out MBM because MBM is not obviously
related to L3. But with SNC, I think we need to call out SNC node for
all monitor events, not just MBM.
Maybe something like this?
/*
* Initialize the event list for the resource.
*
* Monitor events can either be part of RDT_RESOURCE_L3 resource,
* or they may be per NUMA node on systems with sub-NUMA cluster
* enabled and are then in the RDT_RESOURCE_NODE resource.
*
* Note that MBM events are also part of RDT_RESOURCE_L3 or
* RDT_RESOURCE_NODE resource
* because as per the SDM the total and local memory bandwidth
* are enumerated as part of L3 monitoring.
*/
> */
> static void l3_mon_evt_init(struct rdt_resource *r)
> {
> @@ -761,6 +765,8 @@ static void l3_mon_evt_init(struct rdt_resource *r)
> list_add_tail(&mbm_total_event.list, &r->evt_list);
> if (is_mbm_local_enabled())
> list_add_tail(&mbm_local_event.list, &r->evt_list);
> +
> + mon_resource = r;
> }
>
> int rdt_get_mon_l3_config(struct rdt_resource *r)
Thanks.
-Fenghua
