[PATCH v2 0/3] sched: Simplify ifdeffery around CONFIG_SCHED_SMT

[PATCH v2 0/3] sched: Simplify ifdeffery around CONFIG_SCHED_SMT

[Shrikanth Hegde]

Semantics
=========
- For CONFIG_SCHED_SMT=y:
    No functional change.
- For CONFIG_SCHED_SMT=n:
    - cpu_smt_mask(cpu) becomes cpumask_of(cpu), effectively making it
      per CPU with no siblings.
    - sched_smt_present remains defined, but never becomes active:
    	 Since cpumask_weight(cpumask_of(cpu)) == 1

Performance impact
==================
- CONFIG_SCHED_SMT=y:
    No change in generated code.
- CONFIG_SCHED_SMT=n:
    - Small increase in text size (~0.01%) due to removal of compile-time
      stubs. Most paths remain effectively dead due to static keys.
    - Fast paths are protected using IS_ENABLED(CONFIG_SCHED_SMT).

With that, cpu_smt_mask() to be used unconditionally and reduces
CONFIG_SCHED_SMT-specific code paths, improving readability and
maintainability.

This leaves the remaining use of CONFIG_SCHED_SMT mainly for topology
handling bits.

Since v1[1]:
- Dropped changes for stop_core_cpuslocked. intel_ifs is the only
  user and it gets compiled with SCHED_SMT always. Added comments
  around it why ifdefs are still being kept.
- Decided not to add sched_smt_active checks for sched_core_cpu_deactivate
  and sched_core_cpu_starting, they bail out if weight(smt_mask) == 1.
  Hence core_lock won't be held for long. It is not fastpath either.
- Thanks to Valentin Schneider and Tejun Heo for checking out the series
  and the feedback.

Shrikanth Hegde (3):
  topology: Introduce cpu_smt_mask for CONFIG_SCHED_SMT=n
  sched: Simplify ifdeffery around cpu_smt_mask
  sched/fair: Add compile time check in fastpaths for CONFIG_SCHED_SMT=n

 include/linux/sched/smt.h |  4 ----
 include/linux/topology.h  | 15 +++++++++++++-
 kernel/sched/core.c       |  6 ------
 kernel/sched/ext_idle.c   |  6 ------
 kernel/sched/fair.c       | 41 +++++----------------------------------
 kernel/sched/sched.h      |  6 ------
 kernel/sched/topology.c   |  2 --
 kernel/stop_machine.c     |  5 +++++
 kernel/workqueue.c        |  4 ----
 9 files changed, 24 insertions(+), 65 deletions(-)

-- 
2.47.3

[PATCH v2 1/3] topology: Introduce cpu_smt_mask for CONFIG_SCHED_SMT=n

[Shrikanth Hegde]

Define cpu_smt_mask in case of CONFIG_SCHED_SMT=n as cpumask_of that
CPU. With that config, it is expected that kernel treats each CPU
as individual core. Using cpumask_of(cpu) reflects that.

This would help to get rid of the ifdeffery that is spread across
the codebase since cpu_smt_mask is defined only in case of
CONFIG_SCHED_SMT=y.

Note: There is no arch today which defines cpu_smt_mask unconditionally.
So likely defining the cpu_smt_mask shouldn't lead redefintion errors.

Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
---
 include/linux/topology.h | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/include/linux/topology.h b/include/linux/topology.h
index 6575af39fd10..709a2dcf4c73 100644
--- a/include/linux/topology.h
+++ b/include/linux/topology.h
@@ -230,11 +230,24 @@ static inline int cpu_to_mem(int cpu)
 #define topology_drawer_cpumask(cpu)		cpumask_of(cpu)
 #endif
 
-#if defined(CONFIG_SCHED_SMT) && !defined(cpu_smt_mask)
+/*
+ * Defining cpu_smt_mask as cpumask_of that CPU helps to get
+ * rid of lot of ifdeffery all around the codebase in case of
+ * CONFIG_SCHED_SMT=n. It just means there are no other siblings, which
+ * is what is expected.
+ */
+#if defined(CONFIG_SCHED_SMT)
+# if !defined(cpu_smt_mask)
 static inline const struct cpumask *cpu_smt_mask(int cpu)
 {
 	return topology_sibling_cpumask(cpu);
 }
+# endif
+#else	/* !CONFIG_SCHED_SMT */
+static inline const struct cpumask *cpu_smt_mask(int cpu)
+{
+	return cpumask_of(cpu);
+}
 #endif
 
 #ifndef topology_is_primary_thread
-- 
2.47.3

[PATCH v2 2/3] sched: Simplify ifdeffery around cpu_smt_mask

[Shrikanth Hegde]

Now, that cpu_smt_mask is defined as cpumask_of(cpu) for
CONFIG_SCHED_SMT=n, it is possible to get rid of the ifdeffery.

Effectively,
- This makes sched_smt_present is defined always

- cpumask_weight(cpumask_of(cpu)) == 1. So sched_smt_present_inc/dec
  will never enable the sched_smt_present. Which is expected.

- Paths that were compile-time eliminated become runtime guarded
  using static keys.

- Defines set_idle_cores, test_idle_cores etc which could likely benefit
  the CONFIG_SCHED_SMT=n systems to use the same optimizations within the
  LLC at wakeups.

- This will expose sched_smt_present symbol for CONFIG_SCHED_SMT=n.
  Likely not a concern.

- There a bloat of code CONFIG_SCHED_SMT=n. (NR_CPUS=2048)
  add/remove: 24/18 grow/shrink: 26/28 up/down: 6396/-3188 (3208)
  Total: Before=30629880, After=30633088, chg +0.01%

- No code bloat for CONFIG_SCHED_SMT=y, which is expected.

- Add comments around stop_core_cpuslocked on why ifdefs are not
  removed.

- This leaves the remaining uses of CONFIG_SCHED_SMT mainly for
  topology building bits which have a policy based decision.

Acked-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
---
 include/linux/sched/smt.h |  4 ----
 kernel/sched/core.c       |  6 ------
 kernel/sched/ext_idle.c   |  6 ------
 kernel/sched/fair.c       | 35 -----------------------------------
 kernel/sched/sched.h      |  6 ------
 kernel/sched/topology.c   |  2 --
 kernel/stop_machine.c     |  5 +++++
 kernel/workqueue.c        |  4 ----
 8 files changed, 5 insertions(+), 63 deletions(-)

diff --git a/include/linux/sched/smt.h b/include/linux/sched/smt.h
index 166b19af956f..cde6679c0278 100644
--- a/include/linux/sched/smt.h
+++ b/include/linux/sched/smt.h
@@ -4,16 +4,12 @@
 
 #include <linux/static_key.h>
 
-#ifdef CONFIG_SCHED_SMT
 extern struct static_key_false sched_smt_present;
 
 static __always_inline bool sched_smt_active(void)
 {
 	return static_branch_likely(&sched_smt_present);
 }
-#else
-static __always_inline bool sched_smt_active(void) { return false; }
-#endif
 
 void arch_smt_update(void);
 
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index b905805bbcbe..3ae5f19c1b7e 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -8612,18 +8612,14 @@ static void cpuset_cpu_inactive(unsigned int cpu)
 
 static inline void sched_smt_present_inc(int cpu)
 {
-#ifdef CONFIG_SCHED_SMT
 	if (cpumask_weight(cpu_smt_mask(cpu)) == 2)
 		static_branch_inc_cpuslocked(&sched_smt_present);
-#endif
 }
 
 static inline void sched_smt_present_dec(int cpu)
 {
-#ifdef CONFIG_SCHED_SMT
 	if (cpumask_weight(cpu_smt_mask(cpu)) == 2)
 		static_branch_dec_cpuslocked(&sched_smt_present);
-#endif
 }
 
 int sched_cpu_activate(unsigned int cpu)
@@ -8711,9 +8707,7 @@ int sched_cpu_deactivate(unsigned int cpu)
 	 */
 	sched_smt_present_dec(cpu);
 
-#ifdef CONFIG_SCHED_SMT
 	sched_core_cpu_deactivate(cpu);
-#endif
 
 	if (!sched_smp_initialized)
 		return 0;
diff --git a/kernel/sched/ext_idle.c b/kernel/sched/ext_idle.c
index 6e1980763270..9f5ad6b071f9 100644
--- a/kernel/sched/ext_idle.c
+++ b/kernel/sched/ext_idle.c
@@ -79,7 +79,6 @@ static bool scx_idle_test_and_clear_cpu(int cpu)
 	int node = scx_cpu_node_if_enabled(cpu);
 	struct cpumask *idle_cpus = idle_cpumask(node)->cpu;
 
-#ifdef CONFIG_SCHED_SMT
 	/*
 	 * SMT mask should be cleared whether we can claim @cpu or not. The SMT
 	 * cluster is not wholly idle either way. This also prevents
@@ -104,7 +103,6 @@ static bool scx_idle_test_and_clear_cpu(int cpu)
 		else if (cpumask_test_cpu(cpu, idle_smts))
 			__cpumask_clear_cpu(cpu, idle_smts);
 	}
-#endif
 
 	return cpumask_test_and_clear_cpu(cpu, idle_cpus);
 }
@@ -622,7 +620,6 @@ s32 scx_select_cpu_dfl(struct task_struct *p, s32 prev_cpu, u64 wake_flags,
 		goto out_unlock;
 	}
 
-#ifdef CONFIG_SCHED_SMT
 	/*
 	 * Use @prev_cpu's sibling if it's idle.
 	 */
@@ -634,7 +631,6 @@ s32 scx_select_cpu_dfl(struct task_struct *p, s32 prev_cpu, u64 wake_flags,
 				goto out_unlock;
 		}
 	}
-#endif
 
 	/*
 	 * Search for any idle CPU in the same LLC domain.
@@ -714,7 +710,6 @@ static void update_builtin_idle(int cpu, bool idle)
 
 	assign_cpu(cpu, idle_cpus, idle);
 
-#ifdef CONFIG_SCHED_SMT
 	if (sched_smt_active()) {
 		const struct cpumask *smt = cpu_smt_mask(cpu);
 		struct cpumask *idle_smts = idle_cpumask(node)->smt;
@@ -731,7 +726,6 @@ static void update_builtin_idle(int cpu, bool idle)
 			cpumask_andnot(idle_smts, idle_smts, smt);
 		}
 	}
-#endif
 }
 
 /*
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 3ebec186f982..353e31ecaadc 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -1584,7 +1584,6 @@ update_stats_curr_start(struct cfs_rq *cfs_rq, struct sched_entity *se)
 
 static inline bool is_core_idle(int cpu)
 {
-#ifdef CONFIG_SCHED_SMT
 	int sibling;
 
 	for_each_cpu(sibling, cpu_smt_mask(cpu)) {
@@ -1594,7 +1593,6 @@ static inline bool is_core_idle(int cpu)
 		if (!idle_cpu(sibling))
 			return false;
 	}
-#endif
 
 	return true;
 }
@@ -2277,7 +2275,6 @@ numa_type numa_classify(unsigned int imbalance_pct,
 	return node_fully_busy;
 }
 
-#ifdef CONFIG_SCHED_SMT
 /* Forward declarations of select_idle_sibling helpers */
 static inline bool test_idle_cores(int cpu);
 static inline int numa_idle_core(int idle_core, int cpu)
@@ -2295,12 +2292,6 @@ static inline int numa_idle_core(int idle_core, int cpu)
 
 	return idle_core;
 }
-#else /* !CONFIG_SCHED_SMT: */
-static inline int numa_idle_core(int idle_core, int cpu)
-{
-	return idle_core;
-}
-#endif /* !CONFIG_SCHED_SMT */
 
 /*
  * Gather all necessary information to make NUMA balancing placement
@@ -7811,7 +7802,6 @@ static inline int __select_idle_cpu(int cpu, struct task_struct *p)
 	return -1;
 }
 
-#ifdef CONFIG_SCHED_SMT
 DEFINE_STATIC_KEY_FALSE(sched_smt_present);
 EXPORT_SYMBOL_GPL(sched_smt_present);
 
@@ -7921,29 +7911,6 @@ static int select_idle_smt(struct task_struct *p, struct sched_domain *sd, int t
 	return -1;
 }
 
-#else /* !CONFIG_SCHED_SMT: */
-
-static inline void set_idle_cores(int cpu, int val)
-{
-}
-
-static inline bool test_idle_cores(int cpu)
-{
-	return false;
-}
-
-static inline int select_idle_core(struct task_struct *p, int core, struct cpumask *cpus, int *idle_cpu)
-{
-	return __select_idle_cpu(core, p);
-}
-
-static inline int select_idle_smt(struct task_struct *p, struct sched_domain *sd, int target)
-{
-	return -1;
-}
-
-#endif /* !CONFIG_SCHED_SMT */
-
 /*
  * Scan the LLC domain for idle CPUs; this is dynamically regulated by
  * comparing the average scan cost (tracked in sd->avg_scan_cost) against the
@@ -12036,9 +12003,7 @@ static int should_we_balance(struct lb_env *env)
 			 * idle has been found, then its not needed to check other
 			 * SMT siblings for idleness:
 			 */
-#ifdef CONFIG_SCHED_SMT
 			cpumask_andnot(swb_cpus, swb_cpus, cpu_smt_mask(cpu));
-#endif
 			continue;
 		}
 
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 9f63b15d309d..e476623a0c2a 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1667,7 +1667,6 @@ do {						\
 	flags = _raw_spin_rq_lock_irqsave(rq);	\
 } while (0)
 
-#ifdef CONFIG_SCHED_SMT
 extern void __update_idle_core(struct rq *rq);
 
 static inline void update_idle_core(struct rq *rq)
@@ -1676,12 +1675,7 @@ static inline void update_idle_core(struct rq *rq)
 		__update_idle_core(rq);
 }
 
-#else /* !CONFIG_SCHED_SMT: */
-static inline void update_idle_core(struct rq *rq) { }
-#endif /* !CONFIG_SCHED_SMT */
-
 #ifdef CONFIG_FAIR_GROUP_SCHED
-
 static inline struct task_struct *task_of(struct sched_entity *se)
 {
 	WARN_ON_ONCE(!entity_is_task(se));
diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index 5847b83d9d55..a1f46e3f4ede 100644
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -1310,9 +1310,7 @@ static void init_sched_groups_capacity(int cpu, struct sched_domain *sd)
 		cpumask_copy(mask, sched_group_span(sg));
 		for_each_cpu(cpu, mask) {
 			cores++;
-#ifdef CONFIG_SCHED_SMT
 			cpumask_andnot(mask, mask, cpu_smt_mask(cpu));
-#endif
 		}
 		sg->cores = cores;
 
diff --git a/kernel/stop_machine.c b/kernel/stop_machine.c
index 3fe6b0c99f3d..773d8e9ae30c 100644
--- a/kernel/stop_machine.c
+++ b/kernel/stop_machine.c
@@ -633,6 +633,11 @@ int stop_machine(cpu_stop_fn_t fn, void *data, const struct cpumask *cpus)
 EXPORT_SYMBOL_GPL(stop_machine);
 
 #ifdef CONFIG_SCHED_SMT
+/*
+ * INTEL_IFS is the only user of this API. That selftest can
+ * only be compiled if SMP=y. On x86 it selects SCHED_SMT.
+ * Keep the ifdefs for now.
+ */
 int stop_core_cpuslocked(unsigned int cpu, cpu_stop_fn_t fn, void *data)
 {
 	const struct cpumask *smt_mask = cpu_smt_mask(cpu);
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 3d2e3b2ec528..c911fdcb4428 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -8198,11 +8198,7 @@ static bool __init cpus_dont_share(int cpu0, int cpu1)
 
 static bool __init cpus_share_smt(int cpu0, int cpu1)
 {
-#ifdef CONFIG_SCHED_SMT
 	return cpumask_test_cpu(cpu0, cpu_smt_mask(cpu1));
-#else
-	return false;
-#endif
 }
 
 static bool __init cpus_share_numa(int cpu0, int cpu1)
-- 
2.47.3

[PATCH v2 3/3] sched/fair: Add compile time check in fastpaths for CONFIG_SCHED_SMT=n

[Shrikanth Hegde]

For fastpaths such as wakeup, load balance even a minimal code additons
can pop up. Add IS_ENABLED checks there to ensure there is no overhead.

Other places are either have sched_smt_active() check or they are not in
fast paths.

Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
---
 kernel/sched/fair.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 353e31ecaadc..b6f9592b31fd 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -1586,6 +1586,9 @@ static inline bool is_core_idle(int cpu)
 {
 	int sibling;
 
+	if (!IS_ENABLED(CONFIG_SCHED_SMT))
+		return true;
+
 	for_each_cpu(sibling, cpu_smt_mask(cpu)) {
 		if (cpu == sibling)
 			continue;
@@ -12003,7 +12006,8 @@ static int should_we_balance(struct lb_env *env)
 			 * idle has been found, then its not needed to check other
 			 * SMT siblings for idleness:
 			 */
-			cpumask_andnot(swb_cpus, swb_cpus, cpu_smt_mask(cpu));
+			if (IS_ENABLED(CONFIG_SCHED_SMT))
+				cpumask_andnot(swb_cpus, swb_cpus, cpu_smt_mask(cpu));
 			continue;
 		}
 
-- 
2.47.3

Re: [PATCH v2 1/3] topology: Introduce cpu_smt_mask for CONFIG_SCHED_SMT=n

[Phil Auld]

On Tue, May 12, 2026 at 08:51:23PM +0530 Shrikanth Hegde wrote:
> Define cpu_smt_mask in case of CONFIG_SCHED_SMT=n as cpumask_of that
> CPU. With that config, it is expected that kernel treats each CPU
> as individual core. Using cpumask_of(cpu) reflects that.
> 
> This would help to get rid of the ifdeffery that is spread across
> the codebase since cpu_smt_mask is defined only in case of
> CONFIG_SCHED_SMT=y.
> 
> Note: There is no arch today which defines cpu_smt_mask unconditionally.
> So likely defining the cpu_smt_mask shouldn't lead redefintion errors.
> 
> Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>


I like this whole cleanup, fwiw.

Reviewed-by: Phil Auld <pauld@redhat.com>


Cheers,
Phil

> ---
>  include/linux/topology.h | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/topology.h b/include/linux/topology.h
> index 6575af39fd10..709a2dcf4c73 100644
> --- a/include/linux/topology.h
> +++ b/include/linux/topology.h
> @@ -230,11 +230,24 @@ static inline int cpu_to_mem(int cpu)
>  #define topology_drawer_cpumask(cpu)		cpumask_of(cpu)
>  #endif
>  
> -#if defined(CONFIG_SCHED_SMT) && !defined(cpu_smt_mask)
> +/*
> + * Defining cpu_smt_mask as cpumask_of that CPU helps to get
> + * rid of lot of ifdeffery all around the codebase in case of
> + * CONFIG_SCHED_SMT=n. It just means there are no other siblings, which
> + * is what is expected.
> + */
> +#if defined(CONFIG_SCHED_SMT)
> +# if !defined(cpu_smt_mask)
>  static inline const struct cpumask *cpu_smt_mask(int cpu)
>  {
>  	return topology_sibling_cpumask(cpu);
>  }
> +# endif
> +#else	/* !CONFIG_SCHED_SMT */
> +static inline const struct cpumask *cpu_smt_mask(int cpu)
> +{
> +	return cpumask_of(cpu);
> +}
>  #endif
>  
>  #ifndef topology_is_primary_thread
> -- 
> 2.47.3
> 
> 

-- 

Re: [PATCH v2 2/3] sched: Simplify ifdeffery around cpu_smt_mask

[Phil Auld]

On Tue, May 12, 2026 at 08:51:24PM +0530 Shrikanth Hegde wrote:
> Now, that cpu_smt_mask is defined as cpumask_of(cpu) for
> CONFIG_SCHED_SMT=n, it is possible to get rid of the ifdeffery.
> 
> Effectively,
> - This makes sched_smt_present is defined always
> 
> - cpumask_weight(cpumask_of(cpu)) == 1. So sched_smt_present_inc/dec
>   will never enable the sched_smt_present. Which is expected.
> 
> - Paths that were compile-time eliminated become runtime guarded
>   using static keys.
> 
> - Defines set_idle_cores, test_idle_cores etc which could likely benefit
>   the CONFIG_SCHED_SMT=n systems to use the same optimizations within the
>   LLC at wakeups.
> 
> - This will expose sched_smt_present symbol for CONFIG_SCHED_SMT=n.
>   Likely not a concern.
> 
> - There a bloat of code CONFIG_SCHED_SMT=n. (NR_CPUS=2048)
>   add/remove: 24/18 grow/shrink: 26/28 up/down: 6396/-3188 (3208)
>   Total: Before=30629880, After=30633088, chg +0.01%
> 
> - No code bloat for CONFIG_SCHED_SMT=y, which is expected.
> 
> - Add comments around stop_core_cpuslocked on why ifdefs are not
>   removed.
> 
> - This leaves the remaining uses of CONFIG_SCHED_SMT mainly for
>   topology building bits which have a policy based decision.
> 
> Acked-by: Tejun Heo <tj@kernel.org>
> Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>

LGTM.

Reviewed-by: Phil Auld <pauld@redhat.com>

Cheers,
Phil


> ---
>  include/linux/sched/smt.h |  4 ----
>  kernel/sched/core.c       |  6 ------
>  kernel/sched/ext_idle.c   |  6 ------
>  kernel/sched/fair.c       | 35 -----------------------------------
>  kernel/sched/sched.h      |  6 ------
>  kernel/sched/topology.c   |  2 --
>  kernel/stop_machine.c     |  5 +++++
>  kernel/workqueue.c        |  4 ----
>  8 files changed, 5 insertions(+), 63 deletions(-)
> 
> diff --git a/include/linux/sched/smt.h b/include/linux/sched/smt.h
> index 166b19af956f..cde6679c0278 100644
> --- a/include/linux/sched/smt.h
> +++ b/include/linux/sched/smt.h
> @@ -4,16 +4,12 @@
>  
>  #include <linux/static_key.h>
>  
> -#ifdef CONFIG_SCHED_SMT
>  extern struct static_key_false sched_smt_present;
>  
>  static __always_inline bool sched_smt_active(void)
>  {
>  	return static_branch_likely(&sched_smt_present);
>  }
> -#else
> -static __always_inline bool sched_smt_active(void) { return false; }
> -#endif
>  
>  void arch_smt_update(void);
>  
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index b905805bbcbe..3ae5f19c1b7e 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -8612,18 +8612,14 @@ static void cpuset_cpu_inactive(unsigned int cpu)
>  
>  static inline void sched_smt_present_inc(int cpu)
>  {
> -#ifdef CONFIG_SCHED_SMT
>  	if (cpumask_weight(cpu_smt_mask(cpu)) == 2)
>  		static_branch_inc_cpuslocked(&sched_smt_present);
> -#endif
>  }
>  
>  static inline void sched_smt_present_dec(int cpu)
>  {
> -#ifdef CONFIG_SCHED_SMT
>  	if (cpumask_weight(cpu_smt_mask(cpu)) == 2)
>  		static_branch_dec_cpuslocked(&sched_smt_present);
> -#endif
>  }
>  
>  int sched_cpu_activate(unsigned int cpu)
> @@ -8711,9 +8707,7 @@ int sched_cpu_deactivate(unsigned int cpu)
>  	 */
>  	sched_smt_present_dec(cpu);
>  
> -#ifdef CONFIG_SCHED_SMT
>  	sched_core_cpu_deactivate(cpu);
> -#endif
>  
>  	if (!sched_smp_initialized)
>  		return 0;
> diff --git a/kernel/sched/ext_idle.c b/kernel/sched/ext_idle.c
> index 6e1980763270..9f5ad6b071f9 100644
> --- a/kernel/sched/ext_idle.c
> +++ b/kernel/sched/ext_idle.c
> @@ -79,7 +79,6 @@ static bool scx_idle_test_and_clear_cpu(int cpu)
>  	int node = scx_cpu_node_if_enabled(cpu);
>  	struct cpumask *idle_cpus = idle_cpumask(node)->cpu;
>  
> -#ifdef CONFIG_SCHED_SMT
>  	/*
>  	 * SMT mask should be cleared whether we can claim @cpu or not. The SMT
>  	 * cluster is not wholly idle either way. This also prevents
> @@ -104,7 +103,6 @@ static bool scx_idle_test_and_clear_cpu(int cpu)
>  		else if (cpumask_test_cpu(cpu, idle_smts))
>  			__cpumask_clear_cpu(cpu, idle_smts);
>  	}
> -#endif
>  
>  	return cpumask_test_and_clear_cpu(cpu, idle_cpus);
>  }
> @@ -622,7 +620,6 @@ s32 scx_select_cpu_dfl(struct task_struct *p, s32 prev_cpu, u64 wake_flags,
>  		goto out_unlock;
>  	}
>  
> -#ifdef CONFIG_SCHED_SMT
>  	/*
>  	 * Use @prev_cpu's sibling if it's idle.
>  	 */
> @@ -634,7 +631,6 @@ s32 scx_select_cpu_dfl(struct task_struct *p, s32 prev_cpu, u64 wake_flags,
>  				goto out_unlock;
>  		}
>  	}
> -#endif
>  
>  	/*
>  	 * Search for any idle CPU in the same LLC domain.
> @@ -714,7 +710,6 @@ static void update_builtin_idle(int cpu, bool idle)
>  
>  	assign_cpu(cpu, idle_cpus, idle);
>  
> -#ifdef CONFIG_SCHED_SMT
>  	if (sched_smt_active()) {
>  		const struct cpumask *smt = cpu_smt_mask(cpu);
>  		struct cpumask *idle_smts = idle_cpumask(node)->smt;
> @@ -731,7 +726,6 @@ static void update_builtin_idle(int cpu, bool idle)
>  			cpumask_andnot(idle_smts, idle_smts, smt);
>  		}
>  	}
> -#endif
>  }
>  
>  /*
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 3ebec186f982..353e31ecaadc 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -1584,7 +1584,6 @@ update_stats_curr_start(struct cfs_rq *cfs_rq, struct sched_entity *se)
>  
>  static inline bool is_core_idle(int cpu)
>  {
> -#ifdef CONFIG_SCHED_SMT
>  	int sibling;
>  
>  	for_each_cpu(sibling, cpu_smt_mask(cpu)) {
> @@ -1594,7 +1593,6 @@ static inline bool is_core_idle(int cpu)
>  		if (!idle_cpu(sibling))
>  			return false;
>  	}
> -#endif
>  
>  	return true;
>  }
> @@ -2277,7 +2275,6 @@ numa_type numa_classify(unsigned int imbalance_pct,
>  	return node_fully_busy;
>  }
>  
> -#ifdef CONFIG_SCHED_SMT
>  /* Forward declarations of select_idle_sibling helpers */
>  static inline bool test_idle_cores(int cpu);
>  static inline int numa_idle_core(int idle_core, int cpu)
> @@ -2295,12 +2292,6 @@ static inline int numa_idle_core(int idle_core, int cpu)
>  
>  	return idle_core;
>  }
> -#else /* !CONFIG_SCHED_SMT: */
> -static inline int numa_idle_core(int idle_core, int cpu)
> -{
> -	return idle_core;
> -}
> -#endif /* !CONFIG_SCHED_SMT */
>  
>  /*
>   * Gather all necessary information to make NUMA balancing placement
> @@ -7811,7 +7802,6 @@ static inline int __select_idle_cpu(int cpu, struct task_struct *p)
>  	return -1;
>  }
>  
> -#ifdef CONFIG_SCHED_SMT
>  DEFINE_STATIC_KEY_FALSE(sched_smt_present);
>  EXPORT_SYMBOL_GPL(sched_smt_present);
>  
> @@ -7921,29 +7911,6 @@ static int select_idle_smt(struct task_struct *p, struct sched_domain *sd, int t
>  	return -1;
>  }
>  
> -#else /* !CONFIG_SCHED_SMT: */
> -
> -static inline void set_idle_cores(int cpu, int val)
> -{
> -}
> -
> -static inline bool test_idle_cores(int cpu)
> -{
> -	return false;
> -}
> -
> -static inline int select_idle_core(struct task_struct *p, int core, struct cpumask *cpus, int *idle_cpu)
> -{
> -	return __select_idle_cpu(core, p);
> -}
> -
> -static inline int select_idle_smt(struct task_struct *p, struct sched_domain *sd, int target)
> -{
> -	return -1;
> -}
> -
> -#endif /* !CONFIG_SCHED_SMT */
> -
>  /*
>   * Scan the LLC domain for idle CPUs; this is dynamically regulated by
>   * comparing the average scan cost (tracked in sd->avg_scan_cost) against the
> @@ -12036,9 +12003,7 @@ static int should_we_balance(struct lb_env *env)
>  			 * idle has been found, then its not needed to check other
>  			 * SMT siblings for idleness:
>  			 */
> -#ifdef CONFIG_SCHED_SMT
>  			cpumask_andnot(swb_cpus, swb_cpus, cpu_smt_mask(cpu));
> -#endif
>  			continue;
>  		}
>  
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index 9f63b15d309d..e476623a0c2a 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -1667,7 +1667,6 @@ do {						\
>  	flags = _raw_spin_rq_lock_irqsave(rq);	\
>  } while (0)
>  
> -#ifdef CONFIG_SCHED_SMT
>  extern void __update_idle_core(struct rq *rq);
>  
>  static inline void update_idle_core(struct rq *rq)
> @@ -1676,12 +1675,7 @@ static inline void update_idle_core(struct rq *rq)
>  		__update_idle_core(rq);
>  }
>  
> -#else /* !CONFIG_SCHED_SMT: */
> -static inline void update_idle_core(struct rq *rq) { }
> -#endif /* !CONFIG_SCHED_SMT */
> -
>  #ifdef CONFIG_FAIR_GROUP_SCHED
> -
>  static inline struct task_struct *task_of(struct sched_entity *se)
>  {
>  	WARN_ON_ONCE(!entity_is_task(se));
> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> index 5847b83d9d55..a1f46e3f4ede 100644
> --- a/kernel/sched/topology.c
> +++ b/kernel/sched/topology.c
> @@ -1310,9 +1310,7 @@ static void init_sched_groups_capacity(int cpu, struct sched_domain *sd)
>  		cpumask_copy(mask, sched_group_span(sg));
>  		for_each_cpu(cpu, mask) {
>  			cores++;
> -#ifdef CONFIG_SCHED_SMT
>  			cpumask_andnot(mask, mask, cpu_smt_mask(cpu));
> -#endif
>  		}
>  		sg->cores = cores;
>  
> diff --git a/kernel/stop_machine.c b/kernel/stop_machine.c
> index 3fe6b0c99f3d..773d8e9ae30c 100644
> --- a/kernel/stop_machine.c
> +++ b/kernel/stop_machine.c
> @@ -633,6 +633,11 @@ int stop_machine(cpu_stop_fn_t fn, void *data, const struct cpumask *cpus)
>  EXPORT_SYMBOL_GPL(stop_machine);
>  
>  #ifdef CONFIG_SCHED_SMT
> +/*
> + * INTEL_IFS is the only user of this API. That selftest can
> + * only be compiled if SMP=y. On x86 it selects SCHED_SMT.
> + * Keep the ifdefs for now.
> + */
>  int stop_core_cpuslocked(unsigned int cpu, cpu_stop_fn_t fn, void *data)
>  {
>  	const struct cpumask *smt_mask = cpu_smt_mask(cpu);
> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> index 3d2e3b2ec528..c911fdcb4428 100644
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
> @@ -8198,11 +8198,7 @@ static bool __init cpus_dont_share(int cpu0, int cpu1)
>  
>  static bool __init cpus_share_smt(int cpu0, int cpu1)
>  {
> -#ifdef CONFIG_SCHED_SMT
>  	return cpumask_test_cpu(cpu0, cpu_smt_mask(cpu1));
> -#else
> -	return false;
> -#endif
>  }
>  
>  static bool __init cpus_share_numa(int cpu0, int cpu1)
> -- 
> 2.47.3
> 
> 

-- 

Re: [PATCH v2 3/3] sched/fair: Add compile time check in fastpaths for CONFIG_SCHED_SMT=n

[Phil Auld]

Hi Shrikanth,

On Tue, May 12, 2026 at 08:51:25PM +0530 Shrikanth Hegde wrote:
> For fastpaths such as wakeup, load balance even a minimal code additons
> can pop up. Add IS_ENABLED checks there to ensure there is no overhead.

"For fastpaths such as wakeup and load balance, even minimal code additions
can add up. Add IS_ENABLED() checks..."  ?

> 
> Other places are either have sched_smt_active() check or they are not in

s/are//

"have a sched_smt_active_check()" or drop "check" ? 


Otherwise lgtm.

Reviewed-by: Phil Auld <pauld@redhat.com>


Cheers,
Phil



> fast paths.
> 
> Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
> ---
>  kernel/sched/fair.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 353e31ecaadc..b6f9592b31fd 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -1586,6 +1586,9 @@ static inline bool is_core_idle(int cpu)
>  {
>  	int sibling;
>  
> +	if (!IS_ENABLED(CONFIG_SCHED_SMT))
> +		return true;
> +
>  	for_each_cpu(sibling, cpu_smt_mask(cpu)) {
>  		if (cpu == sibling)
>  			continue;
> @@ -12003,7 +12006,8 @@ static int should_we_balance(struct lb_env *env)
>  			 * idle has been found, then its not needed to check other
>  			 * SMT siblings for idleness:
>  			 */
> -			cpumask_andnot(swb_cpus, swb_cpus, cpu_smt_mask(cpu));
> +			if (IS_ENABLED(CONFIG_SCHED_SMT))
> +				cpumask_andnot(swb_cpus, swb_cpus, cpu_smt_mask(cpu));
>  			continue;
>  		}
>  
> -- 
> 2.47.3
> 
> 

-- 

Re: [PATCH v2 0/3] sched: Simplify ifdeffery around CONFIG_SCHED_SMT

[Valentin Schneider]

On 12/05/26 20:51, Shrikanth Hegde wrote:
> Semantics
> =========
> - For CONFIG_SCHED_SMT=y:
>     No functional change.
> - For CONFIG_SCHED_SMT=n:
>     - cpu_smt_mask(cpu) becomes cpumask_of(cpu), effectively making it
>       per CPU with no siblings.
>     - sched_smt_present remains defined, but never becomes active:
>        Since cpumask_weight(cpumask_of(cpu)) == 1
>
> Performance impact
> ==================
> - CONFIG_SCHED_SMT=y:
>     No change in generated code.
> - CONFIG_SCHED_SMT=n:
>     - Small increase in text size (~0.01%) due to removal of compile-time
>       stubs. Most paths remain effectively dead due to static keys.
>     - Fast paths are protected using IS_ENABLED(CONFIG_SCHED_SMT).
>
> With that, cpu_smt_mask() to be used unconditionally and reduces
> CONFIG_SCHED_SMT-specific code paths, improving readability and
> maintainability.
>
> This leaves the remaining use of CONFIG_SCHED_SMT mainly for topology
> handling bits.
>

Reviewed-by: Valentin Schneider <vschneid@redhat.com>

Re: [PATCH v2 3/3] sched/fair: Add compile time check in fastpaths for CONFIG_SCHED_SMT=n

[Shrikanth Hegde]

Hi Phil.

On 5/12/26 10:29 PM, Phil Auld wrote:
> Hi Shrikanth,
> 
> On Tue, May 12, 2026 at 08:51:25PM +0530 Shrikanth Hegde wrote:
>> For fastpaths such as wakeup, load balance even a minimal code additons
>> can pop up. Add IS_ENABLED checks there to ensure there is no overhead.
> 
> "For fastpaths such as wakeup and load balance, even minimal code additions
> can add up. Add IS_ENABLED() checks..."  ?
> 

Ok.

>>
>> Other places are either have sched_smt_active() check or they are not in
> 
> s/are//

ok.

> 
> "have a sched_smt_active_check()" or drop "check" ?
> 
> 

I was saying sched_smt_active() runtime gating is there for many fast paths where
cpu_smt_mask is being referred or those code paths are not fast paths.

Let me re-word a bit.

> Otherwise lgtm.
> 
> Reviewed-by: Phil Auld <pauld@redhat.com>
> 
> 
> Cheers,
> Phil
> 
Thanks for the review.

Re: [PATCH v2 3/3] sched/fair: Add compile time check in fastpaths for CONFIG_SCHED_SMT=n

[K Prateek Nayak]

Hello Shrikanth,

Thank you for cleaning these bits up! Couple comments below:

On 5/12/2026 8:51 PM, Shrikanth Hegde wrote:
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 353e31ecaadc..b6f9592b31fd 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -1586,6 +1586,9 @@ static inline bool is_core_idle(int cpu)
>  {
>  	int sibling;
>  
> +	if (!IS_ENABLED(CONFIG_SCHED_SMT))
> +		return true;

I think !sched_smt_active() can also return early here. That said ...

> +
>  	for_each_cpu(sibling, cpu_smt_mask(cpu)) {
>  		if (cpu == sibling)
>  			continue;
> @@ -12003,7 +12006,8 @@ static int should_we_balance(struct lb_env *env)

... I think the if() above can simply check for sched_smt_active()
before calling is_core_idle() like:

    if (sched_smt_active() &&
        !(env->sd->flags & SD_SHARE_CPUCAPACITY) &&
        !is_core_idle(cpu)) {
        ...
    }

That way, we ensure we don't call is_core_idle() and the cpumask_and()
unnecessarily on topologies that don't have SMT domain similar to how
sched_use_asym_prio() guards the call to is_core_idle().

Thoughts?

>  			 * idle has been found, then its not needed to check other
>  			 * SMT siblings for idleness:
>  			 */
> -			cpumask_andnot(swb_cpus, swb_cpus, cpu_smt_mask(cpu));
> +			if (IS_ENABLED(CONFIG_SCHED_SMT))
> +				cpumask_andnot(swb_cpus, swb_cpus, cpu_smt_mask(cpu));
>  			continue;
>  		}
>  

-- 
Thanks and Regards,
Prateek

Re: [PATCH v2 3/3] sched/fair: Add compile time check in fastpaths for CONFIG_SCHED_SMT=n

[Shrikanth Hegde]

On 5/13/26 11:37 AM, K Prateek Nayak wrote:
> Hello Shrikanth,
> 
> Thank you for cleaning these bits up! Couple comments below:
> 
> On 5/12/2026 8:51 PM, Shrikanth Hegde wrote:
>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>> index 353e31ecaadc..b6f9592b31fd 100644
>> --- a/kernel/sched/fair.c
>> +++ b/kernel/sched/fair.c
>> @@ -1586,6 +1586,9 @@ static inline bool is_core_idle(int cpu)
>>   {
>>   	int sibling;
>>   
>> +	if (!IS_ENABLED(CONFIG_SCHED_SMT))
>> +		return true;
> 
> I think !sched_smt_active() can also return early here. That said ...
> 

If we do below, the rest users already have the checks.
So we should be good.
I will add a comment saying this expects to be called under sched_smt_active.

>> +
>>   	for_each_cpu(sibling, cpu_smt_mask(cpu)) {
>>   		if (cpu == sibling)
>>   			continue;
>> @@ -12003,7 +12006,8 @@ static int should_we_balance(struct lb_env *env)
> 
> ... I think the if() above can simply check for sched_smt_active()
> before calling is_core_idle() like:
> 
>      if (sched_smt_active() &&
>          !(env->sd->flags & SD_SHARE_CPUCAPACITY) &&
>          !is_core_idle(cpu)) {
>          ...
>      }
> 
> That way, we ensure we don't call is_core_idle() and the cpumask_and()
> unnecessarily on topologies that don't have SMT domain similar to how
> sched_use_asym_prio() guards the call to is_core_idle().
> 
> Thoughts?

Ack. Let me edit the v3. I was about to send.

> 
>>   			 * idle has been found, then its not needed to check other
>>   			 * SMT siblings for idleness:
>>   			 */
>> -			cpumask_andnot(swb_cpus, swb_cpus, cpu_smt_mask(cpu));
>> +			if (IS_ENABLED(CONFIG_SCHED_SMT))
>> +				cpumask_andnot(swb_cpus, swb_cpus, cpu_smt_mask(cpu));
>>   			continue;
>>   		}
>>   
> 

Re: [PATCH v2 0/3] sched: Simplify ifdeffery around CONFIG_SCHED_SMT

[K Prateek Nayak]

Hello Shrikanth,

On 5/12/2026 8:51 PM, Shrikanth Hegde wrote:
> Semantics
> =========
> - For CONFIG_SCHED_SMT=y:
>     No functional change.
> - For CONFIG_SCHED_SMT=n:
>     - cpu_smt_mask(cpu) becomes cpumask_of(cpu), effectively making it
>       per CPU with no siblings.
>     - sched_smt_present remains defined, but never becomes active:
>     	 Since cpumask_weight(cpumask_of(cpu)) == 1
> 
> Performance impact
> ==================
> - CONFIG_SCHED_SMT=y:
>     No change in generated code.
> - CONFIG_SCHED_SMT=n:
>     - Small increase in text size (~0.01%) due to removal of compile-time
>       stubs. Most paths remain effectively dead due to static keys.
>     - Fast paths are protected using IS_ENABLED(CONFIG_SCHED_SMT).
> 
> With that, cpu_smt_mask() to be used unconditionally and reduces
> CONFIG_SCHED_SMT-specific code paths, improving readability and
> maintainability.
> 
> This leaves the remaining use of CONFIG_SCHED_SMT mainly for topology
> handling bits.

Thank you again cleaning this up! Feel free to include:

Tested-by: K Prateek Nayak <kprateek.nayak@amd.com>

I also tested removing the "select SCHED_SMT" dependency for x86 after
guarding SDTL_INIT(..., SMT) behind CONFIG_SCHED_SMT and the system is
up fine; performance is terrible as expected :-)

-- 
Thanks and Regards,
Prateek

Re: [PATCH v2 3/3] sched/fair: Add compile time check in fastpaths for CONFIG_SCHED_SMT=n

[Shrikanth Hegde]

Hi Prateek.

> ... I think the if() above can simply check for sched_smt_active()
> before calling is_core_idle() like:
> 
>      if (sched_smt_active() &&
>          !(env->sd->flags & SD_SHARE_CPUCAPACITY) &&
>          !is_core_idle(cpu)) {
>          ...
>      }
> 
> That way, we ensure we don't call is_core_idle() and the cpumask_and()
> unnecessarily on topologies that don't have SMT domain similar to how
> sched_use_asym_prio() guards the call to is_core_idle().
> 
> Thoughts?
> 
In addition, Do you think it makes sense to consolidate use of
sched_smt_present always using sched_smt_active() ?


static __always_inline bool sched_smt_active(void)
{
         return static_branch_likely(&sched_smt_present);
}

kernel/sched/core_sched.c:      if (!static_branch_likely(&sched_smt_present))
kernel/sched/fair.c:    if (!static_branch_likely(&sched_smt_present) ||
kernel/sched/sched.h:   if (static_branch_unlikely(&sched_smt_present))


The last being differing with unlikely in update_idle_core.
Does it make sense to call sched_smt_active there as well?

Re: [PATCH v2 3/3] sched/fair: Add compile time check in fastpaths for CONFIG_SCHED_SMT=n

[K Prateek Nayak]

Hello Shrikanth,

On 5/13/2026 12:09 PM, Shrikanth Hegde wrote:
> 
> Hi Prateek.
> 
>> ... I think the if() above can simply check for sched_smt_active()
>> before calling is_core_idle() like:
>>
>>      if (sched_smt_active() &&
>>          !(env->sd->flags & SD_SHARE_CPUCAPACITY) &&
>>          !is_core_idle(cpu)) {
>>          ...
>>      }
>>
>> That way, we ensure we don't call is_core_idle() and the cpumask_and()
>> unnecessarily on topologies that don't have SMT domain similar to how
>> sched_use_asym_prio() guards the call to is_core_idle().
>>
>> Thoughts?
>>
> In addition, Do you think it makes sense to consolidate use of
> sched_smt_present always using sched_smt_active() ?
> 
> 
> static __always_inline bool sched_smt_active(void)
> {
>         return static_branch_likely(&sched_smt_present);
> }
> 
> kernel/sched/core_sched.c:      if (!static_branch_likely(&sched_smt_present))
> kernel/sched/fair.c:    if (!static_branch_likely(&sched_smt_present) ||
> kernel/sched/sched.h:   if (static_branch_unlikely(&sched_smt_present))
> 
> 
> The last being differing with unlikely in update_idle_core.
> Does it make sense to call sched_smt_active there as well?

I think using sched_smt_active() everywhere should be fine.

I'm not sure what made update_idle_core() special; Perhaps there was
a greater chance of running it on systems with CONFIG_SCHED_SMT
enabled but doesn't physically have SMT.

This came from commit 1b568f0aabf2 ("sched/core: Optimize SCHED_SMT")
in 2016 when SMT was perhaps not so common and sched_smt_active()
came later in 2018 with the spectre mitigation optimizations on x86
where SMT was more prevalent so it defaulted to likely.

I'll let Peter comment if he prefers keeping this branch unlikely
but from my side, unifying eveything under sched_smt_active() looks
better.

-- 
Thanks and Regards,
Prateek

Re: [PATCH v2 3/3] sched/fair: Add compile time check in fastpaths for CONFIG_SCHED_SMT=n

[Phil Auld]

On Wed, May 13, 2026 at 09:45:53AM +0530 Shrikanth Hegde wrote:
> Hi Phil.
> 
> On 5/12/26 10:29 PM, Phil Auld wrote:
> > Hi Shrikanth,
> > 
> > On Tue, May 12, 2026 at 08:51:25PM +0530 Shrikanth Hegde wrote:
> > > For fastpaths such as wakeup, load balance even a minimal code additons
> > > can pop up. Add IS_ENABLED checks there to ensure there is no overhead.
> > 
> > "For fastpaths such as wakeup and load balance, even minimal code additions
> > can add up. Add IS_ENABLED() checks..."  ?
> > 
> 
> Ok.
> 
> > > 
> > > Other places are either have sched_smt_active() check or they are not in
> > 
> > s/are//
> 
> ok.
> 
> > 
> > "have a sched_smt_active_check()" or drop "check" ?
> > 
> > 
> 
> I was saying sched_smt_active() runtime gating is there for many fast paths where
> cpu_smt_mask is being referred or those code paths are not fast paths.
> 
> Let me re-word a bit.
>

I know what you were saying. I just wanted an article when I read it :)


Cheers,
Phil

> > Otherwise lgtm.
> > 
> > Reviewed-by: Phil Auld <pauld@redhat.com>
> > 
> > 
> > Cheers,
> > Phil
> > 
> Thanks for the review.
> 

--