Re: [bug #2] Re: [PATCH 00/35] cpumask: Replace cpumask_t with struct cpumask

From: Rusty Russell
Date: Thu Oct 23 2008 - 18:30:14 EST


On Friday 24 October 2008 03:09:59 Ingo Molnar wrote:
> * Mike Travis <travis@xxxxxxx> wrote:
> > Rusty Russell wrote:
> > > On Friday 24 October 2008 01:20:25 Ingo Molnar wrote:
> > >> Thomas has started a -tip cross-build test, and there's massive
> > >> cross-build failures as well due to the cpumask changes:
> > >
> > > Yes. linux-next reported the same thing. I've backed out various arch
> > > changes for this reason.
> > >
> > >> it seems to me that this commit is massively borked:
> > >>
> > >> 4a792c2: cpumask: make CONFIG_NR_CPUS always valid
> > >
> > > Yep. This is the big one I dropped. There are a few others; Mike is
> > > just porting the changes across to your tree now.
> > >
> > > Cheers,
> > > Rusty.
> >
> > Hi Ingo,
> >
> > It would seem easier to back out previous patches and apply the
> > replacements. Does this work for you?
>
> no, it does not work fine. As i said, i reworked various small bits
> along the way of reviewing and integrating them, under the obvious
> assumption that you submitted the latest and greatest. I spent hours
> testing every single step as well. If you cannot get your workflow right
> then we cannot have this stuff in v2.6.28.

Indeed, though to be honest it's been easier to do the fixes in linux-next.

Here's the interdiff; it's not too bad (you already reverted the
arch-destroying config changes).

Summary:
1) Alpha defines cpu_possible_map to cpu_present_map: this isn't possible
under centralization, so just manipulate both together.
2) IA64, cris and m32r gratuitously re-declared cpu_possible_map; remove.
3) PowerPC Cell used __cpu_setall in a questionable way.
Arnd approved this version.
4) That also leads to weakening the BUG_ON() to WARN_ON_ONCE() for
CONFIG_DEBUG_PER_CPU_MAPS; Arnd wants to be reminded, but only once :)
5) Revert most of S390 smp_call_function_mask->smp_call_function_many: it's
safer to just do the minimal conversion.
6) Remove __deprecated from smp_call_function_mask: we're not pushing
replacement patches yet so it's not appropriate, and it breaks sparc64
which compiles arch/sparc with -Werror.
7) Hack header to set CONFIG_NR_CPUS for UP on archs; safer than touching
Kconfigs for them. Also update comment.

Thanks,
Rusty.

diff --git a/arch/alpha/include/asm/smp.h b/arch/alpha/include/asm/smp.h
index 544c69a..547e909 100644
--- a/arch/alpha/include/asm/smp.h
+++ b/arch/alpha/include/asm/smp.h
@@ -45,7 +45,6 @@ extern struct cpuinfo_alpha cpu_data[NR_CPUS];
#define raw_smp_processor_id() (current_thread_info()->cpu)

extern int smp_num_cpus;
-#define cpu_possible_map cpu_present_map

extern void arch_send_call_function_single_ipi(int cpu);
extern void arch_send_call_function_ipi(cpumask_t mask);
diff --git a/arch/alpha/kernel/process.c b/arch/alpha/kernel/process.c
index 351407e..f238370 100644
--- a/arch/alpha/kernel/process.c
+++ b/arch/alpha/kernel/process.c
@@ -94,6 +94,7 @@ common_shutdown_1(void *generic_ptr)
flags |= 0x00040000UL; /* "remain halted" */
*pflags = flags;
cpu_clear(cpuid, cpu_present_map);
+ cpu_clear(cpuid, cpu_possible_map);
halt();
}
#endif
@@ -120,6 +121,7 @@ common_shutdown_1(void *generic_ptr)
#ifdef CONFIG_SMP
/* Wait for the secondaries to halt. */
cpu_clear(boot_cpuid, cpu_present_map);
+ cpu_clear(boot_cpuid, cpu_possible_map);
while (cpus_weight(cpu_present_map))
barrier();
#endif
diff --git a/arch/alpha/kernel/smp.c b/arch/alpha/kernel/smp.c
index ac26335..ce6791a 100644
--- a/arch/alpha/kernel/smp.c
+++ b/arch/alpha/kernel/smp.c
@@ -435,6 +435,7 @@ setup_smp(void)
((char *)cpubase + i*hwrpb->processor_size);
if ((cpu->flags & 0x1cc) == 0x1cc) {
smp_num_probed++;
+ cpu_set(i, cpu_possible_map);
cpu_set(i, cpu_present_map);
cpu->pal_revision = boot_cpu_palrev;
}
@@ -468,6 +469,7 @@ smp_prepare_cpus(unsigned int max_cpus)

/* Nothing to do on a UP box, or when told not to. */
if (smp_num_probed == 1 || max_cpus == 0) {
+ cpu_possible_map = cpumask_of_cpu(boot_cpuid);
cpu_present_map = cpumask_of_cpu(boot_cpuid);
printk(KERN_INFO "SMP mode deactivated.\n");
return;
diff --git a/arch/ia64/include/asm/smp.h b/arch/ia64/include/asm/smp.h
index 12d96e0..21c4023 100644
--- a/arch/ia64/include/asm/smp.h
+++ b/arch/ia64/include/asm/smp.h
@@ -57,7 +57,6 @@ extern struct smp_boot_data {

extern char no_int_routing __devinitdata;

-extern cpumask_t cpu_online_map;
extern cpumask_t cpu_core_map[NR_CPUS];
DECLARE_PER_CPU(cpumask_t, cpu_sibling_map);
extern int smp_num_siblings;
diff --git a/arch/powerpc/platforms/cell/spu_base.c b/arch/powerpc/platforms/cell/spu_base.c
index a5bdb89..402c183 100644
--- a/arch/powerpc/platforms/cell/spu_base.c
+++ b/arch/powerpc/platforms/cell/spu_base.c
@@ -111,10 +111,11 @@ void spu_flush_all_slbs(struct mm_struct *mm)
*/
static inline void mm_needs_global_tlbie(struct mm_struct *mm)
{
- int nr = (NR_CPUS > 1) ? NR_CPUS : NR_CPUS + 1;
+ cpumask_setall(&mm->cpu_vm_mask);

/* Global TLBIE broadcast required with SPEs. */
- __cpus_setall(&mm->cpu_vm_mask, nr);
+ if (nr_cpu_ids == 1)
+ cpumask_set_cpu(1, &mm->cpu_vm_mask);
}

void spu_associate_mm(struct spu *spu, struct mm_struct *mm)
diff --git a/arch/s390/kernel/smp.c b/arch/s390/kernel/smp.c
index 10e7f97..29a3eb1 100644
--- a/arch/s390/kernel/smp.c
+++ b/arch/s390/kernel/smp.c
@@ -103,7 +103,7 @@ static void do_call_function(void)
}

static void __smp_call_function_map(void (*func) (void *info), void *info,
- int wait, struct cpumask *map)
+ int wait, cpumask_t map)
{
struct call_data_struct data;
int cpu, local = 0;
@@ -163,16 +163,14 @@ out:
* You must not call this function with disabled interrupts, from a
* hardware interrupt handler or from a bottom half.
*/
-
-/* protected by call_lock */
-static DEFINE_BITMAP(smp_call_map, CONFIG_NR_CPUS);
-
int smp_call_function(void (*func) (void *info), void *info, int wait)
{
+ cpumask_t map;
+
spin_lock(&call_lock);
- cpumask_copy(to_cpumask(smp_call_map), cpu_online_mask);
- cpumask_clear_cpu(smp_processor_id(), to_cpumask(smp_call_map));
- __smp_call_function_map(func, info, wait, to_cpumask(smp_call_map));
+ map = cpu_online_map;
+ cpu_clear(smp_processor_id(), map);
+ __smp_call_function_map(func, info, wait, map);
spin_unlock(&call_lock);
return 0;
}
@@ -194,8 +192,7 @@ int smp_call_function_single(int cpu, void (*func) (void *info), void *info,
int wait)
{
spin_lock(&call_lock);
- cpumask_copy(to_cpumask(smp_call_map), cpumask_of(cpu));
- __smp_call_function_map(func, info, wait, cpumask_of(cpu));
+ __smp_call_function_map(func, info, wait, cpumask_of_cpu(cpu));
spin_unlock(&call_lock);
return 0;
}
@@ -216,13 +213,13 @@ EXPORT_SYMBOL(smp_call_function_single);
* You must not call this function with disabled interrupts or from a
* hardware interrupt handler or from a bottom half handler.
*/
-int smp_call_function_many(const struct cpumask *mask,
+int smp_call_function_many(const struct cpumask *maskp,
void (*func)(void *), void *info, bool wait)
{
+ cpumask_t mask = *maskp;
spin_lock(&call_lock);
- cpumask_copy(to_cpumask(smp_call_map), cpu_online_mask);
- cpumask_clear_cpu(smp_processor_id(), to_cpumask(smp_call_map));
- __smp_call_function_map(func, info, wait, to_cpumask(smp_call_map));
+ cpu_clear(smp_processor_id(), mask);
+ __smp_call_function_map(func, info, wait, mask);
spin_unlock(&call_lock);
return 0;
}
diff --git a/include/asm-cris/smp.h b/include/asm-cris/smp.h
index dba33ab..c615a06 100644
--- a/include/asm-cris/smp.h
+++ b/include/asm-cris/smp.h
@@ -4,7 +4,6 @@
#include <linux/cpumask.h>

extern cpumask_t phys_cpu_present_map;
-extern cpumask_t cpu_possible_map;

#define raw_smp_processor_id() (current_thread_info()->cpu)

diff --git a/include/asm-m32r/smp.h b/include/asm-m32r/smp.h
index c5dd669..b96a6d2 100644
--- a/include/asm-m32r/smp.h
+++ b/include/asm-m32r/smp.h
@@ -63,8 +63,6 @@ extern volatile int cpu_2_physid[NR_CPUS];
#define raw_smp_processor_id() (current_thread_info()->cpu)

extern cpumask_t cpu_callout_map;
-extern cpumask_t cpu_possible_map;
-extern cpumask_t cpu_present_map;

static __inline__ int hard_smp_processor_id(void)
{
diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
index d1f22ee..295d9e8 100644
--- a/include/linux/cpumask.h
+++ b/include/linux/cpumask.h
@@ -254,8 +254,7 @@ extern cpumask_t _unused_cpumask_arg_;
static inline unsigned int cpumask_check(unsigned int cpu)
{
#ifdef CONFIG_DEBUG_PER_CPU_MAPS
- /* This breaks at runtime. */
- BUG_ON(cpu >= nr_cpumask_bits);
+ WARN_ON_ONCE(cpu >= nr_cpumask_bits);
#endif /* CONFIG_DEBUG_PER_CPU_MAPS */
return cpu;
}
diff --git a/include/linux/smp.h b/include/linux/smp.h
index c7bc2fa..748ebc9 100644
--- a/include/linux/smp.h
+++ b/include/linux/smp.h
@@ -71,7 +71,7 @@ int smp_call_function_single(int cpuid, void (*func) (void *info), void *info,
void __smp_call_function_single(int cpuid, struct call_single_data *data);

/* Use smp_call_function_many, which takes a pointer to the mask. */
-static inline int __deprecated
+static inline int
smp_call_function_mask(cpumask_t mask, void(*func)(void *info), void *info,
int wait)
{
diff --git a/include/linux/threads.h b/include/linux/threads.h
index 38d1a5d..08a0286 100644
--- a/include/linux/threads.h
+++ b/include/linux/threads.h
@@ -8,17 +8,18 @@
*/

/*
- * Maximum supported processors that can run under SMP. This value is
- * set via configure setting. The maximum is equal to the size of the
- * bitmasks used on that platform, i.e. 32 or 64. Setting this smaller
- * saves quite a bit of memory.
+ * Maximum supported processors that can run under SMP. Setting this smaller
+ * saves quite a bit of memory. Use nr_cpu_ids instead of this except for
+ * static bitmaps.
*/
-#ifdef CONFIG_SMP
-#define NR_CPUS CONFIG_NR_CPUS
-#else
-#define NR_CPUS 1
+#ifndef CONFIG_NR_CPUS
+/* FIXME: This should be fixed in the arch's Kconfig */
+#define CONFIG_NR_CPUS 1
#endif

+/* Places which use this should consider cpumask_var_t. */
+#define NR_CPUS CONFIG_NR_CPUS
+
#define MIN_THREADS_LEFT_FOR_ROOT 4

/*
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/