* [PATCH] make voyager work again after the cpumask_t changes
@ 2003-08-28 19:02 James Bottomley
2003-08-28 19:10 ` Andrew Morton
2003-08-28 19:16 ` William Lee Irwin III
0 siblings, 2 replies; 7+ messages in thread
From: James Bottomley @ 2003-08-28 19:02 UTC (permalink / raw)
To: wli, Andrew Morton; +Cc: Linux Kernel
[-- Attachment #1: Type: text/plain, Size: 394 bytes --]
Most is just simple fixes; however, the needless change from atomic to
non-atomic operations in smp_invalidate_interrupt() caused me a lot of
pain to track down since it introduced some very subtle bugs.
I've also taken phys_cpu_present_map out of smp.h. Since it
physid_mask_t is defined in mpspec.h anyway, and contains a duplicate
definition, I don't believe it can hurt anything.
James
[-- Attachment #2: cpumask.diff --]
[-- Type: text/plain, Size: 3204 bytes --]
===== arch/i386/mach-voyager/voyager_smp.c 1.15 vs edited =====
--- 1.15/arch/i386/mach-voyager/voyager_smp.c Mon Aug 18 22:46:23 2003
+++ edited/arch/i386/mach-voyager/voyager_smp.c Thu Aug 28 12:50:04 2003
@@ -130,7 +130,7 @@
{
int cpu;
- for_each_cpu(cpu, mk_cpumask_const(cpu_online_map)) {
+ for_each_cpu(cpu, cpu_online_map) {
if(cpuset & (1<<cpu)) {
#ifdef VOYAGER_DEBUG
if(!cpu_isset(cpu, cpu_online_map))
@@ -874,10 +874,10 @@
asmlinkage void
smp_invalidate_interrupt(void)
{
- __u8 cpu = get_cpu();
+ __u8 cpu = smp_processor_id();
- if (!(smp_invalidate_needed & (1UL << cpu)))
- goto out;
+ if (!test_bit(cpu, &smp_invalidate_needed))
+ return;
/* This will flood messages. Don't uncomment unless you see
* Problems with cross cpu invalidation
VDEBUG(("VOYAGER SMP: CPU%d received INVALIDATE_CPI\n",
@@ -893,9 +893,9 @@
} else
leave_mm(cpu);
}
- smp_invalidate_needed |= 1UL << cpu;
- out:
- put_cpu_no_resched();
+ smp_mb__before_clear_bit();
+ clear_bit(cpu, &smp_invalidate_needed);
+ smp_mb__after_clear_bit();
}
/* All the new flush operations for 2.4 */
@@ -929,6 +929,7 @@
send_CPI(cpumask, VIC_INVALIDATE_CPI);
while (smp_invalidate_needed) {
+ mb();
if(--stuck == 0) {
printk("***WARNING*** Stuck doing invalidate CPI (CPU%d)\n", smp_processor_id());
break;
@@ -1464,7 +1465,7 @@
cpuset &= 0xff; /* only first 8 CPUs vaild for VIC CPI */
if(cpuset == 0)
return;
- for_each_cpu(cpu, mk_cpumask_const(cpu_online_map)) {
+ for_each_cpu(cpu, cpu_online_map) {
if(cpuset & (1<<cpu))
set_bit(cpi, &vic_cpi_mailbox[cpu]);
}
@@ -1578,7 +1579,7 @@
VDEBUG(("VOYAGER: enable_vic_irq(%d) CPU%d affinity 0x%lx\n",
irq, cpu, cpu_irq_affinity[cpu]));
spin_lock_irqsave(&vic_irq_lock, flags);
- for_each_cpu(real_cpu, mk_cpumask_const(cpu_online_map)) {
+ for_each_cpu(real_cpu, cpu_online_map) {
if(!(voyager_extended_vic_processors & (1<<real_cpu)))
continue;
if(!(cpu_irq_affinity[real_cpu] & mask)) {
@@ -1723,7 +1724,7 @@
printk("VOYAGER SMP: CPU%d lost interrupt %d\n",
cpu, irq);
- for_each_cpu(real_cpu, mk_cpumask_const(mask)) {
+ for_each_cpu(real_cpu, mask) {
outb(VIC_CPU_MASQUERADE_ENABLE | real_cpu,
VIC_PROCESSOR_ID);
@@ -1808,7 +1809,7 @@
* bus) */
return;
- for_each_cpu(cpu, mk_cpumask_const(cpu_online_map)) {
+ for_each_cpu(cpu, cpu_online_map) {
unsigned long cpu_mask = 1 << cpu;
if(cpu_mask & real_mask) {
@@ -1874,7 +1875,7 @@
int old_cpu = smp_processor_id(), cpu;
/* dump the interrupt masks of each processor */
- for_each_cpu(cpu, mk_cpumask_const(cpu_online_map)) {
+ for_each_cpu(cpu, cpu_online_map) {
__u16 imr, isr, irr;
unsigned long flags;
===== include/asm-i386/smp.h 1.28 vs edited =====
--- 1.28/include/asm-i386/smp.h Mon Aug 18 22:46:23 2003
+++ edited/include/asm-i386/smp.h Thu Aug 28 08:12:36 2003
@@ -32,7 +32,6 @@
*/
extern void smp_alloc_memory(void);
-extern physid_mask_t phys_cpu_present_map;
extern int pic_mode;
extern int smp_num_siblings;
extern int cpu_sibling_map[];
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] make voyager work again after the cpumask_t changes
2003-08-28 19:02 [PATCH] make voyager work again after the cpumask_t changes James Bottomley
@ 2003-08-28 19:10 ` Andrew Morton
2003-08-28 19:31 ` William Lee Irwin III
2003-08-28 19:37 ` James Bottomley
2003-08-28 19:16 ` William Lee Irwin III
1 sibling, 2 replies; 7+ messages in thread
From: Andrew Morton @ 2003-08-28 19:10 UTC (permalink / raw)
To: James Bottomley; +Cc: wli, linux-kernel
James Bottomley <James.Bottomley@SteelEye.com> wrote:
>
> Most is just simple fixes; however, the needless change from atomic to
> non-atomic operations in smp_invalidate_interrupt() caused me a lot of
> pain to track down since it introduced some very subtle bugs.
Yes, the generic code was like that too. It was causing lockups. Sorry, I
did not realise that voyager had a private invalidatation implementation.
Officially smp_invalidate_needed should be a cpumask_t and
smp_invalidate_interrupt() should be using cpu_isset() rather than
open-coded bitops. For all those 64-way voyagers out there ;)
(Actually it is legitimate: you may want to run a NR_CPUS=48 kernel on a
2-way voyager just for testing purposes). I'll drop your patch in as-is,
and maybe Bill can take a look at cpumaskifying it sometime?
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] make voyager work again after the cpumask_t changes
2003-08-28 19:02 [PATCH] make voyager work again after the cpumask_t changes James Bottomley
2003-08-28 19:10 ` Andrew Morton
@ 2003-08-28 19:16 ` William Lee Irwin III
1 sibling, 0 replies; 7+ messages in thread
From: William Lee Irwin III @ 2003-08-28 19:16 UTC (permalink / raw)
To: James Bottomley; +Cc: Andrew Morton, Linux Kernel
On Thu, Aug 28, 2003 at 03:02:55PM -0400, James Bottomley wrote:
> Most is just simple fixes; however, the needless change from atomic to
> non-atomic operations in smp_invalidate_interrupt() caused me a lot of
> pain to track down since it introduced some very subtle bugs.
> I've also taken phys_cpu_present_map out of smp.h. Since it
> physid_mask_t is defined in mpspec.h anyway, and contains a duplicate
> definition, I don't believe it can hurt anything.
Sorry about this; I'm not sure what kind of catastrophe brought about
the non-atomic smp_invalidate_interrupt() bits in voyager.c
-- wli
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] make voyager work again after the cpumask_t changes
2003-08-28 19:31 ` William Lee Irwin III
@ 2003-08-28 19:20 ` Andrew Morton
2003-08-28 19:40 ` William Lee Irwin III
0 siblings, 1 reply; 7+ messages in thread
From: Andrew Morton @ 2003-08-28 19:20 UTC (permalink / raw)
To: William Lee Irwin III; +Cc: James.Bottomley, linux-kernel
William Lee Irwin III <wli@holomorphy.com> wrote:
>
> > (Actually it is legitimate: you may want to run a NR_CPUS=48 kernel on a
> > 2-way voyager just for testing purposes). I'll drop your patch in as-is,
> > and maybe Bill can take a look at cpumaskifying it sometime?
>
> I'm not convinced it's worth it; AIUI there are architectural limits to
> Voyager that prevent it from ever supporting > 32x in hardware,
Sure. But we want a kernel which was compiled with NR_CPUS>32 to still
boot and run correctly on voyager.
Yes, the code as-is will happen to work, but dtrt and all that.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] make voyager work again after the cpumask_t changes
2003-08-28 19:10 ` Andrew Morton
@ 2003-08-28 19:31 ` William Lee Irwin III
2003-08-28 19:20 ` Andrew Morton
2003-08-28 19:37 ` James Bottomley
1 sibling, 1 reply; 7+ messages in thread
From: William Lee Irwin III @ 2003-08-28 19:31 UTC (permalink / raw)
To: Andrew Morton; +Cc: James Bottomley, linux-kernel
James Bottomley <James.Bottomley@SteelEye.com> wrote:
>> Most is just simple fixes; however, the needless change from atomic to
>> non-atomic operations in smp_invalidate_interrupt() caused me a lot of
>> pain to track down since it introduced some very subtle bugs.
On Thu, Aug 28, 2003 at 12:10:16PM -0700, Andrew Morton wrote:
> Yes, the generic code was like that too. It was causing lockups. Sorry, I
> did not realise that voyager had a private invalidatation implementation.
> Officially smp_invalidate_needed should be a cpumask_t and
> smp_invalidate_interrupt() should be using cpu_isset() rather than
> open-coded bitops. For all those 64-way voyagers out there ;)
> (Actually it is legitimate: you may want to run a NR_CPUS=48 kernel on a
> 2-way voyager just for testing purposes). I'll drop your patch in as-is,
> and maybe Bill can take a look at cpumaskifying it sometime?
I'm not convinced it's worth it; AIUI there are architectural limits to
Voyager that prevent it from ever supporting > 32x in hardware, though
it could be worth doing so in tandem with an across-the-board all-
subarch extension of generic i386 support.
-- wli
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] make voyager work again after the cpumask_t changes
2003-08-28 19:10 ` Andrew Morton
2003-08-28 19:31 ` William Lee Irwin III
@ 2003-08-28 19:37 ` James Bottomley
1 sibling, 0 replies; 7+ messages in thread
From: James Bottomley @ 2003-08-28 19:37 UTC (permalink / raw)
To: Andrew Morton; +Cc: wli, Linux Kernel
On Thu, 2003-08-28 at 15:10, Andrew Morton wrote:
> Yes, the generic code was like that too. It was causing lockups. Sorry, I
> did not realise that voyager had a private invalidatation implementation.
It actually has to since the invalidation implementation is a property
of the SMP HAL...fortunately voyager is the only subarch that has to
replace the SMP HAL wholesale.
> Officially smp_invalidate_needed should be a cpumask_t and
> smp_invalidate_interrupt() should be using cpu_isset() rather than
> open-coded bitops. For all those 64-way voyagers out there ;)
>
> (Actually it is legitimate: you may want to run a NR_CPUS=48 kernel on a
> 2-way voyager just for testing purposes). I'll drop your patch in as-is,
> and maybe Bill can take a look at cpumaskifying it sometime?
OK.
Actually, looking at the code made me realise that we can kill the
tlbstate_lock and run lockless, so I'll play with doing that too.
James
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] make voyager work again after the cpumask_t changes
2003-08-28 19:20 ` Andrew Morton
@ 2003-08-28 19:40 ` William Lee Irwin III
0 siblings, 0 replies; 7+ messages in thread
From: William Lee Irwin III @ 2003-08-28 19:40 UTC (permalink / raw)
To: Andrew Morton; +Cc: James.Bottomley, linux-kernel
William Lee Irwin III <wli@holomorphy.com> wrote:
>> I'm not convinced it's worth it; AIUI there are architectural limits to
>> Voyager that prevent it from ever supporting > 32x in hardware,
On Thu, Aug 28, 2003 at 12:20:06PM -0700, Andrew Morton wrote:
> Sure. But we want a kernel which was compiled with NR_CPUS>32 to still
> boot and run correctly on voyager.
> Yes, the code as-is will happen to work, but dtrt and all that.
Okay, I'll send in the obvious cleanup and run it by jejb first.
-- wli
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2003-08-28 19:38 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-08-28 19:02 [PATCH] make voyager work again after the cpumask_t changes James Bottomley
2003-08-28 19:10 ` Andrew Morton
2003-08-28 19:31 ` William Lee Irwin III
2003-08-28 19:20 ` Andrew Morton
2003-08-28 19:40 ` William Lee Irwin III
2003-08-28 19:37 ` James Bottomley
2003-08-28 19:16 ` William Lee Irwin III
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).