2008-07-23 17:19:10

by Mike Travis

[permalink] [raw]
Subject: [PATCH 1/1] cpumask: Change cpumask_of_cpu to use cpumask_of_cpu_map

* The previous "lvalue replacement for cpumask_of_cpu()" was not usable
in certain situations and generally added unneeded complexity. So
this patch replaces the cpumask_of_cpu_ptr* macros with a generic
cpumask_of_cpu_map[]. The only config option is whether this is a
static map, or allocated at boot up time:

#ifdef CONFIG_HAVE_CPUMASK_OF_CPU_MAP_PTR
/* architecture initializes *cpumask_of_cpu_map */
#else
/* start_kernel() initializes cpumask_of_cpu_map[] */

#endif

The reason for two is in the case where NR_CPUS is a very large
value it's preferred to allocate the map sized on the actual
number of possible cpus, and not NR_CPUS.

Note that the declaration of cpumask_of_cpu_map[] is initialized
so that cpumask_of_cpu(0) is defined early. This assumes that
cpu 0 is the boot cpu.

Based on linux-2.6.tip/master at the following commit:

commit a099a9b18ab434594bb01ed920e8c58574203fa9
Merge: 9b4ae4d... f3b51d7...
Author: Ingo Molnar <[email protected]>
Date: Tue Jul 22 15:43:36 2008 +0200

Merge branch 'out-of-tree'

Signed-off-by: Mike Travis <[email protected]>

* ACPI
Cc: Len Brown <[email protected]>

* ARM (???)
Cc: Lennert Buytenhek <[email protected]>

* CPUFREQ
Cc: Dave Jones <[email protected]>

* CPUMASK
Cc: Paul Jackson <[email protected]>

* IA64 (Itanium) PLATFORM
Cc: Tony Luck <[email protected]>

* MICROCODE
Cc: Tigran Aivazian <[email protected]>

* POWERPC (32-BIT AND 64-BIT)
Cc: Paul Mackerras <[email protected]>
Cc: Benjamin Herrenschmidt <[email protected]>

* PROFILE
Cc: Robert Richter <[email protected]>

# S390
Cc: Martin Schwidefsky <[email protected]>
Cc: Heiko Carstens <[email protected]>

* SUN3/3X
Cc: Sam Creasey <[email protected]>

* SUNRPC
Cc: Greg Banks <[email protected]>

* Various
Cc: Eric W. Biederman <[email protected]>
Cc: Adrian Bunk <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Andreas Schwab <[email protected]>
Cc: Johannes Weiner <[email protected]>

---
Note: 2 false WARNINGS from checkpatch.pl
---
arch/x86/Kconfig | 2 -
arch/x86/configs/i386_defconfig | 2 -
arch/x86/configs/x86_64_defconfig | 2 -
arch/x86/kernel/acpi/cstate.c | 3 -
arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c | 10 +-----
arch/x86/kernel/cpu/cpufreq/powernow-k8.c | 15 +++------
arch/x86/kernel/cpu/cpufreq/speedstep-centrino.c | 12 ++-----
arch/x86/kernel/cpu/cpufreq/speedstep-ich.c | 3 -
arch/x86/kernel/cpu/intel_cacheinfo.c | 3 -
arch/x86/kernel/ldt.c | 6 +--
arch/x86/kernel/microcode.c | 17 +++-------
arch/x86/kernel/reboot.c | 11 +-----
arch/x86/kernel/setup_percpu.c | 11 +++++-
drivers/acpi/processor_throttling.c | 11 +-----
drivers/firmware/dcdbas.c | 3 -
drivers/misc/sgi-xp/xpc_main.c | 3 -
include/linux/cpumask.h | 38 +++++++----------------
init/main.c | 28 +++++++++++++++-
kernel/stop_machine.c | 3 -
kernel/time/tick-common.c | 8 +---
kernel/trace/trace_sysprof.c | 4 --
lib/smp_processor_id.c | 5 ---
net/sunrpc/svc.c | 3 -
23 files changed, 87 insertions(+), 116 deletions(-)

--- linux-2.6.tip.orig/arch/x86/Kconfig
+++ linux-2.6.tip/arch/x86/Kconfig
@@ -126,7 +126,7 @@ config ARCH_HAS_CACHE_LINE_SIZE
config HAVE_SETUP_PER_CPU_AREA
def_bool X86_64_SMP || (X86_SMP && !X86_VOYAGER)

-config HAVE_CPUMASK_OF_CPU_MAP
+config HAVE_CPUMASK_OF_CPU_MAP_PTR
def_bool X86_64_SMP

config ARCH_HIBERNATION_POSSIBLE
--- linux-2.6.tip.orig/arch/x86/configs/i386_defconfig
+++ linux-2.6.tip/arch/x86/configs/i386_defconfig
@@ -36,7 +36,7 @@ CONFIG_GENERIC_CALIBRATE_DELAY=y
CONFIG_ARCH_HAS_CPU_RELAX=y
CONFIG_ARCH_HAS_CACHE_LINE_SIZE=y
CONFIG_HAVE_SETUP_PER_CPU_AREA=y
-# CONFIG_HAVE_CPUMASK_OF_CPU_MAP is not set
+# CONFIG_HAVE_CPUMASK_OF_CPU_MAP_PTR is not set
CONFIG_ARCH_HIBERNATION_POSSIBLE=y
CONFIG_ARCH_SUSPEND_POSSIBLE=y
# CONFIG_ZONE_DMA32 is not set
--- linux-2.6.tip.orig/arch/x86/configs/x86_64_defconfig
+++ linux-2.6.tip/arch/x86/configs/x86_64_defconfig
@@ -36,7 +36,7 @@ CONFIG_GENERIC_TIME_VSYSCALL=y
CONFIG_ARCH_HAS_CPU_RELAX=y
CONFIG_ARCH_HAS_CACHE_LINE_SIZE=y
CONFIG_HAVE_SETUP_PER_CPU_AREA=y
-CONFIG_HAVE_CPUMASK_OF_CPU_MAP=y
+CONFIG_HAVE_CPUMASK_OF_CPU_MAP_PTR=y
CONFIG_ARCH_HIBERNATION_POSSIBLE=y
CONFIG_ARCH_SUSPEND_POSSIBLE=y
CONFIG_ZONE_DMA32=y
--- linux-2.6.tip.orig/arch/x86/kernel/acpi/cstate.c
+++ linux-2.6.tip/arch/x86/kernel/acpi/cstate.c
@@ -73,7 +73,6 @@ int acpi_processor_ffh_cstate_probe(unsi
struct cpuinfo_x86 *c = &cpu_data(cpu);

cpumask_t saved_mask;
- cpumask_of_cpu_ptr(new_mask, cpu);
int retval;
unsigned int eax, ebx, ecx, edx;
unsigned int edx_part;
@@ -92,7 +91,7 @@ int acpi_processor_ffh_cstate_probe(unsi

/* Make sure we are running on right CPU */
saved_mask = current->cpus_allowed;
- retval = set_cpus_allowed_ptr(current, new_mask);
+ retval = set_cpus_allowed_ptr(current, &cpumask_of_cpu(cpu));
if (retval)
return -1;

--- linux-2.6.tip.orig/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
+++ linux-2.6.tip/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
@@ -200,12 +200,10 @@ static void drv_read(struct drv_cmd *cmd
static void drv_write(struct drv_cmd *cmd)
{
cpumask_t saved_mask = current->cpus_allowed;
- cpumask_of_cpu_ptr_declare(cpu_mask);
unsigned int i;

for_each_cpu_mask_nr(i, cmd->mask) {
- cpumask_of_cpu_ptr_next(cpu_mask, i);
- set_cpus_allowed_ptr(current, cpu_mask);
+ set_cpus_allowed_ptr(current, &cpumask_of_cpu(i));
do_drv_write(cmd);
}

@@ -269,12 +267,11 @@ static unsigned int get_measured_perf(un
} aperf_cur, mperf_cur;

cpumask_t saved_mask;
- cpumask_of_cpu_ptr(cpu_mask, cpu);
unsigned int perf_percent;
unsigned int retval;

saved_mask = current->cpus_allowed;
- set_cpus_allowed_ptr(current, cpu_mask);
+ set_cpus_allowed_ptr(current, &cpumask_of_cpu(cpu));
if (get_cpu() != cpu) {
/* We were not able to run on requested processor */
put_cpu();
@@ -340,7 +337,6 @@ static unsigned int get_measured_perf(un

static unsigned int get_cur_freq_on_cpu(unsigned int cpu)
{
- cpumask_of_cpu_ptr(cpu_mask, cpu);
struct acpi_cpufreq_data *data = per_cpu(drv_data, cpu);
unsigned int freq;
unsigned int cached_freq;
@@ -353,7 +349,7 @@ static unsigned int get_cur_freq_on_cpu(
}

cached_freq = data->freq_table[data->acpi_data->state].frequency;
- freq = extract_freq(get_cur_val(cpu_mask), data);
+ freq = extract_freq(get_cur_val(&cpumask_of_cpu(cpu)), data);
if (freq != cached_freq) {
/*
* The dreaded BIOS frequency change behind our back.
--- linux-2.6.tip.orig/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
+++ linux-2.6.tip/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
@@ -479,12 +479,11 @@ static int core_voltage_post_transition(
static int check_supported_cpu(unsigned int cpu)
{
cpumask_t oldmask;
- cpumask_of_cpu_ptr(cpu_mask, cpu);
u32 eax, ebx, ecx, edx;
unsigned int rc = 0;

oldmask = current->cpus_allowed;
- set_cpus_allowed_ptr(current, cpu_mask);
+ set_cpus_allowed_ptr(current, &cpumask_of_cpu(cpu));

if (smp_processor_id() != cpu) {
printk(KERN_ERR PFX "limiting to cpu %u failed\n", cpu);
@@ -1017,7 +1016,6 @@ static int transition_frequency_pstate(s
static int powernowk8_target(struct cpufreq_policy *pol, unsigned targfreq, unsigned relation)
{
cpumask_t oldmask;
- cpumask_of_cpu_ptr(cpu_mask, pol->cpu);
struct powernow_k8_data *data = per_cpu(powernow_data, pol->cpu);
u32 checkfid;
u32 checkvid;
@@ -1032,7 +1030,7 @@ static int powernowk8_target(struct cpuf

/* only run on specific CPU from here on */
oldmask = current->cpus_allowed;
- set_cpus_allowed_ptr(current, cpu_mask);
+ set_cpus_allowed_ptr(current, &cpumask_of_cpu(pol->cpu));

if (smp_processor_id() != pol->cpu) {
printk(KERN_ERR PFX "limiting to cpu %u failed\n", pol->cpu);
@@ -1107,7 +1105,6 @@ static int __cpuinit powernowk8_cpu_init
{
struct powernow_k8_data *data;
cpumask_t oldmask;
- cpumask_of_cpu_ptr_declare(newmask);
int rc;

if (!cpu_online(pol->cpu))
@@ -1159,8 +1156,7 @@ static int __cpuinit powernowk8_cpu_init

/* only run on specific CPU from here on */
oldmask = current->cpus_allowed;
- cpumask_of_cpu_ptr_next(newmask, pol->cpu);
- set_cpus_allowed_ptr(current, newmask);
+ set_cpus_allowed_ptr(current, &cpumask_of_cpu(pol->cpu));

if (smp_processor_id() != pol->cpu) {
printk(KERN_ERR PFX "limiting to cpu %u failed\n", pol->cpu);
@@ -1182,7 +1178,7 @@ static int __cpuinit powernowk8_cpu_init
set_cpus_allowed_ptr(current, &oldmask);

if (cpu_family == CPU_HW_PSTATE)
- pol->cpus = *newmask;
+ pol->cpus = cpumask_of_cpu(pol->cpu);
else
pol->cpus = per_cpu(cpu_core_map, pol->cpu);
data->available_cores = &(pol->cpus);
@@ -1248,7 +1244,6 @@ static unsigned int powernowk8_get (unsi
{
struct powernow_k8_data *data;
cpumask_t oldmask = current->cpus_allowed;
- cpumask_of_cpu_ptr(newmask, cpu);
unsigned int khz = 0;
unsigned int first;

@@ -1258,7 +1253,7 @@ static unsigned int powernowk8_get (unsi
if (!data)
return -EINVAL;

- set_cpus_allowed_ptr(current, newmask);
+ set_cpus_allowed_ptr(current, &cpumask_of_cpu(cpu));
if (smp_processor_id() != cpu) {
printk(KERN_ERR PFX
"limiting to CPU %d failed in powernowk8_get\n", cpu);
--- linux-2.6.tip.orig/arch/x86/kernel/cpu/cpufreq/speedstep-centrino.c
+++ linux-2.6.tip/arch/x86/kernel/cpu/cpufreq/speedstep-centrino.c
@@ -417,10 +417,9 @@ static unsigned int get_cur_freq(unsigne
unsigned l, h;
unsigned clock_freq;
cpumask_t saved_mask;
- cpumask_of_cpu_ptr(new_mask, cpu);

saved_mask = current->cpus_allowed;
- set_cpus_allowed_ptr(current, new_mask);
+ set_cpus_allowed_ptr(current, &cpumask_of_cpu(cpu));
if (smp_processor_id() != cpu)
return 0;

@@ -678,15 +677,12 @@ static int centrino_target (struct cpufr
* Best effort undo..
*/

- if (!cpus_empty(*covered_cpus)) {
- cpumask_of_cpu_ptr_declare(new_mask);
-
+ if (!cpus_empty(*covered_cpus))
for_each_cpu_mask_nr(j, *covered_cpus) {
- cpumask_of_cpu_ptr_next(new_mask, j);
- set_cpus_allowed_ptr(current, new_mask);
+ set_cpus_allowed_ptr(current,
+ &cpumask_of_cpu(j));
wrmsr(MSR_IA32_PERF_CTL, oldmsr, h);
}
- }

tmp = freqs.new;
freqs.new = freqs.old;
--- linux-2.6.tip.orig/arch/x86/kernel/cpu/cpufreq/speedstep-ich.c
+++ linux-2.6.tip/arch/x86/kernel/cpu/cpufreq/speedstep-ich.c
@@ -244,8 +244,7 @@ static unsigned int _speedstep_get(const

static unsigned int speedstep_get(unsigned int cpu)
{
- cpumask_of_cpu_ptr(newmask, cpu);
- return _speedstep_get(newmask);
+ return _speedstep_get(&cpumask_of_cpu(cpu));
}

/**
--- linux-2.6.tip.orig/arch/x86/kernel/cpu/intel_cacheinfo.c
+++ linux-2.6.tip/arch/x86/kernel/cpu/intel_cacheinfo.c
@@ -516,7 +516,6 @@ static int __cpuinit detect_cache_attrib
unsigned long j;
int retval;
cpumask_t oldmask;
- cpumask_of_cpu_ptr(newmask, cpu);

if (num_cache_leaves == 0)
return -ENOENT;
@@ -527,7 +526,7 @@ static int __cpuinit detect_cache_attrib
return -ENOMEM;

oldmask = current->cpus_allowed;
- retval = set_cpus_allowed_ptr(current, newmask);
+ retval = set_cpus_allowed_ptr(current, &cpumask_of_cpu(cpu));
if (retval)
goto out;

--- linux-2.6.tip.orig/arch/x86/kernel/ldt.c
+++ linux-2.6.tip/arch/x86/kernel/ldt.c
@@ -63,12 +63,10 @@ static int alloc_ldt(mm_context_t *pc, i

if (reload) {
#ifdef CONFIG_SMP
- cpumask_of_cpu_ptr_declare(mask);
-
preempt_disable();
load_LDT(pc);
- cpumask_of_cpu_ptr_next(mask, smp_processor_id());
- if (!cpus_equal(current->mm->cpu_vm_mask, *mask))
+ if (!cpus_equal(current->mm->cpu_vm_mask,
+ cpumask_of_cpu(smp_processor_id())))
smp_call_function(flush_ldt, current->mm, 1);
preempt_enable();
#else
--- linux-2.6.tip.orig/arch/x86/kernel/microcode.c
+++ linux-2.6.tip/arch/x86/kernel/microcode.c
@@ -388,7 +388,6 @@ static int do_microcode_update (void)
void *new_mc = NULL;
int cpu;
cpumask_t old;
- cpumask_of_cpu_ptr_declare(newmask);

old = current->cpus_allowed;

@@ -405,8 +404,7 @@ static int do_microcode_update (void)

if (!uci->valid)
continue;
- cpumask_of_cpu_ptr_next(newmask, cpu);
- set_cpus_allowed_ptr(current, newmask);
+ set_cpus_allowed_ptr(current, &cpumask_of_cpu(cpu));
error = get_maching_microcode(new_mc, cpu);
if (error < 0)
goto out;
@@ -576,7 +574,6 @@ static int apply_microcode_check_cpu(int
struct cpuinfo_x86 *c = &cpu_data(cpu);
struct ucode_cpu_info *uci = ucode_cpu_info + cpu;
cpumask_t old;
- cpumask_of_cpu_ptr(newmask, cpu);
unsigned int val[2];
int err = 0;

@@ -585,7 +582,7 @@ static int apply_microcode_check_cpu(int
return 0;

old = current->cpus_allowed;
- set_cpus_allowed_ptr(current, newmask);
+ set_cpus_allowed_ptr(current, &cpumask_of_cpu(cpu));

/* Check if the microcode we have in memory matches the CPU */
if (c->x86_vendor != X86_VENDOR_INTEL || c->x86 < 6 ||
@@ -623,12 +620,11 @@ static int apply_microcode_check_cpu(int
static void microcode_init_cpu(int cpu, int resume)
{
cpumask_t old;
- cpumask_of_cpu_ptr(newmask, cpu);
struct ucode_cpu_info *uci = ucode_cpu_info + cpu;

old = current->cpus_allowed;

- set_cpus_allowed_ptr(current, newmask);
+ set_cpus_allowed_ptr(current, &cpumask_of_cpu(cpu));
mutex_lock(&microcode_mutex);
collect_cpu_info(cpu);
if (uci->valid && system_state == SYSTEM_RUNNING && !resume)
@@ -659,13 +655,10 @@ static ssize_t reload_store(struct sys_d
if (end == buf)
return -EINVAL;
if (val == 1) {
- cpumask_t old;
- cpumask_of_cpu_ptr(newmask, cpu);
-
- old = current->cpus_allowed;
+ cpumask_t old = current->cpus_allowed;

get_online_cpus();
- set_cpus_allowed_ptr(current, newmask);
+ set_cpus_allowed_ptr(current, &cpumask_of_cpu(cpu));

mutex_lock(&microcode_mutex);
if (uci->valid)
--- linux-2.6.tip.orig/arch/x86/kernel/reboot.c
+++ linux-2.6.tip/arch/x86/kernel/reboot.c
@@ -414,25 +414,20 @@ void native_machine_shutdown(void)

/* The boot cpu is always logical cpu 0 */
int reboot_cpu_id = 0;
- cpumask_of_cpu_ptr(newmask, reboot_cpu_id);

#ifdef CONFIG_X86_32
/* See if there has been given a command line override */
if ((reboot_cpu != -1) && (reboot_cpu < NR_CPUS) &&
- cpu_online(reboot_cpu)) {
+ cpu_online(reboot_cpu))
reboot_cpu_id = reboot_cpu;
- cpumask_of_cpu_ptr_next(newmask, reboot_cpu_id);
- }
#endif

/* Make certain the cpu I'm about to reboot on is online */
- if (!cpu_online(reboot_cpu_id)) {
+ if (!cpu_online(reboot_cpu_id))
reboot_cpu_id = smp_processor_id();
- cpumask_of_cpu_ptr_next(newmask, reboot_cpu_id);
- }

/* Make certain I only run on the appropriate processor */
- set_cpus_allowed_ptr(current, newmask);
+ set_cpus_allowed_ptr(current, &cpumask_of_cpu(reboot_cpu_id));

/* O.K Now that I'm on the appropriate processor,
* stop all of the others.
--- linux-2.6.tip.orig/arch/x86/kernel/setup_percpu.c
+++ linux-2.6.tip/arch/x86/kernel/setup_percpu.c
@@ -80,8 +80,15 @@ static void __init setup_per_cpu_maps(vo
#endif
}

-#ifdef CONFIG_HAVE_CPUMASK_OF_CPU_MAP
-cpumask_t *cpumask_of_cpu_map __read_mostly;
+#ifdef CONFIG_HAVE_CPUMASK_OF_CPU_MAP_PTR
+/*
+ * Configure an initial cpumask_of_cpu(0) for early users
+ */
+static cpumask_t initial_cpumask_of_cpu_map __initdata = (cpumask_t) { {
+ [BITS_TO_LONGS(NR_CPUS)-1] = 1
+} };
+cpumask_t *cpumask_of_cpu_map __read_mostly =
+ (cpumask_t *)&initial_cpumask_of_cpu_map;
EXPORT_SYMBOL(cpumask_of_cpu_map);

/* requires nr_cpu_ids to be initialized */
--- linux-2.6.tip.orig/drivers/acpi/processor_throttling.c
+++ linux-2.6.tip/drivers/acpi/processor_throttling.c
@@ -827,7 +827,6 @@ static int acpi_processor_get_throttling
static int acpi_processor_get_throttling(struct acpi_processor *pr)
{
cpumask_t saved_mask;
- cpumask_of_cpu_ptr_declare(new_mask);
int ret;

if (!pr)
@@ -839,8 +838,7 @@ static int acpi_processor_get_throttling
* Migrate task to the cpu pointed by pr.
*/
saved_mask = current->cpus_allowed;
- cpumask_of_cpu_ptr_next(new_mask, pr->id);
- set_cpus_allowed_ptr(current, new_mask);
+ set_cpus_allowed_ptr(current, &cpumask_of_cpu(pr->id));
ret = pr->throttling.acpi_processor_get_throttling(pr);
/* restore the previous state */
set_cpus_allowed_ptr(current, &saved_mask);
@@ -989,7 +987,6 @@ static int acpi_processor_set_throttling
int acpi_processor_set_throttling(struct acpi_processor *pr, int state)
{
cpumask_t saved_mask;
- cpumask_of_cpu_ptr_declare(new_mask);
int ret = 0;
unsigned int i;
struct acpi_processor *match_pr;
@@ -1028,8 +1025,7 @@ int acpi_processor_set_throttling(struct
* it can be called only for the cpu pointed by pr.
*/
if (p_throttling->shared_type == DOMAIN_COORD_TYPE_SW_ANY) {
- cpumask_of_cpu_ptr_next(new_mask, pr->id);
- set_cpus_allowed_ptr(current, new_mask);
+ set_cpus_allowed_ptr(current, &cpumask_of_cpu(pr->id));
ret = p_throttling->acpi_processor_set_throttling(pr,
t_state.target_state);
} else {
@@ -1060,8 +1056,7 @@ int acpi_processor_set_throttling(struct
continue;
}
t_state.cpu = i;
- cpumask_of_cpu_ptr_next(new_mask, i);
- set_cpus_allowed_ptr(current, new_mask);
+ set_cpus_allowed_ptr(current, &cpumask_of_cpu(i));
ret = match_pr->throttling.
acpi_processor_set_throttling(
match_pr, t_state.target_state);
--- linux-2.6.tip.orig/drivers/firmware/dcdbas.c
+++ linux-2.6.tip/drivers/firmware/dcdbas.c
@@ -254,7 +254,6 @@ static ssize_t host_control_on_shutdown_
static int smi_request(struct smi_cmd *smi_cmd)
{
cpumask_t old_mask;
- cpumask_of_cpu_ptr(new_mask, 0);
int ret = 0;

if (smi_cmd->magic != SMI_CMD_MAGIC) {
@@ -265,7 +264,7 @@ static int smi_request(struct smi_cmd *s

/* SMI requires CPU 0 */
old_mask = current->cpus_allowed;
- set_cpus_allowed_ptr(current, new_mask);
+ set_cpus_allowed_ptr(current, &cpumask_of_cpu(0));
if (smp_processor_id() != 0) {
dev_dbg(&dcdbas_pdev->dev, "%s: failed to get CPU 0\n",
__func__);
--- linux-2.6.tip.orig/drivers/misc/sgi-xp/xpc_main.c
+++ linux-2.6.tip/drivers/misc/sgi-xp/xpc_main.c
@@ -229,11 +229,10 @@ xpc_hb_checker(void *ignore)
int last_IRQ_count = 0;
int new_IRQ_count;
int force_IRQ = 0;
- cpumask_of_cpu_ptr(cpumask, XPC_HB_CHECK_CPU);

/* this thread was marked active by xpc_hb_init() */

- set_cpus_allowed_ptr(current, cpumask);
+ set_cpus_allowed_ptr(current, &cpumask_of_cpu(XPC_HB_CHECK_CPU));

/* set our heartbeating to other partitions into motion */
xpc_hb_check_timeout = jiffies + (xpc_hb_check_interval * HZ);
--- linux-2.6.tip.orig/include/linux/cpumask.h
+++ linux-2.6.tip/include/linux/cpumask.h
@@ -62,15 +62,7 @@
* int next_cpu_nr(cpu, mask) Next cpu past 'cpu', or nr_cpu_ids
*
* cpumask_t cpumask_of_cpu(cpu) Return cpumask with bit 'cpu' set
- *ifdef CONFIG_HAS_CPUMASK_OF_CPU
- * cpumask_of_cpu_ptr_declare(v) Declares cpumask_t *v
- * cpumask_of_cpu_ptr_next(v, cpu) Sets v = &cpumask_of_cpu_map[cpu]
- * cpumask_of_cpu_ptr(v, cpu) Combines above two operations
- *else
- * cpumask_of_cpu_ptr_declare(v) Declares cpumask_t _v and *v = &_v
- * cpumask_of_cpu_ptr_next(v, cpu) Sets _v = cpumask_of_cpu(cpu)
- * cpumask_of_cpu_ptr(v, cpu) Combines above two operations
- *endif
+ * (can be used as an lvalue)
* CPU_MASK_ALL Initializer - all bits set
* CPU_MASK_NONE Initializer - no bits set
* unsigned long *cpus_addr(mask) Array of unsigned long's in mask
@@ -273,17 +265,17 @@ static inline void __cpus_shift_left(cpu
bitmap_shift_left(dstp->bits, srcp->bits, n, nbits);
}

+#ifdef CONFIG_SMP
+/* lvalue version of cpumask_of_cpu() */
+#define cpumask_of_cpu(cpu) (cpumask_of_cpu_map[cpu])

-#ifdef CONFIG_HAVE_CPUMASK_OF_CPU_MAP
+#ifdef CONFIG_HAVE_CPUMASK_OF_CPU_MAP_PTR
extern cpumask_t *cpumask_of_cpu_map;
-#define cpumask_of_cpu(cpu) (cpumask_of_cpu_map[cpu])
-#define cpumask_of_cpu_ptr(v, cpu) \
- const cpumask_t *v = &cpumask_of_cpu(cpu)
-#define cpumask_of_cpu_ptr_declare(v) \
- const cpumask_t *v
-#define cpumask_of_cpu_ptr_next(v, cpu) \
- v = &cpumask_of_cpu(cpu)
#else
+extern cpumask_t cpumask_of_cpu_map[NR_CPUS];
+#endif
+
+#if 0 /* previous (non-lvalue) version */
#define cpumask_of_cpu(cpu) \
({ \
typeof(_unused_cpumask_arg_) m; \
@@ -295,14 +287,10 @@ extern cpumask_t *cpumask_of_cpu_map;
} \
m; \
})
-#define cpumask_of_cpu_ptr(v, cpu) \
- cpumask_t _##v = cpumask_of_cpu(cpu); \
- const cpumask_t *v = &_##v
-#define cpumask_of_cpu_ptr_declare(v) \
- cpumask_t _##v; \
- const cpumask_t *v = &_##v
-#define cpumask_of_cpu_ptr_next(v, cpu) \
- _##v = cpumask_of_cpu(cpu)
+#endif
+
+#else /* !CONFIG_SMP */
+#define cpumask_of_cpu(cpu) (cpumask_t) { { [0] = 1 } }
#endif

#define CPU_MASK_LAST_WORD BITMAP_LAST_WORD_MASK(NR_CPUS)
--- linux-2.6.tip.orig/init/main.c
+++ linux-2.6.tip/init/main.c
@@ -366,8 +366,9 @@ static void __init smp_init(void)
static inline void setup_per_cpu_areas(void) { }
static inline void setup_nr_cpu_ids(void) { }
static inline void smp_prepare_cpus(unsigned int maxcpus) { }
+static inline void setup_cpumask_of_cpu(void) { }

-#else
+#else /* CONFIG_SMP */

#if NR_CPUS > BITS_PER_LONG
cpumask_t cpu_mask_all __read_mostly = CPU_MASK_ALL;
@@ -412,6 +413,27 @@ static void __init setup_per_cpu_areas(v
}
#endif /* CONFIG_HAVE_SETUP_PER_CPU_AREA */

+#ifndef CONFIG_HAVE_CPUMASK_OF_CPU_MAP_PTR
+/*
+ * Define map for cpumask_of_cpu() macro with cpumask_of_cpu(0) initialized,
+ * in case it's referenced before setup_cpumask_of_cpu() is called.
+ */
+cpumask_t cpumask_of_cpu_map[NR_CPUS] __read_mostly = { {
+ [BITS_TO_LONGS(NR_CPUS)-1] = 1
+} };
+EXPORT_SYMBOL(cpumask_of_cpu_map);
+
+static void __init setup_cpumask_of_cpu(void)
+{
+ int cpu;
+
+ for_each_possible_cpu(cpu)
+ cpu_set(cpu, cpumask_of_cpu_map[cpu]);
+}
+#else
+static inline void setup_cpumask_of_cpu(void) { }
+#endif
+
/* Called by boot processor to activate the rest. */
static void __init smp_init(void)
{
@@ -436,8 +458,7 @@ static void __init smp_init(void)
printk(KERN_INFO "Brought up %ld CPUs\n", (long)num_online_cpus());
smp_cpus_done(setup_max_cpus);
}
-
-#endif
+#endif /* CONFIG_SMP */

/*
* We need to store the untouched command line for future reference.
@@ -583,6 +604,7 @@ asmlinkage void __init start_kernel(void
unwind_setup();
setup_per_cpu_areas();
setup_nr_cpu_ids();
+ setup_cpumask_of_cpu();
smp_prepare_boot_cpu(); /* arch-specific boot-cpu hooks */

/*
--- linux-2.6.tip.orig/kernel/stop_machine.c
+++ linux-2.6.tip/kernel/stop_machine.c
@@ -33,9 +33,8 @@ static int stopmachine(void *cpu)
{
int irqs_disabled = 0;
int prepared = 0;
- cpumask_of_cpu_ptr(cpumask, (int)(long)cpu);

- set_cpus_allowed_ptr(current, cpumask);
+ set_cpus_allowed_ptr(current, &cpumask_of_cpu((int)(long)cpu));

/* Ack: we are alive */
smp_mb(); /* Theoretically the ack = 0 might not be on this CPU yet. */
--- linux-2.6.tip.orig/kernel/time/tick-common.c
+++ linux-2.6.tip/kernel/time/tick-common.c
@@ -196,12 +196,10 @@ static int tick_check_new_device(struct
struct tick_device *td;
int cpu, ret = NOTIFY_OK;
unsigned long flags;
- cpumask_of_cpu_ptr_declare(cpumask);

spin_lock_irqsave(&tick_device_lock, flags);

cpu = smp_processor_id();
- cpumask_of_cpu_ptr_next(cpumask, cpu);
if (!cpu_isset(cpu, newdev->cpumask))
goto out_bc;

@@ -209,7 +207,7 @@ static int tick_check_new_device(struct
curdev = td->evtdev;

/* cpu local device ? */
- if (!cpus_equal(newdev->cpumask, *cpumask)) {
+ if (!cpus_equal(newdev->cpumask, cpumask_of_cpu(cpu))) {

/*
* If the cpu affinity of the device interrupt can not
@@ -222,7 +220,7 @@ static int tick_check_new_device(struct
* If we have a cpu local device already, do not replace it
* by a non cpu local device
*/
- if (curdev && cpus_equal(curdev->cpumask, *cpumask))
+ if (curdev && cpus_equal(curdev->cpumask, cpumask_of_cpu(cpu)))
goto out_bc;
}

@@ -254,7 +252,7 @@ static int tick_check_new_device(struct
curdev = NULL;
}
clockevents_exchange_device(curdev, newdev);
- tick_setup_device(td, newdev, cpu, cpumask);
+ tick_setup_device(td, newdev, cpu, &cpumask_of_cpu(cpu));
if (newdev->features & CLOCK_EVT_FEAT_ONESHOT)
tick_oneshot_notify();

--- linux-2.6.tip.orig/kernel/trace/trace_sysprof.c
+++ linux-2.6.tip/kernel/trace/trace_sysprof.c
@@ -213,9 +213,7 @@ static void start_stack_timers(void)
int cpu;

for_each_online_cpu(cpu) {
- cpumask_of_cpu_ptr(new_mask, cpu);
-
- set_cpus_allowed_ptr(current, new_mask);
+ set_cpus_allowed_ptr(current, &cpumask_of_cpu(cpu));
start_stack_timer(cpu);
}
set_cpus_allowed_ptr(current, &saved_mask);
--- linux-2.6.tip.orig/lib/smp_processor_id.c
+++ linux-2.6.tip/lib/smp_processor_id.c
@@ -11,7 +11,6 @@ notrace unsigned int debug_smp_processor
{
unsigned long preempt_count = preempt_count();
int this_cpu = raw_smp_processor_id();
- cpumask_of_cpu_ptr_declare(this_mask);

if (likely(preempt_count))
goto out;
@@ -23,9 +22,7 @@ notrace unsigned int debug_smp_processor
* Kernel threads bound to a single CPU can safely use
* smp_processor_id():
*/
- cpumask_of_cpu_ptr_next(this_mask, this_cpu);
-
- if (cpus_equal(current->cpus_allowed, *this_mask))
+ if (cpus_equal(current->cpus_allowed, cpumask_of_cpu(this_cpu)))
goto out;

/*
--- linux-2.6.tip.orig/net/sunrpc/svc.c
+++ linux-2.6.tip/net/sunrpc/svc.c
@@ -310,8 +310,7 @@ svc_pool_map_set_cpumask(struct task_str
switch (m->mode) {
case SVC_POOL_PERCPU:
{
- cpumask_of_cpu_ptr(cpumask, node);
- set_cpus_allowed_ptr(task, cpumask);
+ set_cpus_allowed_ptr(task, &cpumask_of_cpu(node));
break;
}
case SVC_POOL_PERNODE:

--


2008-07-24 03:45:51

by Rusty Russell

[permalink] [raw]
Subject: Re: [PATCH 1/1] cpumask: Change cpumask_of_cpu to use cpumask_of_cpu_map

On Thursday 24 July 2008 03:18:42 Mike Travis wrote:
> Note that the declaration of cpumask_of_cpu_map[] is initialized
> so that cpumask_of_cpu(0) is defined early. This assumes that
> cpu 0 is the boot cpu.

Hi Mike,

Make this statically initialized please. That almost guarantees there'll
be no problems. It's a little tricky to do, but possible. Patch below
tested on 32 bit x86 only.

> Based on linux-2.6.tip/master at the following commit:
>
> commit a099a9b18ab434594bb01ed920e8c58574203fa9
> Merge: 9b4ae4d... f3b51d7...
> Author: Ingo Molnar <[email protected]>
> Date: Tue Jul 22 15:43:36 2008 +0200
>
> Merge branch 'out-of-tree'

This is two-steps forward one step back. Have Ingo drop the original
patches, and roll together into a single change. That should be much
clearer. I know it's a PITA.

Thanks,
Rusty.

Make cpumask_of_cpu_map generic

If an arch doesn't define cpumask_of_cpu_map, create a generic
statically-initialized one for them. This allows removal of the buggy
cpumask_of_cpu() macro (&cpumask_of_cpu() gives address of
out-of-scope var).

An arch with NR_CPUS of 4096 probably wants to allocate this itself
based on the actual number of CPUs, since otherwise they're using 2MB
of rodata (1024 cpus means 128k). That's what
CONFIG_HAVE_CPUMASK_OF_CPU_MAP is for (only x86/64 does so at the
moment).

In future as we support more CPUs, we'll need to resort to a
get_cpu_map()/put_cpu_map() allocation scheme.

Signed-off-by: Rusty Russell <[email protected]>

diff -r 95e128369c3c include/linux/cpumask.h
--- a/include/linux/cpumask.h Thu Jul 24 11:16:27 2008 +1000
+++ b/include/linux/cpumask.h Thu Jul 24 12:33:45 2008 +1000
@@ -226,23 +226,8 @@ int __next_cpu(int n, const cpumask_t *s
#define next_cpu(n, src) ({ (void)(src); 1; })
#endif

-#ifdef CONFIG_HAVE_CPUMASK_OF_CPU_MAP
-extern cpumask_t *cpumask_of_cpu_map;
+extern const cpumask_t *cpumask_of_cpu_map;
#define cpumask_of_cpu(cpu) (cpumask_of_cpu_map[cpu])
-
-#else
-#define cpumask_of_cpu(cpu) \
-(*({ \
- typeof(_unused_cpumask_arg_) m; \
- if (sizeof(m) == sizeof(unsigned long)) { \
- m.bits[0] = 1UL<<(cpu); \
- } else { \
- cpus_clear(m); \
- cpu_set((cpu), m); \
- } \
- &m; \
-}))
-#endif

#define CPU_MASK_LAST_WORD BITMAP_LAST_WORD_MASK(NR_CPUS)

diff -r 95e128369c3c kernel/cpu.c
--- a/kernel/cpu.c Thu Jul 24 11:16:27 2008 +1000
+++ b/kernel/cpu.c Thu Jul 24 12:33:45 2008 +1000
@@ -428,3 +428,112 @@ out:
#endif /* CONFIG_PM_SLEEP_SMP */

#endif /* CONFIG_SMP */
+
+#ifndef CONFIG_HAVE_CPUMASK_OF_CPU_MAP
+/* 64 bits of zeros, for initializers. */
+#if BITS_PER_LONG == 32
+#define Z64 0, 0
+#else
+#define Z64 0
+#endif
+
+/* Initializer macros. */
+#define CMI0(n) { .bits = { 1UL << (n) } }
+#define CMI(n, ...) { .bits = { __VA_ARGS__, 1UL << ((n) % BITS_PER_LONG) } }
+
+#define CMI8(n, ...) \
+ CMI((n), __VA_ARGS__), CMI((n)+1, __VA_ARGS__), \
+ CMI((n)+2, __VA_ARGS__), CMI((n)+3, __VA_ARGS__), \
+ CMI((n)+4, __VA_ARGS__), CMI((n)+5, __VA_ARGS__), \
+ CMI((n)+6, __VA_ARGS__), CMI((n)+7, __VA_ARGS__)
+
+#if BITS_PER_LONG == 32
+#define CMI64(n, ...) \
+ CMI8((n), __VA_ARGS__), CMI8((n)+8, __VA_ARGS__), \
+ CMI8((n)+16, __VA_ARGS__), CMI8((n)+24, __VA_ARGS__), \
+ CMI8((n)+32, 0, __VA_ARGS__), CMI8((n)+40, 0, __VA_ARGS__), \
+ CMI8((n)+48, 0, __VA_ARGS__), CMI8((n)+56, 0, __VA_ARGS__)
+#else
+#define CMI64(n, ...) \
+ CMI8((n), __VA_ARGS__), CMI8((n)+8, __VA_ARGS__), \
+ CMI8((n)+16, __VA_ARGS__), CMI8((n)+24, __VA_ARGS__), \
+ CMI8((n)+32, __VA_ARGS__), CMI8((n)+40, __VA_ARGS__), \
+ CMI8((n)+48, __VA_ARGS__), CMI8((n)+56, __VA_ARGS__)
+#endif
+
+#define CMI256(n, ...) \
+ CMI64((n), __VA_ARGS__), CMI64((n)+64, Z64, __VA_ARGS__), \
+ CMI64((n)+128, Z64, Z64, __VA_ARGS__), \
+ CMI64((n)+192, Z64, Z64, Z64, __VA_ARGS__)
+#define Z256 Z64, Z64, Z64, Z64
+
+#define CMI1024(n, ...) \
+ CMI256((n), __VA_ARGS__), \
+ CMI256((n)+256, Z256, __VA_ARGS__), \
+ CMI256((n)+512, Z256, Z256, __VA_ARGS__), \
+ CMI256((n)+768, Z256, Z256, Z256, __VA_ARGS__)
+#define Z1024 Z256, Z256, Z256, Z256
+
+/* We want this statically initialized, just to be safe. We try not
+ * to waste too much space, either. */
+static const cpumask_t cpumask_map[] = {
+ CMI0(0), CMI0(1), CMI0(2), CMI0(3),
+#if NR_CPUS > 4
+ CMI0(4), CMI0(5), CMI0(6), CMI0(7),
+#endif
+#if NR_CPUS > 8
+ CMI0(8), CMI0(9), CMI0(10), CMI0(11),
+ CMI0(12), CMI0(13), CMI0(14), CMI0(15),
+#endif
+#if NR_CPUS > 16
+ CMI0(16), CMI0(17), CMI0(18), CMI0(19),
+ CMI0(20), CMI0(21), CMI0(22), CMI0(23),
+ CMI0(24), CMI0(25), CMI0(26), CMI0(27),
+ CMI0(28), CMI0(29), CMI0(30), CMI0(31),
+#endif
+#if NR_CPUS > 32
+#if BITS_PER_LONG == 32
+ CMI(32, 0), CMI(33, 0), CMI(34, 0), CMI(35, 0),
+ CMI(36, 0), CMI(37, 0), CMI(38, 0), CMI(39, 0),
+ CMI(40, 0), CMI(41, 0), CMI(42, 0), CMI(43, 0),
+ CMI(44, 0), CMI(45, 0), CMI(46, 0), CMI(47, 0),
+ CMI(48, 0), CMI(49, 0), CMI(50, 0), CMI(51, 0),
+ CMI(52, 0), CMI(53, 0), CMI(54, 0), CMI(55, 0),
+ CMI(56, 0), CMI(57, 0), CMI(58, 0), CMI(59, 0),
+ CMI(60, 0), CMI(61, 0), CMI(62, 0), CMI(63, 0),
+#else
+ CMI0(32), CMI0(33), CMI0(34), CMI0(35),
+ CMI0(36), CMI0(37), CMI0(38), CMI0(39),
+ CMI0(40), CMI0(41), CMI0(42), CMI0(43),
+ CMI0(44), CMI0(45), CMI0(46), CMI0(47),
+ CMI0(48), CMI0(49), CMI0(50), CMI0(51),
+ CMI0(52), CMI0(53), CMI0(54), CMI0(55),
+ CMI0(56), CMI0(57), CMI0(58), CMI0(59),
+ CMI0(60), CMI0(61), CMI0(62), CMI0(63),
+#endif /* BITS_PER_LONG == 64 */
+#endif
+#if NR_CPUS > 64
+ CMI64(64, Z64),
+#endif
+#if NR_CPUS > 128
+ CMI64(128, Z64, Z64), CMI64(192, Z64, Z64, Z64),
+#endif
+#if NR_CPUS > 256
+ CMI256(256, Z256),
+#endif
+#if NR_CPUS > 512
+ CMI256(512, Z256, Z256), CMI256(768, Z256, Z256, Z256),
+#endif
+#if NR_CPUS > 1024
+ CMI1024(1024, Z1024),
+#endif
+#if NR_CPUS > 2048
+ CMI1024(2048, Z1024, Z1024), CMI1024(3072, Z1024, Z1024, Z1024),
+#endif
+#if NR_CPUS > 4096
+#error NR_CPUS too big. Fix initializers or set CONFIG_HAVE_CPUMASK_OF_CPU_MAP
+#endif
+};
+
+const cpumask_t *cpumask_of_cpu_map = cpumask_map;
+#endif /* !CONFIG_HAVE_CPUMASK_OF_CPU_MAP */

2008-07-24 11:47:38

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 1/1] cpumask: Change cpumask_of_cpu to use cpumask_of_cpu_map


found a build failure with your patch:

init/main.c:493: error: array index in non-array initializer
init/main.c:493: error: (near initialization for 'cpumask_of_cpu_map[0]')
init/main.c:494: warning: missing braces around initializer
init/main.c:494: warning: (near initialization for 'cpumask_of_cpu_map[0].bits')

with this config:

http://redhat.com/~mingo/misc/config-Thu_Jul_24_13_44_56_CEST_2008.bad

Ingo

2008-07-24 12:57:19

by Bert Wesarg

[permalink] [raw]
Subject: Re: [PATCH 1/1] cpumask: Change cpumask_of_cpu to use cpumask_of_cpu_map

Hi,

On Wed, Jul 23, 2008 at 19:18, Mike Travis <[email protected]> wrote:
> --- linux-2.6.tip.orig/arch/x86/kernel/setup_percpu.c
> +++ linux-2.6.tip/arch/x86/kernel/setup_percpu.c
> @@ -80,8 +80,15 @@ static void __init setup_per_cpu_maps(vo
> #endif
> }
>
> -#ifdef CONFIG_HAVE_CPUMASK_OF_CPU_MAP
> -cpumask_t *cpumask_of_cpu_map __read_mostly;
> +#ifdef CONFIG_HAVE_CPUMASK_OF_CPU_MAP_PTR
> +/*
> + * Configure an initial cpumask_of_cpu(0) for early users
> + */
> +static cpumask_t initial_cpumask_of_cpu_map __initdata = (cpumask_t) { {
> + [BITS_TO_LONGS(NR_CPUS)-1] = 1
> +} };
This looks weird, first one missing {} pair, which may explain Ingo's
build error. Second, why do you want to set the last unsigned long to
one? Shouldn't this be the first?

Regards
Bert


> +cpumask_t *cpumask_of_cpu_map __read_mostly =
> + (cpumask_t *)&initial_cpumask_of_cpu_map;
> EXPORT_SYMBOL(cpumask_of_cpu_map);
>

2008-07-24 17:11:39

by Mike Travis

[permalink] [raw]
Subject: Re: [PATCH 1/1] cpumask: Change cpumask_of_cpu to use cpumask_of_cpu_map

Ingo Molnar wrote:
> found a build failure with your patch:
>
> init/main.c:493: error: array index in non-array initializer
> init/main.c:493: error: (near initialization for 'cpumask_of_cpu_map[0]')
> init/main.c:494: warning: missing braces around initializer
> init/main.c:494: warning: (near initialization for 'cpumask_of_cpu_map[0].bits')
>
> with this config:
>
> http://redhat.com/~mingo/misc/config-Thu_Jul_24_13_44_56_CEST_2008.bad
>
> Ingo

Hmm, it does not fail for me...

12> gcc --version
gcc (GCC) 4.2.4
Copyright (C) 2007 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

There are a number of warnings but none of the files are related to this patch.

(I'm still on hold until our sys admin gets back from vacation to install
gcc-4.2.3.)

Thanks,
Mike

2008-07-24 17:15:28

by Mike Travis

[permalink] [raw]
Subject: Re: [PATCH 1/1] cpumask: Change cpumask_of_cpu to use cpumask_of_cpu_map

Bert Wesarg wrote:
> Hi,
>
> On Wed, Jul 23, 2008 at 19:18, Mike Travis <[email protected]> wrote:
>> --- linux-2.6.tip.orig/arch/x86/kernel/setup_percpu.c
>> +++ linux-2.6.tip/arch/x86/kernel/setup_percpu.c
>> @@ -80,8 +80,15 @@ static void __init setup_per_cpu_maps(vo
>> #endif
>> }
>>
>> -#ifdef CONFIG_HAVE_CPUMASK_OF_CPU_MAP
>> -cpumask_t *cpumask_of_cpu_map __read_mostly;
>> +#ifdef CONFIG_HAVE_CPUMASK_OF_CPU_MAP_PTR
>> +/*
>> + * Configure an initial cpumask_of_cpu(0) for early users
>> + */
>> +static cpumask_t initial_cpumask_of_cpu_map __initdata = (cpumask_t) { {
>> + [BITS_TO_LONGS(NR_CPUS)-1] = 1
>> +} };
> This looks weird, first one missing {} pair, which may explain Ingo's
> build error.

Yes the compiler was pretty finicky, but it compiled with gcc-4.2.4. I'll
try out some other versions. But Rusty's different method of statically
initializing the array would replace the above anyways.

.... Second, why do you want to set the last unsigned long to
> one? Shouldn't this be the first?

The MSB of the cpumask is the highest numbered cpu, the LSB is the lowest.
(Caught me early on as well.)

Thanks,
Mike

>
> Regards
> Bert
>
>
>> +cpumask_t *cpumask_of_cpu_map __read_mostly =
>> + (cpumask_t *)&initial_cpumask_of_cpu_map;
>> EXPORT_SYMBOL(cpumask_of_cpu_map);
>>

2008-07-24 17:56:20

by Mike Travis

[permalink] [raw]
Subject: Re: [PATCH 1/1] cpumask: Change cpumask_of_cpu to use cpumask_of_cpu_map

Rusty Russell wrote:
> On Thursday 24 July 2008 03:18:42 Mike Travis wrote:
>> Note that the declaration of cpumask_of_cpu_map[] is initialized
>> so that cpumask_of_cpu(0) is defined early. This assumes that
>> cpu 0 is the boot cpu.
>
> Hi Mike,
>
> Make this statically initialized please. That almost guarantees there'll
> be no problems. It's a little tricky to do, but possible. Patch below
> tested on 32 bit x86 only.

I thought about it, but it didn't seem to be worth the effort. One problem
though, the cpumask bits are such that the LSB of the last word is cpu 0.
So your initializer sets it up in reverse order. I'll see if I can't
figure out how to invert it (very tricky coding btw... ;-)

There was another version of cpumask_of_cpu() that used a percpu variable
for each cpumask_t but I'm not sure where that went. It still required a run
time initializer though (except that cpumask_of_cpu(0) was available early.)

static const cpumask_t cpumask_map[] = {
{ .bits = { 1UL << (0) } }, { .bits = { 1UL << (1) } }, { .bits = { 1UL << (2) } }, { .bits = { 1UL << (3) } },

{ .bits = { 1UL << (4) } }, { .bits = { 1UL << (5) } }, { .bits = { 1UL << (6) } }, { .bits = { 1UL << (7) } },


{ .bits = { 1UL << (8) } }, { .bits = { 1UL << (9) } }, { .bits = { 1UL << (10) } }, { .bits = { 1UL << (11) } },
{ .bits = { 1UL << (12) } }, { .bits = { 1UL << (13) } }, { .bits = { 1UL << (14) } }, { .bits = { 1UL << (15) } },


{ .bits = { 1UL << (16) } }, { .bits = { 1UL << (17) } }, { .bits = { 1UL << (18) } }, { .bits = { 1UL << (19) } },
{ .bits = { 1UL << (20) } }, { .bits = { 1UL << (21) } }, { .bits = { 1UL << (22) } }, { .bits = { 1UL << (23) } },
{ .bits = { 1UL << (24) } }, { .bits = { 1UL << (25) } }, { .bits = { 1UL << (26) } }, { .bits = { 1UL << (27) } },
{ .bits = { 1UL << (28) } }, { .bits = { 1UL << (29) } }, { .bits = { 1UL << (30) } }, { .bits = { 1UL << (31) } },
# 533 ".../linux-2.6.tip/kernel/cpu.c"
{ .bits = { 1UL << (32) } }, { .bits = { 1UL << (33) } }, { .bits = { 1UL << (34) } }, { .bits = { 1UL << (35) } },
{ .bits = { 1UL << (36) } }, { .bits = { 1UL << (37) } }, { .bits = { 1UL << (38) } }, { .bits = { 1UL << (39) } },
{ .bits = { 1UL << (40) } }, { .bits = { 1UL << (41) } }, { .bits = { 1UL << (42) } }, { .bits = { 1UL << (43) } },
{ .bits = { 1UL << (44) } }, { .bits = { 1UL << (45) } }, { .bits = { 1UL << (46) } }, { .bits = { 1UL << (47) } },
{ .bits = { 1UL << (48) } }, { .bits = { 1UL << (49) } }, { .bits = { 1UL << (50) } }, { .bits = { 1UL << (51) } },
{ .bits = { 1UL << (52) } }, { .bits = { 1UL << (53) } }, { .bits = { 1UL << (54) } }, { .bits = { 1UL << (55) } },
{ .bits = { 1UL << (56) } }, { .bits = { 1UL << (57) } }, { .bits = { 1UL << (58) } }, { .bits = { 1UL << (59) } },
{ .bits = { 1UL << (60) } }, { .bits = { 1UL << (61) } }, { .bits = { 1UL << (62) } }, { .bits = { 1UL << (63) } },

>
>> Based on linux-2.6.tip/master at the following commit:
>>
>> commit a099a9b18ab434594bb01ed920e8c58574203fa9
>> Merge: 9b4ae4d... f3b51d7...
>> Author: Ingo Molnar <[email protected]>
>> Date: Tue Jul 22 15:43:36 2008 +0200
>>
>> Merge branch 'out-of-tree'
>
> This is two-steps forward one step back. Have Ingo drop the original
> patches, and roll together into a single change. That should be much
> clearer. I know it's a PITA.
>
> Thanks,
> Rusty.
>
> Make cpumask_of_cpu_map generic
>
> If an arch doesn't define cpumask_of_cpu_map, create a generic
> statically-initialized one for them. This allows removal of the buggy
> cpumask_of_cpu() macro (&cpumask_of_cpu() gives address of
> out-of-scope var).
>
> An arch with NR_CPUS of 4096 probably wants to allocate this itself
> based on the actual number of CPUs, since otherwise they're using 2MB
> of rodata (1024 cpus means 128k). That's what
> CONFIG_HAVE_CPUMASK_OF_CPU_MAP is for (only x86/64 does so at the
> moment).
>
> In future as we support more CPUs, we'll need to resort to a
> get_cpu_map()/put_cpu_map() allocation scheme.
>
> Signed-off-by: Rusty Russell <[email protected]>
>
> diff -r 95e128369c3c include/linux/cpumask.h
> --- a/include/linux/cpumask.h Thu Jul 24 11:16:27 2008 +1000
> +++ b/include/linux/cpumask.h Thu Jul 24 12:33:45 2008 +1000
> @@ -226,23 +226,8 @@ int __next_cpu(int n, const cpumask_t *s
> #define next_cpu(n, src) ({ (void)(src); 1; })
> #endif
>
> -#ifdef CONFIG_HAVE_CPUMASK_OF_CPU_MAP
> -extern cpumask_t *cpumask_of_cpu_map;
> +extern const cpumask_t *cpumask_of_cpu_map;
> #define cpumask_of_cpu(cpu) (cpumask_of_cpu_map[cpu])
> -
> -#else
> -#define cpumask_of_cpu(cpu) \
> -(*({ \
> - typeof(_unused_cpumask_arg_) m; \
> - if (sizeof(m) == sizeof(unsigned long)) { \
> - m.bits[0] = 1UL<<(cpu); \
> - } else { \
> - cpus_clear(m); \
> - cpu_set((cpu), m); \
> - } \
> - &m; \
> -}))
> -#endif
>
> #define CPU_MASK_LAST_WORD BITMAP_LAST_WORD_MASK(NR_CPUS)
>
> diff -r 95e128369c3c kernel/cpu.c
> --- a/kernel/cpu.c Thu Jul 24 11:16:27 2008 +1000
> +++ b/kernel/cpu.c Thu Jul 24 12:33:45 2008 +1000
> @@ -428,3 +428,112 @@ out:
> #endif /* CONFIG_PM_SLEEP_SMP */
>
> #endif /* CONFIG_SMP */
> +
> +#ifndef CONFIG_HAVE_CPUMASK_OF_CPU_MAP
> +/* 64 bits of zeros, for initializers. */
> +#if BITS_PER_LONG == 32
> +#define Z64 0, 0
> +#else
> +#define Z64 0
> +#endif
> +
> +/* Initializer macros. */
> +#define CMI0(n) { .bits = { 1UL << (n) } }
> +#define CMI(n, ...) { .bits = { __VA_ARGS__, 1UL << ((n) % BITS_PER_LONG) } }
> +
> +#define CMI8(n, ...) \
> + CMI((n), __VA_ARGS__), CMI((n)+1, __VA_ARGS__), \
> + CMI((n)+2, __VA_ARGS__), CMI((n)+3, __VA_ARGS__), \
> + CMI((n)+4, __VA_ARGS__), CMI((n)+5, __VA_ARGS__), \
> + CMI((n)+6, __VA_ARGS__), CMI((n)+7, __VA_ARGS__)
> +
> +#if BITS_PER_LONG == 32
> +#define CMI64(n, ...) \
> + CMI8((n), __VA_ARGS__), CMI8((n)+8, __VA_ARGS__), \
> + CMI8((n)+16, __VA_ARGS__), CMI8((n)+24, __VA_ARGS__), \
> + CMI8((n)+32, 0, __VA_ARGS__), CMI8((n)+40, 0, __VA_ARGS__), \
> + CMI8((n)+48, 0, __VA_ARGS__), CMI8((n)+56, 0, __VA_ARGS__)
> +#else
> +#define CMI64(n, ...) \
> + CMI8((n), __VA_ARGS__), CMI8((n)+8, __VA_ARGS__), \
> + CMI8((n)+16, __VA_ARGS__), CMI8((n)+24, __VA_ARGS__), \
> + CMI8((n)+32, __VA_ARGS__), CMI8((n)+40, __VA_ARGS__), \
> + CMI8((n)+48, __VA_ARGS__), CMI8((n)+56, __VA_ARGS__)
> +#endif
> +
> +#define CMI256(n, ...) \
> + CMI64((n), __VA_ARGS__), CMI64((n)+64, Z64, __VA_ARGS__), \
> + CMI64((n)+128, Z64, Z64, __VA_ARGS__), \
> + CMI64((n)+192, Z64, Z64, Z64, __VA_ARGS__)
> +#define Z256 Z64, Z64, Z64, Z64
> +
> +#define CMI1024(n, ...) \
> + CMI256((n), __VA_ARGS__), \
> + CMI256((n)+256, Z256, __VA_ARGS__), \
> + CMI256((n)+512, Z256, Z256, __VA_ARGS__), \
> + CMI256((n)+768, Z256, Z256, Z256, __VA_ARGS__)
> +#define Z1024 Z256, Z256, Z256, Z256
> +
> +/* We want this statically initialized, just to be safe. We try not
> + * to waste too much space, either. */
> +static const cpumask_t cpumask_map[] = {
> + CMI0(0), CMI0(1), CMI0(2), CMI0(3),
> +#if NR_CPUS > 4
> + CMI0(4), CMI0(5), CMI0(6), CMI0(7),
> +#endif
> +#if NR_CPUS > 8
> + CMI0(8), CMI0(9), CMI0(10), CMI0(11),
> + CMI0(12), CMI0(13), CMI0(14), CMI0(15),
> +#endif
> +#if NR_CPUS > 16
> + CMI0(16), CMI0(17), CMI0(18), CMI0(19),
> + CMI0(20), CMI0(21), CMI0(22), CMI0(23),
> + CMI0(24), CMI0(25), CMI0(26), CMI0(27),
> + CMI0(28), CMI0(29), CMI0(30), CMI0(31),
> +#endif
> +#if NR_CPUS > 32
> +#if BITS_PER_LONG == 32
> + CMI(32, 0), CMI(33, 0), CMI(34, 0), CMI(35, 0),
> + CMI(36, 0), CMI(37, 0), CMI(38, 0), CMI(39, 0),
> + CMI(40, 0), CMI(41, 0), CMI(42, 0), CMI(43, 0),
> + CMI(44, 0), CMI(45, 0), CMI(46, 0), CMI(47, 0),
> + CMI(48, 0), CMI(49, 0), CMI(50, 0), CMI(51, 0),
> + CMI(52, 0), CMI(53, 0), CMI(54, 0), CMI(55, 0),
> + CMI(56, 0), CMI(57, 0), CMI(58, 0), CMI(59, 0),
> + CMI(60, 0), CMI(61, 0), CMI(62, 0), CMI(63, 0),
> +#else
> + CMI0(32), CMI0(33), CMI0(34), CMI0(35),
> + CMI0(36), CMI0(37), CMI0(38), CMI0(39),
> + CMI0(40), CMI0(41), CMI0(42), CMI0(43),
> + CMI0(44), CMI0(45), CMI0(46), CMI0(47),
> + CMI0(48), CMI0(49), CMI0(50), CMI0(51),
> + CMI0(52), CMI0(53), CMI0(54), CMI0(55),
> + CMI0(56), CMI0(57), CMI0(58), CMI0(59),
> + CMI0(60), CMI0(61), CMI0(62), CMI0(63),
> +#endif /* BITS_PER_LONG == 64 */
> +#endif
> +#if NR_CPUS > 64
> + CMI64(64, Z64),
> +#endif
> +#if NR_CPUS > 128
> + CMI64(128, Z64, Z64), CMI64(192, Z64, Z64, Z64),
> +#endif
> +#if NR_CPUS > 256
> + CMI256(256, Z256),
> +#endif
> +#if NR_CPUS > 512
> + CMI256(512, Z256, Z256), CMI256(768, Z256, Z256, Z256),
> +#endif
> +#if NR_CPUS > 1024
> + CMI1024(1024, Z1024),
> +#endif
> +#if NR_CPUS > 2048
> + CMI1024(2048, Z1024, Z1024), CMI1024(3072, Z1024, Z1024, Z1024),
> +#endif
> +#if NR_CPUS > 4096
> +#error NR_CPUS too big. Fix initializers or set CONFIG_HAVE_CPUMASK_OF_CPU_MAP
> +#endif
> +};
> +
> +const cpumask_t *cpumask_of_cpu_map = cpumask_map;
> +#endif /* !CONFIG_HAVE_CPUMASK_OF_CPU_MAP */

2008-07-24 19:12:39

by Mike Travis

[permalink] [raw]
Subject: Re: [PATCH 1/1] cpumask: Change cpumask_of_cpu to use cpumask_of_cpu_map

Mike Travis wrote:
> Ingo Molnar wrote:
>> found a build failure with your patch:
>>
>> init/main.c:493: error: array index in non-array initializer
>> init/main.c:493: error: (near initialization for 'cpumask_of_cpu_map[0]')
>> init/main.c:494: warning: missing braces around initializer
>> init/main.c:494: warning: (near initialization for 'cpumask_of_cpu_map[0].bits')
>>
>> with this config:
>>
>> http://redhat.com/~mingo/misc/config-Thu_Jul_24_13_44_56_CEST_2008.bad
>>
>> Ingo
>
> Hmm, it does not fail for me...
>
> 12> gcc --version
> gcc (GCC) 4.2.4
> Copyright (C) 2007 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions. There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>
> There are a number of warnings but none of the files are related to this patch.
>
> (I'm still on hold until our sys admin gets back from vacation to install
> gcc-4.2.3.)

It does not fail on gcc-4.2.3 either... ?

(btw, I'm merging Rusty's changes into this patch so the issue should be moot soon.)

Thanks,
Mike

2008-07-25 00:27:30

by Mike Travis

[permalink] [raw]
Subject: Re: [PATCH 1/1] cpumask: Change cpumask_of_cpu to use cpumask_of_cpu_map

Mike Travis wrote:
> Bert Wesarg wrote:
:
>
> .... Second, why do you want to set the last unsigned long to
>> one? Shouldn't this be the first?
>
> The MSB of the cpumask is the highest numbered cpu, the LSB is the lowest.
> (Caught me early on as well.)

Forget what I said. I was thinking of how the cpumask_scnprintf prints out
the cpumask and assumed (incorrectly) the bit layout was the same.

Mike

2008-07-25 00:38:01

by Mike Travis

[permalink] [raw]
Subject: Re: [PATCH 1/1] cpumask: Change cpumask_of_cpu to use cpumask_of_cpu_map

Mike Travis wrote:
> Rusty Russell wrote:
>> On Thursday 24 July 2008 03:18:42 Mike Travis wrote:
>>> Note that the declaration of cpumask_of_cpu_map[] is initialized
>>> so that cpumask_of_cpu(0) is defined early. This assumes that
>>> cpu 0 is the boot cpu.
>> Hi Mike,
>>
>> Make this statically initialized please. That almost guarantees there'll
>> be no problems. It's a little tricky to do, but possible. Patch below
>> tested on 32 bit x86 only.
>
> I thought about it, but it didn't seem to be worth the effort. One problem
> though, the cpumask bits are such that the LSB of the last word is cpu 0.
> So your initializer sets it up in reverse order. I'll see if I can't
> figure out how to invert it (very tricky coding btw... ;-)

I thought since the cpumask_scnprintf prints out the bit map with the LSB
being cpu 0 that the bit layout was the same. Further examination reveals
I was wrong about that.

The updated patchset to follow shortly after a bit more testing...

Thanks,
Mike

2008-07-25 01:26:51

by Paul Jackson

[permalink] [raw]
Subject: Re: [PATCH 1/1] cpumask: Change cpumask_of_cpu to use cpumask_of_cpu_map

Mike wrote:
> the bit layout

See the comments near the top of lib/bitmap.c, ending with
the following lines, for documentation of the bitmap layout.

* The byte ordering of bitmaps is more natural on little
* endian architectures. See the big-endian headers
* include/asm-ppc64/bitops.h and include/asm-s390/bitops.h
* for the best explanations of this ordering.

--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <[email protected]> 1.940.382.4214

2008-07-30 16:56:17

by Dave Jones

[permalink] [raw]
Subject: Re: [PATCH 1/1] cpumask: Change cpumask_of_cpu to use cpumask_of_cpu_map

On Wed, Jul 23, 2008 at 10:18:42AM -0700, Mike Travis wrote:
> * The previous "lvalue replacement for cpumask_of_cpu()" was not usable
> in certain situations and generally added unneeded complexity. So
> this patch replaces the cpumask_of_cpu_ptr* macros with a generic
> cpumask_of_cpu_map[]. The only config option is whether this is a
> static map, or allocated at boot up time:

I've lost the plot on what's going on with these cpumask patches.
But I just saw this on -rc1.

arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c:206:33: error: not addressable
arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c:274:32: error: not addressable
arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c:352:34: error: not addressable

Dave

--
http://www.codemonkey.org.uk

2008-07-30 17:11:22

by Mike Travis

[permalink] [raw]
Subject: Re: [PATCH 1/1] cpumask: Change cpumask_of_cpu to use cpumask_of_cpu_map

Dave Jones wrote:
> On Wed, Jul 23, 2008 at 10:18:42AM -0700, Mike Travis wrote:
> > * The previous "lvalue replacement for cpumask_of_cpu()" was not usable
> > in certain situations and generally added unneeded complexity. So
> > this patch replaces the cpumask_of_cpu_ptr* macros with a generic
> > cpumask_of_cpu_map[]. The only config option is whether this is a
> > static map, or allocated at boot up time:
>
> I've lost the plot on what's going on with these cpumask patches.
> But I just saw this on -rc1.
>
> arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c:206:33: error: not addressable
> arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c:274:32: error: not addressable
> arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c:352:34: error: not addressable
>
> Dave
>

It looks like some old code is still there (patch dropped?). I'll send a fix asap.

Thanks,
Mike

2008-07-30 18:38:27

by Mike Travis

[permalink] [raw]
Subject: Re: [PATCH 1/1] cpumask: Change cpumask_of_cpu to use cpumask_of_cpu_map

Mike Travis wrote:
> Dave Jones wrote:
>> On Wed, Jul 23, 2008 at 10:18:42AM -0700, Mike Travis wrote:
>> > * The previous "lvalue replacement for cpumask_of_cpu()" was not usable
>> > in certain situations and generally added unneeded complexity. So
>> > this patch replaces the cpumask_of_cpu_ptr* macros with a generic
>> > cpumask_of_cpu_map[]. The only config option is whether this is a
>> > static map, or allocated at boot up time:
>>
>> I've lost the plot on what's going on with these cpumask patches.
>> But I just saw this on -rc1.
>>
>> arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c:206:33: error: not addressable
>> arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c:274:32: error: not addressable
>> arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c:352:34: error: not addressable
>>
>> Dave
>>
>
> It looks like some old code is still there (patch dropped?). I'll send a fix asap.
>
> Thanks,
> Mike

After I recreated linux-next the changes all appear to be there now.
One problem though, is !SMP config does not build (something about
DECLARE_BITMAP with NR_CPUS=1.) I wasn't sure how to create a constant
"(cpumask_t)1" that worked as an lvalue, so the following patch is
somewhat a kluge, but fulfills the requirements.

If anyone has a better suggestion, please let me know.

Subject: [PATCH] cpmask: add cpumask_of_cpu(0) for non-SMP

* Fix cpumask_of_cpu(0) when CONFIG_SMP is not set by providing a
pointer to a cpumask_t with cpu 0 bit set.

Signed-of-by: Mike Travis <[email protected]
---
include/linux/cpumask.h | 6 ++++++
kernel/cpu.c | 7 +++++--
2 files changed, 11 insertions(+), 2 deletions(-)

--- linux-2.6.tip.orig/include/linux/cpumask.h
+++ linux-2.6.tip/include/linux/cpumask.h
@@ -265,6 +265,7 @@ static inline void __cpus_shift_left(cpu
bitmap_shift_left(dstp->bits, srcp->bits, n, nbits);
}

+#ifdef CONFIG_SMP
/*
* Special-case data structure for "single bit set only" constant CPU masks.
*
@@ -288,6 +289,11 @@ static inline const cpumask_t *get_cpu_m
* variable created:
*/
#define cpumask_of_cpu(cpu) (*get_cpu_mask(cpu))
+#else
+
+extern const cpumask_t cpu_bit_bitmap_of_one;
+#define cpumask_of_cpu(cpu) cpu_bit_bitmap_of_one
+#endif


#define CPU_MASK_LAST_WORD BITMAP_LAST_WORD_MASK(NR_CPUS)
--- linux-2.6.tip.orig/kernel/cpu.c
+++ linux-2.6.tip/kernel/cpu.c
@@ -35,6 +35,8 @@ EXPORT_SYMBOL(cpu_online_map);
cpumask_t cpu_possible_map __read_mostly = CPU_MASK_ALL;
EXPORT_SYMBOL(cpu_possible_map);

+const cpumask_t cpu_bit_bitmap_of_one = { { [0] = 1 } };
+
#else /* CONFIG_SMP */

/* Serializes the updates to cpu_online_map, cpu_present_map */
@@ -454,8 +456,6 @@ out:
}
#endif /* CONFIG_PM_SLEEP_SMP */

-#endif /* CONFIG_SMP */
-
/*
* cpu_bit_bitmap[] is a special, "compressed" data structure that
* represents all NR_CPUS bits binary values of 1<<nr.
@@ -480,3 +480,6 @@ const unsigned long cpu_bit_bitmap[BITS_
#endif
};
EXPORT_SYMBOL_GPL(cpu_bit_bitmap);
+
+#endif /* CONFIG_SMP */
+

2008-07-30 18:50:17

by Dave Jones

[permalink] [raw]
Subject: Re: [PATCH 1/1] cpumask: Change cpumask_of_cpu to use cpumask_of_cpu_map

On Wed, Jul 30, 2008 at 11:37:52AM -0700, Mike Travis wrote:

> >> I've lost the plot on what's going on with these cpumask patches.
> >> But I just saw this on -rc1.
> >>
> >> arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c:206:33: error: not addressable
> >> arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c:274:32: error: not addressable
> >> arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c:352:34: error: not addressable
> >>
>
> After I recreated linux-next the changes all appear to be there now.
> One problem though, is !SMP config does not build (something about
> DECLARE_BITMAP with NR_CPUS=1.) I wasn't sure how to create a constant
> "(cpumask_t)1" that worked as an lvalue, so the following patch is
> somewhat a kluge, but fulfills the requirements.
>
> If anyone has a better suggestion, please let me know.
>
> Subject: [PATCH] cpmask: add cpumask_of_cpu(0) for non-SMP
>
> * Fix cpumask_of_cpu(0) when CONFIG_SMP is not set by providing a
> pointer to a cpumask_t with cpu 0 bit set.

confused. I saw the error above with a make allyesconfig, which
sets CONFIG_SMP=y

Dave

--
http://www.codemonkey.org.uk

2008-07-30 19:26:11

by Mike Travis

[permalink] [raw]
Subject: Re: [PATCH 1/1] cpumask: Change cpumask_of_cpu to use cpumask_of_cpu_map

Dave Jones wrote:
> On Wed, Jul 30, 2008 at 11:37:52AM -0700, Mike Travis wrote:
>
> > >> I've lost the plot on what's going on with these cpumask patches.
> > >> But I just saw this on -rc1.
> > >>
> > >> arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c:206:33: error: not addressable
> > >> arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c:274:32: error: not addressable
> > >> arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c:352:34: error: not addressable
> > >>
> >
> > After I recreated linux-next the changes all appear to be there now.
> > One problem though, is !SMP config does not build (something about
> > DECLARE_BITMAP with NR_CPUS=1.) I wasn't sure how to create a constant
> > "(cpumask_t)1" that worked as an lvalue, so the following patch is
> > somewhat a kluge, but fulfills the requirements.
> >
> > If anyone has a better suggestion, please let me know.
> >
> > Subject: [PATCH] cpmask: add cpumask_of_cpu(0) for non-SMP
> >
> > * Fix cpumask_of_cpu(0) when CONFIG_SMP is not set by providing a
> > pointer to a cpumask_t with cpu 0 bit set.
>
> confused. I saw the error above with a make allyesconfig, which
> sets CONFIG_SMP=y
>
> Dave
>

Well now the !SMP error is not there, so my "fixup" patch is not needed.
(It might have been because I had a slightly old copy of cpumask.h which had
the troublesome statement expression: "({ *get_cpu_mask(cpu); })" in it.)

But the errors you were seeing are fixed in the latest linux-next. When
I git-fetched, I got the same errors, but when I built a fresh copy of
linux-next, the correct code appeared.

A quick check...

arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c should have:

static void drv_write(struct drv_cmd *cmd)
{
cpumask_t saved_mask = current->cpus_allowed;
unsigned int i;

for_each_cpu_mask_nr(i, cmd->mask) {
set_cpus_allowed_ptr(current, &cpumask_of_cpu(i));
do_drv_write(cmd);
}

set_cpus_allowed_ptr(current, &saved_mask);
return;
}

instead of:

static void drv_write(struct drv_cmd *cmd)
{
cpumask_t saved_mask = current->cpus_allowed;
cpumask_of_cpu_ptr_declare(cpu_mask);
unsigned int i;

for_each_cpu_mask_nr(i, cmd->mask) {
cpumask_of_cpu_ptr_next(cpu_mask, i);
set_cpus_allowed_ptr(current, cpu_mask);
do_drv_write(cmd);
}

set_cpus_allowed_ptr(current, &saved_mask);
return;
}

Thanks,
Mike

2008-07-30 19:51:27

by Dave Jones

[permalink] [raw]
Subject: Re: [PATCH 1/1] cpumask: Change cpumask_of_cpu to use cpumask_of_cpu_map

On Wed, Jul 30, 2008 at 12:25:49PM -0700, Mike Travis wrote:

> But the errors you were seeing are fixed in the latest linux-next.

I swear I'm going to start taxing people for saying that.

Dave

--
http://www.codemonkey.org.uk

2008-07-30 21:00:38

by Tim Bird

[permalink] [raw]
Subject: Re: [PATCH 1/1] cpumask: Change cpumask_of_cpu to use cpumask_of_cpu_map - build breakage

Mike Travis wrote:
> * The previous "lvalue replacement for cpumask_of_cpu()" was not usable
> in certain situations and generally added unneeded complexity. So
> this patch replaces the cpumask_of_cpu_ptr* macros with a generic
> cpumask_of_cpu_map[]. The only config option is whether this is a
> static map, or allocated at boot up time:
I'm having trouble compiling 2.6.27-rc1 for ARM OMAP.

I'm using gcc 3.4.4 (specifically, one used for
cross-compiling for ARM)

The file that fails to compile is kernel/time/tick-common.c

Here is the compiler message:
arm-sony-linux-gcc -Wp,-MD,kernel/time/.tick-common.o.d -nostdinc -isystem /a/home/usr/local/arm-sony-linux/devel/bin/../lib/gcc/arm-sony-linux/3.4.4/include -D__KERNEL__ -Iinclude -Iinclude2
-I/a/home/tbird/work/console-translations/linux/include -I/a/home/tbird/work/console-translations/linux/arch/arm/include -include include/linux/autoconf.h -mlittle-endian
-I/a/home/tbird/work/console-translations/linux/kernel/time -Ikernel/time -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -Werror-implicit-function-declaration -Os
-marm -fno-omit-frame-pointer -mapcs -mno-sched-prolog -mapcs-32 -mno-thumb-interwork -D__LINUX_ARM_ARCH__=5 -march=armv5te -mtune=arm9tdmi -malignment-traps -msoft-float -Uarm -fno-omit-frame-pointer
-fno-optimize-sibling-calls -Wdeclaration-after-statement -D"KBUILD_STR(s)=#s" -D"KBUILD_BASENAME=KBUILD_STR(tick_common)" -D"KBUILD_MODNAME=KBUILD_STR(tick_common)" -c -o kernel/time/tick-common.o
/a/home/tbird/work/console-translations/linux/kernel/time/tick-common.c
/a/home/tbird/work/console-translations/linux/kernel/time/tick-common.c: In function `tick_check_new_device':
/a/home/tbird/work/console-translations/linux/kernel/time/tick-common.c:210: error: invalid lvalue in unary `&'
/a/home/tbird/work/console-translations/linux/kernel/time/tick-common.c:223: error: invalid lvalue in unary `&'
/a/home/tbird/work/console-translations/linux/kernel/time/tick-common.c:255: error: invalid lvalue in unary `&'
make[3]: *** [kernel/time/tick-common.o] Error 1
make[2]: *** [kernel/time] Error 2
make[1]: *** [kernel] Error 2
make: *** [sub-make] Error 2

This seems to be related to commit 0bc3cc03fa6e in Linus' 2.6 tree.

Any assistance you can provide would be appreciated.

Thanks,
-- Tim

=============================
Tim Bird
Architecture Group Chair, CE Linux Forum
Senior Staff Engineer, Sony Corporation of America
=============================

2008-07-30 21:24:43

by Mike Travis

[permalink] [raw]
Subject: Re: [PATCH 1/1] cpumask: Change cpumask_of_cpu to use cpumask_of_cpu_map - build breakage

Tim Bird wrote:
> Mike Travis wrote:
>> * The previous "lvalue replacement for cpumask_of_cpu()" was not usable
>> in certain situations and generally added unneeded complexity. So
>> this patch replaces the cpumask_of_cpu_ptr* macros with a generic
>> cpumask_of_cpu_map[]. The only config option is whether this is a
>> static map, or allocated at boot up time:
> I'm having trouble compiling 2.6.27-rc1 for ARM OMAP.
>
> I'm using gcc 3.4.4 (specifically, one used for
> cross-compiling for ARM)
>
> The file that fails to compile is kernel/time/tick-common.c
>
> Here is the compiler message:
> arm-sony-linux-gcc -Wp,-MD,kernel/time/.tick-common.o.d -nostdinc -isystem /a/home/usr/local/arm-sony-linux/devel/bin/../lib/gcc/arm-sony-linux/3.4.4/include -D__KERNEL__ -Iinclude -Iinclude2
> -I/a/home/tbird/work/console-translations/linux/include -I/a/home/tbird/work/console-translations/linux/arch/arm/include -include include/linux/autoconf.h -mlittle-endian
> -I/a/home/tbird/work/console-translations/linux/kernel/time -Ikernel/time -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -Werror-implicit-function-declaration -Os
> -marm -fno-omit-frame-pointer -mapcs -mno-sched-prolog -mapcs-32 -mno-thumb-interwork -D__LINUX_ARM_ARCH__=5 -march=armv5te -mtune=arm9tdmi -malignment-traps -msoft-float -Uarm -fno-omit-frame-pointer
> -fno-optimize-sibling-calls -Wdeclaration-after-statement -D"KBUILD_STR(s)=#s" -D"KBUILD_BASENAME=KBUILD_STR(tick_common)" -D"KBUILD_MODNAME=KBUILD_STR(tick_common)" -c -o kernel/time/tick-common.o
> /a/home/tbird/work/console-translations/linux/kernel/time/tick-common.c
> /a/home/tbird/work/console-translations/linux/kernel/time/tick-common.c: In function `tick_check_new_device':
> /a/home/tbird/work/console-translations/linux/kernel/time/tick-common.c:210: error: invalid lvalue in unary `&'
> /a/home/tbird/work/console-translations/linux/kernel/time/tick-common.c:223: error: invalid lvalue in unary `&'
> /a/home/tbird/work/console-translations/linux/kernel/time/tick-common.c:255: error: invalid lvalue in unary `&'
> make[3]: *** [kernel/time/tick-common.o] Error 1
> make[2]: *** [kernel/time] Error 2
> make[1]: *** [kernel] Error 2
> make: *** [sub-make] Error 2
>
> This seems to be related to commit 0bc3cc03fa6e in Linus' 2.6 tree.
>
> Any assistance you can provide would be appreciated.
>
> Thanks,
> -- Tim
>
> =============================
> Tim Bird
> Architecture Group Chair, CE Linux Forum
> Senior Staff Engineer, Sony Corporation of America
> =============================

There was a later update that fixed this. You should have the following
in include/linux/cpumask.h particularly line 291.


e56b3bc7 (Linus Torvalds 2008-07-28 11:32:33 -0700 285) /*
e56b3bc7 (Linus Torvalds 2008-07-28 11:32:33 -0700 286) * In cases where we take the address of the cpumask immediately,
e56b3bc7 (Linus Torvalds 2008-07-28 11:32:33 -0700 287) * gcc optimizes it out (it's a constant) and there's no huge stack
e56b3bc7 (Linus Torvalds 2008-07-28 11:32:33 -0700 288) * variable created:
e56b3bc7 (Linus Torvalds 2008-07-28 11:32:33 -0700 289) */
1eddd657 (Stephen Rothwell 2008-07-29 16:07:37 +1000 290) #define cpumask_of_cpu(cpu) (*get_cpu_mask(cpu))
^1da177e (Linus Torvalds 2005-04-16 15:20:36 -0700 291)

If 1eddd657 is there, would you send the config?

Dave - I just re-verified that allyesconfig builds with lot's of warnings but no errors.

Thanks,
Mike

2008-07-30 22:01:34

by Tim Bird

[permalink] [raw]
Subject: Re: [PATCH 1/1] cpumask: Change cpumask_of_cpu to use cpumask_of_cpu_map - build breakage

Mike Travis wrote:
> There was a later update that fixed this. You should have the following
> in include/linux/cpumask.h particularly line 291.
>
>
> e56b3bc7 (Linus Torvalds 2008-07-28 11:32:33 -0700 285) /*
> e56b3bc7 (Linus Torvalds 2008-07-28 11:32:33 -0700 286) * In cases where we take the address of the cpumask immediately,
> e56b3bc7 (Linus Torvalds 2008-07-28 11:32:33 -0700 287) * gcc optimizes it out (it's a constant) and there's no huge stack
> e56b3bc7 (Linus Torvalds 2008-07-28 11:32:33 -0700 288) * variable created:
> e56b3bc7 (Linus Torvalds 2008-07-28 11:32:33 -0700 289) */
> 1eddd657 (Stephen Rothwell 2008-07-29 16:07:37 +1000 290) #define cpumask_of_cpu(cpu) (*get_cpu_mask(cpu))
> ^1da177e (Linus Torvalds 2005-04-16 15:20:36 -0700 291)
>
> If 1eddd657 is there, would you send the config?

I was working out of a linux-2.6.27-rc1.tar.bz2 I got off of kernel.org,
so my working tree doesn't have the commit information.


However, comparing my source with the source above shows a difference.
In a Linus' 2.6 tree I downloaded outside my firewall, I have the following:
3dd730f2 (Stephen Rothwell 2008-07-29 16:07:37 +1000 290)#define cpumask_of_cpu(cpu) (*get_cpu_mask(cpu))

It doesn't appear to have the change you mention. But 3dd730f2 looks
promising:
commit 3dd730f2b49f101b90d283c3efc4e6cd826dd8f6
Author: Stephen Rothwell <[email protected]>
Date: Tue Jul 29 16:07:37 2008 +1000

cpumask: statement expressions confuse some versions of gcc

when you take the address of the result. Noticed on a sparc64 compile
using a version 3.4.5 cross compiler.

kernel/time/tick-common.c: In function `tick_check_new_device':
kernel/time/tick-common.c:210: error: invalid lvalue in unary `&'
...

Just make it a regular expression.

Signed-off-by: Stephen Rothwell <[email protected]>
Acked-by: Ingo Molnar <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>

This looks like it's only 4 hours old. I'll give this a spin.

I don't see 1eddd657 anywhere in the commit log for cpumask.h
Is it in linux-next?

Thanks,
-- Tim

=============================
Tim Bird
Architecture Group Chair, CE Linux Forum
Senior Staff Engineer, Sony Corporation of America
=============================

2008-07-30 23:48:42

by Stephen Rothwell

[permalink] [raw]
Subject: Re: [PATCH 1/1] cpumask: Change cpumask_of_cpu to use cpumask_of_cpu_map - build breakage

Hi Tim,

On Wed, 30 Jul 2008 15:03:37 -0700 Tim Bird <[email protected]> wrote:
>
> I don't see 1eddd657 anywhere in the commit log for cpumask.h
> Is it in linux-next?

Yeah, that was the commit from next-20080730. (one less patch I need to
apply :-))

--
Cheers,
Stephen Rothwell [email protected]
http://www.canb.auug.org.au/~sfr/

2008-07-31 00:09:35

by Tim Bird

[permalink] [raw]
Subject: Re: [PATCH 1/1] cpumask: Change cpumask_of_cpu to use cpumask_of_cpu_map - build breakage

Stephen Rothwell wrote:
> Hi Tim,
>
> On Wed, 30 Jul 2008 15:03:37 -0700 Tim Bird <[email protected]> wrote:
>> I don't see 1eddd657 anywhere in the commit log for cpumask.h
>> Is it in linux-next?
>
> Yeah, that was the commit from next-20080730. (one less patch I need to
> apply :-))

FWIW, the patch fixed the problem for me. With the latest linux-2.6
from Linus' tree, I compiled successfully for ARM OMAP.
-- Tim

=============================
Tim Bird
Architecture Group Chair, CE Linux Forum
Senior Staff Engineer, Sony Corporation of America
=============================