[PATCH 0/6] x86: remove cpu_**_mask() functions

[PATCH 0/6] x86: remove cpu_**_mask() functions

[Bartosz Golaszewski]

Two functions defined in asm/smp.h: cpu_sibling_mask() and cpu_core_mask(),
have the same functionality as macros defined in linux/topology.h:
topology_thread_cpumask() and topology_core_cpumask() respectively.

They are not documented and are architecture specific. Also: different
naming for the same functionality (in case of cpu_sibling_mask() and
topology_thread_cpumask()) is a bit misleading.

This series makes all the callers switch to using the topology.h macros
and removes their smp.h counterparts.

The same duplication exists on powerpc.

Bartosz Golaszewski (6):
  coretemp: replace cpu_sibling_mask() with topology_thread_cpumask()
  powernow-k8: replace cpu_core_mask() with topology_core_cpumask()
  p4-clockmod: replace cpu_sibling_mask() with topology_thread_cpumask()
  acpi-cpufreq: replace cpu_**_mask() with topology_**_cpumask()
  x86: replace cpu_**_mask() with topology_**_cpumask()
  x86: remove cpu_sibling_mask() and cpu_core_mask()

 arch/x86/include/asm/smp.h     | 10 ----------
 arch/x86/kernel/cpu/proc.c     |  5 +++--
 arch/x86/kernel/smpboot.c      | 40 ++++++++++++++++++++--------------------
 arch/x86/kernel/tsc_sync.c     |  3 +--
 drivers/cpufreq/acpi-cpufreq.c |  5 +++--
 drivers/cpufreq/p4-clockmod.c  |  2 +-
 drivers/cpufreq/powernow-k8.c  | 13 +++----------
 drivers/hwmon/coretemp.c       |  3 ++-
 8 files changed, 33 insertions(+), 48 deletions(-)

-- 
2.1.4

[PATCH 1/6] coretemp: replace cpu_sibling_mask() with topology_thread_cpumask()

[Bartosz Golaszewski]

The former duplicates the functionality of the latter but is neither
documented nor arch-independent.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/hwmon/coretemp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/coretemp.c b/drivers/hwmon/coretemp.c
index ed303ba..f9b65f9 100644
--- a/drivers/hwmon/coretemp.c
+++ b/drivers/hwmon/coretemp.c
@@ -63,7 +63,8 @@ MODULE_PARM_DESC(tjmax, "TjMax value in degrees Celsius");
 #define TO_ATTR_NO(cpu)		(TO_CORE_ID(cpu) + BASE_SYSFS_ATTR_NO)
 
 #ifdef CONFIG_SMP
-#define for_each_sibling(i, cpu)	for_each_cpu(i, cpu_sibling_mask(cpu))
+#define for_each_sibling(i, cpu) \
+	for_each_cpu(i, topology_thread_cpumask(cpu))
 #else
 #define for_each_sibling(i, cpu)	for (i = 0; false; )
 #endif
-- 
2.1.4

Re: [PATCH 1/6] coretemp: replace cpu_sibling_mask() with topology_thread_cpumask()

[Guenter Roeck]

On 04/30/2015 03:03 AM, Bartosz Golaszewski wrote:
> The former duplicates the functionality of the latter but is neither
> documented nor arch-independent.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Acked-by: Guenter Roeck <linux@roeck-us.net>

> ---
>   drivers/hwmon/coretemp.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hwmon/coretemp.c b/drivers/hwmon/coretemp.c
> index ed303ba..f9b65f9 100644
> --- a/drivers/hwmon/coretemp.c
> +++ b/drivers/hwmon/coretemp.c
> @@ -63,7 +63,8 @@ MODULE_PARM_DESC(tjmax, "TjMax value in degrees Celsius");
>   #define TO_ATTR_NO(cpu)		(TO_CORE_ID(cpu) + BASE_SYSFS_ATTR_NO)
>
>   #ifdef CONFIG_SMP
> -#define for_each_sibling(i, cpu)	for_each_cpu(i, cpu_sibling_mask(cpu))
> +#define for_each_sibling(i, cpu) \
> +	for_each_cpu(i, topology_thread_cpumask(cpu))
>   #else
>   #define for_each_sibling(i, cpu)	for (i = 0; false; )
>   #endif
>

[PATCH 2/6] powernow-k8: replace cpu_core_mask() with topology_core_cpumask()

[Bartosz Golaszewski]

The former duplicates the functionality of the latter but is neither
documented nor arch-independent.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/cpufreq/powernow-k8.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/cpufreq/powernow-k8.c b/drivers/cpufreq/powernow-k8.c
index f9ce7e4..5c035d0 100644
--- a/drivers/cpufreq/powernow-k8.c
+++ b/drivers/cpufreq/powernow-k8.c
@@ -57,13 +57,6 @@ static DEFINE_PER_CPU(struct powernow_k8_data *, powernow_data);
 
 static struct cpufreq_driver cpufreq_amd64_driver;
 
-#ifndef CONFIG_SMP
-static inline const struct cpumask *cpu_core_mask(int cpu)
-{
-	return cpumask_of(0);
-}
-#endif
-
 /* Return a frequency in MHz, given an input fid */
 static u32 find_freq_from_fid(u32 fid)
 {
@@ -620,7 +613,7 @@ static int fill_powernow_table(struct powernow_k8_data *data,
 
 	pr_debug("cfid 0x%x, cvid 0x%x\n", data->currfid, data->currvid);
 	data->powernow_table = powernow_table;
-	if (cpumask_first(cpu_core_mask(data->cpu)) == data->cpu)
+	if (cpumask_first(topology_core_cpumask(data->cpu)) == data->cpu)
 		print_basics(data);
 
 	for (j = 0; j < data->numps; j++)
@@ -784,7 +777,7 @@ static int powernow_k8_cpu_init_acpi(struct powernow_k8_data *data)
 		CPUFREQ_TABLE_END;
 	data->powernow_table = powernow_table;
 
-	if (cpumask_first(cpu_core_mask(data->cpu)) == data->cpu)
+	if (cpumask_first(topology_core_cpumask(data->cpu)) == data->cpu)
 		print_basics(data);
 
 	/* notify BIOS that we exist */
@@ -1090,7 +1083,7 @@ static int powernowk8_cpu_init(struct cpufreq_policy *pol)
 	if (rc != 0)
 		goto err_out_exit_acpi;
 
-	cpumask_copy(pol->cpus, cpu_core_mask(pol->cpu));
+	cpumask_copy(pol->cpus, topology_core_cpumask(pol->cpu));
 	data->available_cores = pol->cpus;
 
 	/* min/max the cpu is capable of */
-- 
2.1.4

[PATCH 3/6] p4-clockmod: replace cpu_sibling_mask() with topology_thread_cpumask()

[Bartosz Golaszewski]

The former duplicates the functionality of the latter but is neither
documented nor arch-independent.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/cpufreq/p4-clockmod.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/cpufreq/p4-clockmod.c b/drivers/cpufreq/p4-clockmod.c
index 529cfd9..a7bc52e 100644
--- a/drivers/cpufreq/p4-clockmod.c
+++ b/drivers/cpufreq/p4-clockmod.c
@@ -172,7 +172,7 @@ static int cpufreq_p4_cpu_init(struct cpufreq_policy *policy)
 	unsigned int i;
 
 #ifdef CONFIG_SMP
-	cpumask_copy(policy->cpus, cpu_sibling_mask(policy->cpu));
+	cpumask_copy(policy->cpus, topology_thread_cpumask(policy->cpu));
 #endif
 
 	/* Errata workaround */
-- 
2.1.4

[PATCH 4/6] acpi-cpufreq: replace cpu_**_mask() with topology_**_cpumask()

[Bartosz Golaszewski]

The former duplicate the functionalities of the latter but are neither
documented nor arch-independent.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/cpufreq/acpi-cpufreq.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
index b0c18ed..09e9ffd 100644
--- a/drivers/cpufreq/acpi-cpufreq.c
+++ b/drivers/cpufreq/acpi-cpufreq.c
@@ -699,13 +699,14 @@ static int acpi_cpufreq_cpu_init(struct cpufreq_policy *policy)
 	dmi_check_system(sw_any_bug_dmi_table);
 	if (bios_with_sw_any_bug && !policy_is_shared(policy)) {
 		policy->shared_type = CPUFREQ_SHARED_TYPE_ALL;
-		cpumask_copy(policy->cpus, cpu_core_mask(cpu));
+		cpumask_copy(policy->cpus, topology_core_cpumask(cpu));
 	}
 
 	if (check_amd_hwpstate_cpu(cpu) && !acpi_pstate_strict) {
 		cpumask_clear(policy->cpus);
 		cpumask_set_cpu(cpu, policy->cpus);
-		cpumask_copy(data->freqdomain_cpus, cpu_sibling_mask(cpu));
+		cpumask_copy(data->freqdomain_cpus,
+			     topology_thread_cpumask(cpu));
 		policy->shared_type = CPUFREQ_SHARED_TYPE_HW;
 		pr_info_once(PFX "overriding BIOS provided _PSD data\n");
 	}
-- 
2.1.4

[PATCH 5/6] x86: replace cpu_**_mask() with topology_**_cpumask()

[Bartosz Golaszewski]

The former duplicate the functionalities of the latter but are neither
documented nor arch-independent.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 arch/x86/kernel/cpu/proc.c |  5 +++--
 arch/x86/kernel/smpboot.c  | 40 ++++++++++++++++++++--------------------
 arch/x86/kernel/tsc_sync.c |  3 +--
 3 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c
index e7d8c76..3bb8de0 100644
--- a/arch/x86/kernel/cpu/proc.c
+++ b/arch/x86/kernel/cpu/proc.c
@@ -1,8 +1,8 @@
-#include <linux/smp.h>
 #include <linux/timex.h>
 #include <linux/string.h>
 #include <linux/seq_file.h>
 #include <linux/cpufreq.h>
+#include <linux/topology.h>
 
 /*
  *	Get CPU information for use by the procfs.
@@ -12,7 +12,8 @@ static void show_cpuinfo_core(struct seq_file *m, struct cpuinfo_x86 *c,
 {
 #ifdef CONFIG_SMP
 	seq_printf(m, "physical id\t: %d\n", c->phys_proc_id);
-	seq_printf(m, "siblings\t: %d\n", cpumask_weight(cpu_core_mask(cpu)));
+	seq_printf(m, "siblings\t: %d\n",
+		   cpumask_weight(topology_core_cpumask(cpu)));
 	seq_printf(m, "core id\t\t: %d\n", c->cpu_core_id);
 	seq_printf(m, "cpu cores\t: %d\n", c->booted_cores);
 	seq_printf(m, "apicid\t\t: %d\n", c->apicid);
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 50e547e..085ff2e 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -42,7 +42,6 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include <linux/init.h>
-#include <linux/smp.h>
 #include <linux/module.h>
 #include <linux/sched.h>
 #include <linux/percpu.h>
@@ -314,10 +313,10 @@ topology_sane(struct cpuinfo_x86 *c, struct cpuinfo_x86 *o, const char *name)
 		cpu1, name, cpu2, cpu_to_node(cpu1), cpu_to_node(cpu2));
 }
 
-#define link_mask(_m, c1, c2)						\
+#define link_mask(mfunc, c1, c2)					\
 do {									\
-	cpumask_set_cpu((c1), cpu_##_m##_mask(c2));			\
-	cpumask_set_cpu((c2), cpu_##_m##_mask(c1));			\
+	cpumask_set_cpu((c1), mfunc(c2));				\
+	cpumask_set_cpu((c2), mfunc(c1));				\
 } while (0)
 
 static bool match_smt(struct cpuinfo_x86 *c, struct cpuinfo_x86 *o)
@@ -398,9 +397,9 @@ void set_cpu_sibling_map(int cpu)
 	cpumask_set_cpu(cpu, cpu_sibling_setup_mask);
 
 	if (!has_mp) {
-		cpumask_set_cpu(cpu, cpu_sibling_mask(cpu));
+		cpumask_set_cpu(cpu, topology_thread_cpumask(cpu));
 		cpumask_set_cpu(cpu, cpu_llc_shared_mask(cpu));
-		cpumask_set_cpu(cpu, cpu_core_mask(cpu));
+		cpumask_set_cpu(cpu, topology_core_cpumask(cpu));
 		c->booted_cores = 1;
 		return;
 	}
@@ -409,10 +408,10 @@ void set_cpu_sibling_map(int cpu)
 		o = &cpu_data(i);
 
 		if ((i == cpu) || (has_smt && match_smt(c, o)))
-			link_mask(sibling, cpu, i);
+			link_mask(topology_thread_cpumask, cpu, i);
 
 		if ((i == cpu) || (has_mp && match_llc(c, o)))
-			link_mask(llc_shared, cpu, i);
+			link_mask(cpu_llc_shared_mask, cpu, i);
 
 	}
 
@@ -424,17 +423,18 @@ void set_cpu_sibling_map(int cpu)
 		o = &cpu_data(i);
 
 		if ((i == cpu) || (has_mp && match_die(c, o))) {
-			link_mask(core, cpu, i);
+			link_mask(topology_core_cpumask, cpu, i);
 
 			/*
 			 *  Does this new cpu bringup a new core?
 			 */
-			if (cpumask_weight(cpu_sibling_mask(cpu)) == 1) {
+			if (cpumask_weight(topology_thread_cpumask(cpu)) == 1) {
 				/*
 				 * for each core in package, increment
 				 * the booted_cores for this new cpu
 				 */
-				if (cpumask_first(cpu_sibling_mask(i)) == i)
+				if (cpumask_first(
+				    topology_thread_cpumask(i)) == i)
 					c->booted_cores++;
 				/*
 				 * increment the core count for all
@@ -1009,8 +1009,8 @@ static __init void disable_smp(void)
 		physid_set_mask_of_physid(boot_cpu_physical_apicid, &phys_cpu_present_map);
 	else
 		physid_set_mask_of_physid(0, &phys_cpu_present_map);
-	cpumask_set_cpu(0, cpu_sibling_mask(0));
-	cpumask_set_cpu(0, cpu_core_mask(0));
+	cpumask_set_cpu(0, topology_thread_cpumask(0));
+	cpumask_set_cpu(0, topology_core_cpumask(0));
 }
 
 enum {
@@ -1293,22 +1293,22 @@ static void remove_siblinginfo(int cpu)
 	int sibling;
 	struct cpuinfo_x86 *c = &cpu_data(cpu);
 
-	for_each_cpu(sibling, cpu_core_mask(cpu)) {
-		cpumask_clear_cpu(cpu, cpu_core_mask(sibling));
+	for_each_cpu(sibling, topology_core_cpumask(cpu)) {
+		cpumask_clear_cpu(cpu, topology_core_cpumask(sibling));
 		/*/
 		 * last thread sibling in this cpu core going down
 		 */
-		if (cpumask_weight(cpu_sibling_mask(cpu)) == 1)
+		if (cpumask_weight(topology_thread_cpumask(cpu)) == 1)
 			cpu_data(sibling).booted_cores--;
 	}
 
-	for_each_cpu(sibling, cpu_sibling_mask(cpu))
-		cpumask_clear_cpu(cpu, cpu_sibling_mask(sibling));
+	for_each_cpu(sibling, topology_thread_cpumask(cpu))
+		cpumask_clear_cpu(cpu, topology_thread_cpumask(sibling));
 	for_each_cpu(sibling, cpu_llc_shared_mask(cpu))
 		cpumask_clear_cpu(cpu, cpu_llc_shared_mask(sibling));
 	cpumask_clear(cpu_llc_shared_mask(cpu));
-	cpumask_clear(cpu_sibling_mask(cpu));
-	cpumask_clear(cpu_core_mask(cpu));
+	cpumask_clear(topology_thread_cpumask(cpu));
+	cpumask_clear(topology_core_cpumask(cpu));
 	c->phys_proc_id = 0;
 	c->cpu_core_id = 0;
 	cpumask_clear_cpu(cpu, cpu_sibling_setup_mask);
diff --git a/arch/x86/kernel/tsc_sync.c b/arch/x86/kernel/tsc_sync.c
index 2648848..1d78586 100644
--- a/arch/x86/kernel/tsc_sync.c
+++ b/arch/x86/kernel/tsc_sync.c
@@ -16,7 +16,6 @@
  */
 #include <linux/spinlock.h>
 #include <linux/kernel.h>
-#include <linux/smp.h>
 #include <linux/nmi.h>
 #include <asm/tsc.h>
 
@@ -113,7 +112,7 @@ static void check_tsc_warp(unsigned int timeout)
  */
 static inline unsigned int loop_timeout(int cpu)
 {
-	return (cpumask_weight(cpu_core_mask(cpu)) > 1) ? 2 : 20;
+	return (cpumask_weight(topology_core_cpumask(cpu)) > 1) ? 2 : 20;
 }
 
 /*
-- 
2.1.4

Re: [PATCH 5/6] x86: replace cpu_**_mask() with topology_**_cpumask()

[Ingo Molnar]

* Bartosz Golaszewski <bgolaszewski@baylibre.com> wrote:

> The former duplicate the functionalities of the latter but are neither
> documented nor arch-independent.

>  	if (!has_mp) {
> -		cpumask_set_cpu(cpu, cpu_sibling_mask(cpu));
> +		cpumask_set_cpu(cpu, topology_thread_cpumask(cpu));
>  		cpumask_set_cpu(cpu, cpu_llc_shared_mask(cpu));

So why does topology.h invent a new name for 'sibling CPUs'?

At least in the scheduling context, 'sibling' is the term we are using 
in most places in the scheduler - try 'git grep sibling kernel/sched/'.

'thread' is a bad name anyway for a CPU, even if we didn't have an 
existing term for it.

So please rename topology_thread_cpumask to topology_sibling_cpumask 
to not replace one inconsistency for another one ...

Thanks,

	Ingo

Re: [PATCH 5/6] x86: replace cpu_**_mask() with topology_**_cpumask()

[Bartosz Golaszewski]

2015-05-02 8:33 GMT+02:00 Ingo Molnar <mingo@kernel.org>:
>
> * Bartosz Golaszewski <bgolaszewski@baylibre.com> wrote:
>
>> The former duplicate the functionalities of the latter but are neither
>> documented nor arch-independent.
>
>>       if (!has_mp) {
>> -             cpumask_set_cpu(cpu, cpu_sibling_mask(cpu));
>> +             cpumask_set_cpu(cpu, topology_thread_cpumask(cpu));
>>               cpumask_set_cpu(cpu, cpu_llc_shared_mask(cpu));
>
> So why does topology.h invent a new name for 'sibling CPUs'?
>
> At least in the scheduling context, 'sibling' is the term we are using
> in most places in the scheduler - try 'git grep sibling kernel/sched/'.
>
> 'thread' is a bad name anyway for a CPU, even if we didn't have an
> existing term for it.

Actually those macros used to be called topology_core_siblings() and
topology_thread_siblings() until commit fbd59a8d1.

> So please rename topology_thread_cpumask to topology_sibling_cpumask
> to not replace one inconsistency for another one ...

I'll prepare a new series.

Best regards,
Bartosz Golaszewski

[PATCH 6/6] x86: remove cpu_sibling_mask() and cpu_core_mask()

[Bartosz Golaszewski]

These functions are arch-specific and duplicate the functionality
of macros defined in linux/include/topology.h.

Remove them as all the callers in x86 have now switched to using
the topology_**_cpumask() family.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 arch/x86/include/asm/smp.h | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
index 17a8dce..222a6a3 100644
--- a/arch/x86/include/asm/smp.h
+++ b/arch/x86/include/asm/smp.h
@@ -37,16 +37,6 @@ DECLARE_PER_CPU_READ_MOSTLY(cpumask_var_t, cpu_llc_shared_map);
 DECLARE_PER_CPU_READ_MOSTLY(u16, cpu_llc_id);
 DECLARE_PER_CPU_READ_MOSTLY(int, cpu_number);
 
-static inline struct cpumask *cpu_sibling_mask(int cpu)
-{
-	return per_cpu(cpu_sibling_map, cpu);
-}
-
-static inline struct cpumask *cpu_core_mask(int cpu)
-{
-	return per_cpu(cpu_core_map, cpu);
-}
-
 static inline struct cpumask *cpu_llc_shared_mask(int cpu)
 {
 	return per_cpu(cpu_llc_shared_map, cpu);
-- 
2.1.4