[2/7] x86/resctrl: Remove hard code of RDT_RESOURCE_L3 in monitor.c

Message ID 20230126184157.27626-3-tony.luck@intel.com
State New
Headers
Series x86/resctrl: Add support for Sub-NUMA cluster (SNC) systems |

Commit Message

Luck, Tony Jan. 26, 2023, 6:41 p.m. UTC
  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

Fenghua Yu Jan. 27, 2023, 4:51 a.m. UTC | #1
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
  

Patch

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.
  */
 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)