diff mbox series

[v15-RFC,5/8] x86/resctrl: Add "NODE" as an option for resource scope

Message ID 20240130222034.37181-6-tony.luck@intel.com
State New
Headers
Series Add support for Sub-NUMA cluster (SNC) systems |

Commit Message

Luck, Tony Jan. 30, 2024, 10:20 p.m. UTC 
  Add RESCTRL_NODE to the enum, and to the helper function that looks
up a domain id from a scope.

There are a couple of places where the scope must be a cache scope.
Add some defensive WARN_ON checks to those.

Signed-off-by: Tony Luck <tony.luck@intel.com>
---
 include/linux/resctrl.h                   | 1 +
 arch/x86/kernel/cpu/resctrl/core.c        | 3 +++
 arch/x86/kernel/cpu/resctrl/pseudo_lock.c | 4 ++++
 arch/x86/kernel/cpu/resctrl/rdtgroup.c    | 3 +++
 4 files changed, 11 insertions(+)

Comments

Moger, Babu Feb. 9, 2024, 3:29 p.m. UTC | #1 
Hi Tony,

This patch probably needs to be merged with with patch 7.

Thanks

On 1/30/24 16:20, Tony Luck wrote:
> Add RESCTRL_NODE to the enum, and to the helper function that looks
> up a domain id from a scope.
> 
> There are a couple of places where the scope must be a cache scope.
> Add some defensive WARN_ON checks to those.
> 
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---
>  include/linux/resctrl.h                   | 1 +
>  arch/x86/kernel/cpu/resctrl/core.c        | 3 +++
>  arch/x86/kernel/cpu/resctrl/pseudo_lock.c | 4 ++++
>  arch/x86/kernel/cpu/resctrl/rdtgroup.c    | 3 +++
>  4 files changed, 11 insertions(+)
> 
> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
> index 2155dc15e636..e3cddf3f07f8 100644
> --- a/include/linux/resctrl.h
> +++ b/include/linux/resctrl.h
> @@ -147,6 +147,7 @@ struct resctrl_schema;
>  enum resctrl_scope {
>  	RESCTRL_L2_CACHE = 2,
>  	RESCTRL_L3_CACHE = 3,
> +	RESCTRL_NODE,
>  };
>  
>  /**
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> index 59e6aa7abef5..b741cbf61843 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -505,6 +505,9 @@ static int get_domain_id_from_scope(int cpu, enum resctrl_scope scope)
>  	case RESCTRL_L2_CACHE:
>  	case RESCTRL_L3_CACHE:
>  		return get_cpu_cacheinfo_id(cpu, scope);
> +	case RESCTRL_NODE:
> +		return cpu_to_node(cpu);
> +
>  	default:
>  		break;
>  	}
> diff --git a/arch/x86/kernel/cpu/resctrl/pseudo_lock.c b/arch/x86/kernel/cpu/resctrl/pseudo_lock.c
> index 6a72fb627aa5..2bafc73b51e2 100644
> --- a/arch/x86/kernel/cpu/resctrl/pseudo_lock.c
> +++ b/arch/x86/kernel/cpu/resctrl/pseudo_lock.c
> @@ -292,10 +292,14 @@ static void pseudo_lock_region_clear(struct pseudo_lock_region *plr)
>   */
>  static int pseudo_lock_region_init(struct pseudo_lock_region *plr)
>  {
> +	enum resctrl_scope scope = plr->s->res->scope;
>  	struct cpu_cacheinfo *ci;
>  	int ret;
>  	int i;
>  
> +	if (WARN_ON_ONCE(scope != RESCTRL_L2_CACHE && scope != RESCTRL_L3_CACHE))
> +		return -ENODEV;
> +
>  	/* Pick the first cpu we find that is associated with the cache. */
>  	plr->cpu = cpumask_first(&plr->d->cpu_mask);
>  
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index eff9d87547c9..770f2bf98462 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -1413,6 +1413,9 @@ unsigned int rdtgroup_cbm_to_size(struct rdt_resource *r,
>  	unsigned int size = 0;
>  	int num_b, i;
>  
> +	if (WARN_ON_ONCE(r->scope != RESCTRL_L2_CACHE && r->scope != RESCTRL_L3_CACHE))
> +		return size;
> +
>  	num_b = bitmap_weight(&cbm, r->cache.cbm_len);
>  	ci = get_cpu_cacheinfo(cpumask_any(&d->cpu_mask));
>  	for (i = 0; i < ci->num_leaves; i++) {
Luck, Tony Feb. 9, 2024, 7:23 p.m. UTC | #2 
On Fri, Feb 09, 2024 at 09:29:03AM -0600, Moger, Babu wrote:
> Hi Tony,
> 
> This patch probably needs to be merged with with patch 7.

If it just added RESCTRL_NODE to the enum and the switch() I'd
agree (as patch 7 is where RESCTRL_NODE first used). But this
patch also adds the sanity checks on scope where it has to be
a cache level.

Patch 7 is already on the big side (119 lines added to core.c).

If you really think this series needs to cut back the
number of patches, I could move the sanity check pieces
from here to patch 3 (where the enum is introduced) and
just the RESCTRL_NODE bits to patch 7.

-Tony
Moger, Babu Feb. 12, 2024, 5:16 p.m. UTC | #3 
Hi Tony,

On 2/9/24 13:23, Tony Luck wrote:
> On Fri, Feb 09, 2024 at 09:29:03AM -0600, Moger, Babu wrote:
>> Hi Tony,
>>
>> This patch probably needs to be merged with with patch 7.
> 
> If it just added RESCTRL_NODE to the enum and the switch() I'd
> agree (as patch 7 is where RESCTRL_NODE first used). But this
> patch also adds the sanity checks on scope where it has to be
> a cache level.
> 
> Patch 7 is already on the big side (119 lines added to core.c).
> 
> If you really think this series needs to cut back the
> number of patches, I could move the sanity check pieces
> from here to patch 3 (where the enum is introduced) and
> just the RESCTRL_NODE bits to patch 7.

No. You dont have to cut back on number of patches. I think it is easy to
review if these changes are merged with patch 7.
diff mbox series

Patch

diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
index 2155dc15e636..e3cddf3f07f8 100644
--- a/include/linux/resctrl.h
+++ b/include/linux/resctrl.h
@@ -147,6 +147,7 @@  struct resctrl_schema;
 enum resctrl_scope {
 	RESCTRL_L2_CACHE = 2,
 	RESCTRL_L3_CACHE = 3,
+	RESCTRL_NODE,
 };
 
 /**
diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index 59e6aa7abef5..b741cbf61843 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -505,6 +505,9 @@  static int get_domain_id_from_scope(int cpu, enum resctrl_scope scope)
 	case RESCTRL_L2_CACHE:
 	case RESCTRL_L3_CACHE:
 		return get_cpu_cacheinfo_id(cpu, scope);
+	case RESCTRL_NODE:
+		return cpu_to_node(cpu);
+
 	default:
 		break;
 	}
diff --git a/arch/x86/kernel/cpu/resctrl/pseudo_lock.c b/arch/x86/kernel/cpu/resctrl/pseudo_lock.c
index 6a72fb627aa5..2bafc73b51e2 100644
--- a/arch/x86/kernel/cpu/resctrl/pseudo_lock.c
+++ b/arch/x86/kernel/cpu/resctrl/pseudo_lock.c
@@ -292,10 +292,14 @@  static void pseudo_lock_region_clear(struct pseudo_lock_region *plr)
  */
 static int pseudo_lock_region_init(struct pseudo_lock_region *plr)
 {
+	enum resctrl_scope scope = plr->s->res->scope;
 	struct cpu_cacheinfo *ci;
 	int ret;
 	int i;
 
+	if (WARN_ON_ONCE(scope != RESCTRL_L2_CACHE && scope != RESCTRL_L3_CACHE))
+		return -ENODEV;
+
 	/* Pick the first cpu we find that is associated with the cache. */
 	plr->cpu = cpumask_first(&plr->d->cpu_mask);
 
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index eff9d87547c9..770f2bf98462 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -1413,6 +1413,9 @@  unsigned int rdtgroup_cbm_to_size(struct rdt_resource *r,
 	unsigned int size = 0;
 	int num_b, i;
 
+	if (WARN_ON_ONCE(r->scope != RESCTRL_L2_CACHE && r->scope != RESCTRL_L3_CACHE))
+		return size;
+
 	num_b = bitmap_weight(&cbm, r->cache.cbm_len);
 	ci = get_cpu_cacheinfo(cpumask_any(&d->cpu_mask));
 	for (i = 0; i < ci->num_leaves; i++) {