[2/7] x86/resctrl: Remove hard code of RDT_RESOURCE_L3 in monitor.c
Commit Message
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@intel.com>
---
arch/x86/kernel/cpu/resctrl/monitor.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
Comments
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@intel.com>
> ---
> 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
@@ -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.
*/
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)