Re: WARNING: at arch/x86_64/kernel/smp.c:397 smp_call_function_mask()
On Fri, Sep 28, 2007 at 02:07:54PM +0200, Laurent Vivier wrote:
> >> 2- one in global_flush_tlb() which calls smp_call_function() with irqs
> >> disabled.
> >
> > That would be a big bug, and surely we would already have picked it up.
> >
> >
> >
> > argh, mainline's x86_64 smp_call_function() doesn't do the check. We've
> > had *heaps* of bugs in i386 where people were running smp_call_foo() under
> > local_irq_disable(). I wonder how many there are in x86_64?
Thank god it's a bug in alternative_instructions() instead of
global_flush_tlb().
The following patch fixed it.
===
call free_init_pages() with irqs enabled in alternative_instructions()
It fixes the warning message in smp_call_function*(), which should be called
with irqs disabled.
[0.31] CPU: L1 I Cache: 64K (64 bytes/line), D cache 64K (64 bytes/line)
[0.31] CPU: L2 Cache: 512K (64 bytes/line)
[0.31] CPU 0/0 -> Node 0
[0.31] SMP alternatives: switching to UP code
[ 0.31] Freeing SMP alternatives: 25k freed
[0.31] WARNING: at arch/x86_64/kernel/smp.c:397 smp_call_function_mask()
[0.31]
[0.31] Call Trace:
[0.31] [] dump_trace+0x3ee/0x4a0
[0.31] [] show_trace+0x43/0x70
[0.31] [] dump_stack+0x15/0x20
[0.31] [] smp_call_function_mask+0x94/0xa0
[0.31] [] smp_call_function+0x32/0x40
[0.31] [] on_each_cpu+0x1f/0x50
[0.31] [] global_flush_tlb+0x8c/0x110
[0.31] [] free_init_pages+0xe5/0xf0
[0.31] [] alternative_instructions+0x7e/0x150
[0.31] [] check_bugs+0x1a/0x20
[0.31] [] start_kernel+0x2da/0x380
[0.31] [] _sinittext+0x132/0x140
[0.31]
[0.32] ACPI: Core revision 20070126
[0.56] Using local APIC timer interrupts.
[0.59] Detected 62.496 MHz APIC timer.
[0.59] Brought up 1 CPUs
Cc: Laurent Vivier <[EMAIL PROTECTED]>
Cc: Andi Kleen <[EMAIL PROTECTED]>
Signed-off-by: Fengguang Wu <[EMAIL PROTECTED]>
---
arch/i386/kernel/alternative.c |8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
--- linux-2.6.23-rc8-mm2.orig/arch/i386/kernel/alternative.c
+++ linux-2.6.23-rc8-mm2/arch/i386/kernel/alternative.c
@@ -435,9 +435,6 @@ void __init alternative_instructions(voi
alternatives_smp_unlock(__smp_locks, __smp_locks_end,
_text, _etext);
}
- free_init_pages("SMP alternatives",
- (unsigned long)__smp_locks,
- (unsigned long)__smp_locks_end);
} else {
alternatives_smp_module_add(NULL, "core kernel",
__smp_locks, __smp_locks_end,
@@ -448,6 +445,11 @@ void __init alternative_instructions(voi
apply_paravirt(__parainstructions, __parainstructions_end);
local_irq_restore(flags);
+ if (smp_alt_once)
+ free_init_pages("SMP alternatives",
+ (unsigned long)__smp_locks,
+ (unsigned long)__smp_locks_end);
+
restart_nmi();
#ifdef CONFIG_X86_MCE
restart_mce();
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: WARNING: at arch/x86_64/kernel/smp.c:397 smp_call_function_mask()
On Fri, Sep 28, 2007 at 02:07:54PM +0200, Laurent Vivier wrote: > Fengguang could you send me your .config and your qemu command line > parameters ? OK. I applied your patch and reran it. Here are the qemu cmdline, warning messages and .config: qemu-system-x86_64 -m 928 -hda /dev/sdb5 -kernel ./arch/x86_64/boot/bzImage -nographic -append "root=/dev/hda rw console=ttyS0 clock=pit init=/bin/bash" [0.31] CPU: L1 I Cache: 64K (64 bytes/line), D cache 64K (64 bytes/line) [0.31] CPU: L2 Cache: 512K (64 bytes/line) [0.31] CPU 0/0 -> Node 0 [0.31] SMP alternatives: switching to UP code [0.31] Freeing SMP alternatives: 25k freed [ 0.310000] WARNING: at arch/x86_64/kernel/smp.c:397 smp_call_function_mask() [0.31] [0.31] Call Trace: [0.31] [] dump_trace+0x3ee/0x4a0 [0.31] [] show_trace+0x43/0x70 [0.31] [] dump_stack+0x15/0x20 [0.31] [] smp_call_function_mask+0x94/0xa0 [0.31] [] smp_call_function+0x32/0x40 [0.31] [] on_each_cpu+0x1f/0x50 [0.31] [] global_flush_tlb+0x8c/0x110 [0.31] [] free_init_pages+0xe5/0xf0 [0.31] [] alternative_instructions+0x7e/0x150 [0.31] [] check_bugs+0x1a/0x20 [0.31] [] start_kernel+0x2da/0x380 [0.31] [] _sinittext+0x132/0x140 [0.31] [0.32] ACPI: Core revision 20070126 [0.56] Using local APIC timer interrupts. [0.59] Detected 62.496 MHz APIC timer. [0.59] Brought up 1 CPUs # # Automatically generated make config: don't edit # Linux kernel version: 2.6.23-rc8-mm2 # Fri Sep 28 10:26:40 2007 # CONFIG_X86_64=y CONFIG_64BIT=y CONFIG_X86=y CONFIG_GENERIC_TIME=y CONFIG_GENERIC_CLOCKEVENTS=y CONFIG_GENERIC_CLOCKEVENTS_BROADCAST=y CONFIG_NONIRQ_WAKEUP=y CONFIG_GENERIC_TIME_VSYSCALL=y CONFIG_GENERIC_CMOS_UPDATE=y CONFIG_ZONE_DMA32=y CONFIG_LOCKDEP_SUPPORT=y CONFIG_STACKTRACE_SUPPORT=y CONFIG_SEMAPHORE_SLEEPERS=y CONFIG_MMU=y CONFIG_ZONE_DMA=y CONFIG_RWSEM_GENERIC_SPINLOCK=y CONFIG_GENERIC_HWEIGHT=y CONFIG_GENERIC_CALIBRATE_DELAY=y CONFIG_X86_CMPXCHG=y CONFIG_EARLY_PRINTK=y CONFIG_GENERIC_ISA_DMA=y CONFIG_GENERIC_IOMAP=y CONFIG_ARCH_MAY_HAVE_PC_FDC=y CONFIG_ARCH_POPULATES_NODE_MAP=y CONFIG_DMI=y CONFIG_AUDIT_ARCH=y CONFIG_GENERIC_BUG=y # CONFIG_ARCH_HAS_ILOG2_U32 is not set # CONFIG_ARCH_HAS_ILOG2_U64 is not set CONFIG_DEFCONFIG_LIST="/lib/modules/$UNAME_RELEASE/.config" # # General setup # CONFIG_EXPERIMENTAL=y CONFIG_LOCK_KERNEL=y CONFIG_INIT_ENV_ARG_LIMIT=32 CONFIG_LOCALVERSION="" # CONFIG_LOCALVERSION_AUTO is not set # CONFIG_SWAP is not set CONFIG_SYSVIPC=y CONFIG_SYSVIPC_SYSCTL=y CONFIG_POSIX_MQUEUE=y CONFIG_BSD_PROCESS_ACCT=y CONFIG_BSD_PROCESS_ACCT_V3=y CONFIG_TASKSTATS=y CONFIG_TASK_DELAY_ACCT=y CONFIG_TASK_XACCT=y CONFIG_TASK_IO_ACCOUNTING=y # CONFIG_USER_NS is not set CONFIG_AUDIT=y # CONFIG_AUDITSYSCALL is not set CONFIG_IKCONFIG=y CONFIG_IKCONFIG_PROC=y CONFIG_LOG_BUF_SHIFT=18 # CONFIG_CGROUPS is not set CONFIG_FAIR_GROUP_SCHED=y CONFIG_FAIR_USER_SCHED=y # CONFIG_SYSFS_DEPRECATED is not set CONFIG_RELAY=y CONFIG_BLK_DEV_INITRD=y CONFIG_INITRAMFS_SOURCE="" # CONFIG_CC_OPTIMIZE_FOR_SIZE is not set CONFIG_SYSCTL=y CONFIG_EMBEDDED=y CONFIG_UID16=y CONFIG_SYSCTL_SYSCALL=y CONFIG_KALLSYMS=y CONFIG_KALLSYMS_ALL=y # CONFIG_KALLSYMS_EXTRA_PASS is not set CONFIG_HOTPLUG=y CONFIG_PRINTK=y CONFIG_BUG=y CONFIG_ELF_CORE=y CONFIG_BASE_FULL=y CONFIG_FUTEX=y CONFIG_ANON_INODES=y CONFIG_EPOLL=y CONFIG_SIGNALFD=y CONFIG_EVENTFD=y CONFIG_SHMEM=y CONFIG_VM_EVENT_COUNTERS=y CONFIG_SLUB_DEBUG=y # CONFIG_SLAB is not set CONFIG_SLUB=y # CONFIG_SLOB is not set CONFIG_PROC_PAGE_MONITOR=y CONFIG_PROC_KPAGEMAP=y CONFIG_RT_MUTEXES=y # CONFIG_TINY_SHMEM is not set CONFIG_BASE_SMALL=0 CONFIG_MODULES=y CONFIG_MODULE_UNLOAD=y CONFIG_MODULE_FORCE_UNLOAD=y CONFIG_MODVERSIONS=y # CONFIG_MODULE_SRCVERSION_ALL is not set CONFIG_KMOD=y CONFIG_STOP_MACHINE=y CONFIG_BLOCK=y CONFIG_BLK_DEV_IO_TRACE=y # CONFIG_BLK_DEV_BSG is not set # # IO Schedulers # CONFIG_IOSCHED_NOOP=y CONFIG_IOSCHED_AS=m CONFIG_IOSCHED_DEADLINE=m CONFIG_IOSCHED_CFQ=y # CONFIG_DEFAULT_AS is not set # CONFIG_DEFAULT_DEADLINE is not set CONFIG_DEFAULT_CFQ=y # CONFIG_DEFAULT_NOOP is not set CONFIG_DEFAULT_IOSCHED="cfq" CONFIG_PREEMPT_NOTIFIERS=y # # Processor type and features # # CONFIG_TICK_ONESHOT is not set # CONFIG_NO_HZ is not set # CONFIG_HIGH_RES_TIMERS is not set CONFIG_GENERIC_CLOCKEVENTS_BUILD=y CONFIG_X86_PC=y # CONFIG_X86_VSMP is not set # CONFIG_MK8 is not set # CONFIG_MPSC is not set CONFIG_MCORE2=y # CONFIG_GENERIC_CPU is not set CONFIG_X86_L1_CACHE_BYTES=64 CONFIG_X86_L1_CACHE_SHIFT=6 CONFIG_X86_INTERNODE_CACHE_BYTES=64 CONFIG_X86_TSC=y CONFIG_X86_GOOD_APIC=y CONFIG_MICROCODE=m CONFIG_MICROCODE_OLD_INTERFACE=y CONFIG_X86_MSR=m CONFIG_X86_CPUID=m CONFIG_X86_HT=y CONFIG_X86_IO_APIC=y CONFIG_X86_LOCAL_APIC=y CONFIG_MTRR=y CONFIG_SMP=y CONFIG_SCHED_SMT=y CONFIG_SCHED_MC=y
Re: WARNING: at arch/x86_64/kernel/smp.c:397 smp_call_function_mask()
On Fri, Sep 28, 2007 at 02:07:54PM +0200, Laurent Vivier wrote: Fengguang could you send me your .config and your qemu command line parameters ? OK. I applied your patch and reran it. Here are the qemu cmdline, warning messages and .config: qemu-system-x86_64 -m 928 -hda /dev/sdb5 -kernel ./arch/x86_64/boot/bzImage -nographic -append root=/dev/hda rw console=ttyS0 clock=pit init=/bin/bash [0.31] CPU: L1 I Cache: 64K (64 bytes/line), D cache 64K (64 bytes/line) [0.31] CPU: L2 Cache: 512K (64 bytes/line) [0.31] CPU 0/0 - Node 0 [0.31] SMP alternatives: switching to UP code [0.31] Freeing SMP alternatives: 25k freed [0.31] WARNING: at arch/x86_64/kernel/smp.c:397 smp_call_function_mask() [0.31] [0.31] Call Trace: [0.31] [8100dbde] dump_trace+0x3ee/0x4a0 [0.31] [8100dcd3] show_trace+0x43/0x70 [0.31] [8100dd15] dump_stack+0x15/0x20 [0.31] [8101cd44] smp_call_function_mask+0x94/0xa0 [0.31] [8101d0b2] smp_call_function+0x32/0x40 [0.31] [8104277f] on_each_cpu+0x1f/0x50 [0.31] [81026eac] global_flush_tlb+0x8c/0x110 [0.31] [81025c85] free_init_pages+0xe5/0xf0 [0.31] [81549b5e] alternative_instructions+0x7e/0x150 [0.31] [8154a2ea] check_bugs+0x1a/0x20 [0.31] [81540c4a] start_kernel+0x2da/0x380 [0.31] [81540132] _sinittext+0x132/0x140 [0.31] [0.32] ACPI: Core revision 20070126 [0.56] Using local APIC timer interrupts. [0.59] Detected 62.496 MHz APIC timer. [0.59] Brought up 1 CPUs # # Automatically generated make config: don't edit # Linux kernel version: 2.6.23-rc8-mm2 # Fri Sep 28 10:26:40 2007 # CONFIG_X86_64=y CONFIG_64BIT=y CONFIG_X86=y CONFIG_GENERIC_TIME=y CONFIG_GENERIC_CLOCKEVENTS=y CONFIG_GENERIC_CLOCKEVENTS_BROADCAST=y CONFIG_NONIRQ_WAKEUP=y CONFIG_GENERIC_TIME_VSYSCALL=y CONFIG_GENERIC_CMOS_UPDATE=y CONFIG_ZONE_DMA32=y CONFIG_LOCKDEP_SUPPORT=y CONFIG_STACKTRACE_SUPPORT=y CONFIG_SEMAPHORE_SLEEPERS=y CONFIG_MMU=y CONFIG_ZONE_DMA=y CONFIG_RWSEM_GENERIC_SPINLOCK=y CONFIG_GENERIC_HWEIGHT=y CONFIG_GENERIC_CALIBRATE_DELAY=y CONFIG_X86_CMPXCHG=y CONFIG_EARLY_PRINTK=y CONFIG_GENERIC_ISA_DMA=y CONFIG_GENERIC_IOMAP=y CONFIG_ARCH_MAY_HAVE_PC_FDC=y CONFIG_ARCH_POPULATES_NODE_MAP=y CONFIG_DMI=y CONFIG_AUDIT_ARCH=y CONFIG_GENERIC_BUG=y # CONFIG_ARCH_HAS_ILOG2_U32 is not set # CONFIG_ARCH_HAS_ILOG2_U64 is not set CONFIG_DEFCONFIG_LIST=/lib/modules/$UNAME_RELEASE/.config # # General setup # CONFIG_EXPERIMENTAL=y CONFIG_LOCK_KERNEL=y CONFIG_INIT_ENV_ARG_LIMIT=32 CONFIG_LOCALVERSION= # CONFIG_LOCALVERSION_AUTO is not set # CONFIG_SWAP is not set CONFIG_SYSVIPC=y CONFIG_SYSVIPC_SYSCTL=y CONFIG_POSIX_MQUEUE=y CONFIG_BSD_PROCESS_ACCT=y CONFIG_BSD_PROCESS_ACCT_V3=y CONFIG_TASKSTATS=y CONFIG_TASK_DELAY_ACCT=y CONFIG_TASK_XACCT=y CONFIG_TASK_IO_ACCOUNTING=y # CONFIG_USER_NS is not set CONFIG_AUDIT=y # CONFIG_AUDITSYSCALL is not set CONFIG_IKCONFIG=y CONFIG_IKCONFIG_PROC=y CONFIG_LOG_BUF_SHIFT=18 # CONFIG_CGROUPS is not set CONFIG_FAIR_GROUP_SCHED=y CONFIG_FAIR_USER_SCHED=y # CONFIG_SYSFS_DEPRECATED is not set CONFIG_RELAY=y CONFIG_BLK_DEV_INITRD=y CONFIG_INITRAMFS_SOURCE= # CONFIG_CC_OPTIMIZE_FOR_SIZE is not set CONFIG_SYSCTL=y CONFIG_EMBEDDED=y CONFIG_UID16=y CONFIG_SYSCTL_SYSCALL=y CONFIG_KALLSYMS=y CONFIG_KALLSYMS_ALL=y # CONFIG_KALLSYMS_EXTRA_PASS is not set CONFIG_HOTPLUG=y CONFIG_PRINTK=y CONFIG_BUG=y CONFIG_ELF_CORE=y CONFIG_BASE_FULL=y CONFIG_FUTEX=y CONFIG_ANON_INODES=y CONFIG_EPOLL=y CONFIG_SIGNALFD=y CONFIG_EVENTFD=y CONFIG_SHMEM=y CONFIG_VM_EVENT_COUNTERS=y CONFIG_SLUB_DEBUG=y # CONFIG_SLAB is not set CONFIG_SLUB=y # CONFIG_SLOB is not set CONFIG_PROC_PAGE_MONITOR=y CONFIG_PROC_KPAGEMAP=y CONFIG_RT_MUTEXES=y # CONFIG_TINY_SHMEM is not set CONFIG_BASE_SMALL=0 CONFIG_MODULES=y CONFIG_MODULE_UNLOAD=y CONFIG_MODULE_FORCE_UNLOAD=y CONFIG_MODVERSIONS=y # CONFIG_MODULE_SRCVERSION_ALL is not set CONFIG_KMOD=y CONFIG_STOP_MACHINE=y CONFIG_BLOCK=y CONFIG_BLK_DEV_IO_TRACE=y # CONFIG_BLK_DEV_BSG is not set # # IO Schedulers # CONFIG_IOSCHED_NOOP=y CONFIG_IOSCHED_AS=m CONFIG_IOSCHED_DEADLINE=m CONFIG_IOSCHED_CFQ=y # CONFIG_DEFAULT_AS is not set # CONFIG_DEFAULT_DEADLINE is not set CONFIG_DEFAULT_CFQ=y # CONFIG_DEFAULT_NOOP is not set CONFIG_DEFAULT_IOSCHED=cfq CONFIG_PREEMPT_NOTIFIERS=y # # Processor type and features # # CONFIG_TICK_ONESHOT is not set # CONFIG_NO_HZ is not set # CONFIG_HIGH_RES_TIMERS is not set CONFIG_GENERIC_CLOCKEVENTS_BUILD=y CONFIG_X86_PC=y # CONFIG_X86_VSMP is not set # CONFIG_MK8 is not set # CONFIG_MPSC is not set CONFIG_MCORE2=y # CONFIG_GENERIC_CPU is not set CONFIG_X86_L1_CACHE_BYTES=64 CONFIG_X86_L1_CACHE_SHIFT=6 CONFIG_X86_INTERNODE_CACHE_BYTES=64 CONFIG_X86_TSC=y CONFIG_X86_GOOD_APIC=y CONFIG_MICROCODE=m CONFIG_MICROCODE_OLD_INTERFACE=y CONFIG_X86_MSR=m CONFIG_X86_CPUID=m CONFIG_X86_HT
Re: WARNING: at arch/x86_64/kernel/smp.c:397 smp_call_function_mask()
On Fri, Sep 28, 2007 at 02:07:54PM +0200, Laurent Vivier wrote:
2- one in global_flush_tlb() which calls smp_call_function() with irqs
disabled.
That would be a big bug, and surely we would already have picked it up.
looks
argh, mainline's x86_64 smp_call_function() doesn't do the check. We've
had *heaps* of bugs in i386 where people were running smp_call_foo() under
local_irq_disable(). I wonder how many there are in x86_64?
Thank god it's a bug in alternative_instructions() instead of
global_flush_tlb().
The following patch fixed it.
===
call free_init_pages() with irqs enabled in alternative_instructions()
It fixes the warning message in smp_call_function*(), which should be called
with irqs disabled.
[0.31] CPU: L1 I Cache: 64K (64 bytes/line), D cache 64K (64 bytes/line)
[0.31] CPU: L2 Cache: 512K (64 bytes/line)
[0.31] CPU 0/0 - Node 0
[0.31] SMP alternatives: switching to UP code
[0.31] Freeing SMP alternatives: 25k freed
[0.31] WARNING: at arch/x86_64/kernel/smp.c:397 smp_call_function_mask()
[0.31]
[0.31] Call Trace:
[0.31] [8100dbde] dump_trace+0x3ee/0x4a0
[0.31] [8100dcd3] show_trace+0x43/0x70
[0.31] [8100dd15] dump_stack+0x15/0x20
[0.31] [8101cd44] smp_call_function_mask+0x94/0xa0
[0.31] [8101d0b2] smp_call_function+0x32/0x40
[0.31] [8104277f] on_each_cpu+0x1f/0x50
[0.31] [81026eac] global_flush_tlb+0x8c/0x110
[0.31] [81025c85] free_init_pages+0xe5/0xf0
[0.31] [81549b5e] alternative_instructions+0x7e/0x150
[0.31] [8154a2ea] check_bugs+0x1a/0x20
[0.31] [81540c4a] start_kernel+0x2da/0x380
[0.31] [81540132] _sinittext+0x132/0x140
[0.31]
[0.32] ACPI: Core revision 20070126
[0.56] Using local APIC timer interrupts.
[0.59] Detected 62.496 MHz APIC timer.
[0.59] Brought up 1 CPUs
Cc: Laurent Vivier [EMAIL PROTECTED]
Cc: Andi Kleen [EMAIL PROTECTED]
Signed-off-by: Fengguang Wu [EMAIL PROTECTED]
---
arch/i386/kernel/alternative.c |8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
--- linux-2.6.23-rc8-mm2.orig/arch/i386/kernel/alternative.c
+++ linux-2.6.23-rc8-mm2/arch/i386/kernel/alternative.c
@@ -435,9 +435,6 @@ void __init alternative_instructions(voi
alternatives_smp_unlock(__smp_locks, __smp_locks_end,
_text, _etext);
}
- free_init_pages(SMP alternatives,
- (unsigned long)__smp_locks,
- (unsigned long)__smp_locks_end);
} else {
alternatives_smp_module_add(NULL, core kernel,
__smp_locks, __smp_locks_end,
@@ -448,6 +445,11 @@ void __init alternative_instructions(voi
apply_paravirt(__parainstructions, __parainstructions_end);
local_irq_restore(flags);
+ if (smp_alt_once)
+ free_init_pages(SMP alternatives,
+ (unsigned long)__smp_locks,
+ (unsigned long)__smp_locks_end);
+
restart_nmi();
#ifdef CONFIG_X86_MCE
restart_mce();
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: WARNING: at arch/x86_64/kernel/smp.c:397 smp_call_function_mask()
Andrew Morton wrote: > On Fri, 28 Sep 2007 11:18:45 +0200 Laurent Vivier <[EMAIL PROTECTED]> wrote: > >> Andrew Morton wrote: >>> On Fri, 28 Sep 2007 10:52:08 +0200 Laurent Vivier <[EMAIL PROTECTED]> wrote: Andi, is this correct ? Andrew, should I send a patch implementing this change ? >>> umm, I think all the smp_call_function fucntions are deadlocky if called >>> with local interrupts disabled, regardless of whether the calling CPU is in >>> the mask. >>> >>> If CPU A is sending a cross-cpu call to CPU B and CPU B is sending a >>> cross-cpu call to CPU A, and they both have local interrupts disabled... >> OK, so there are two errors: >> >> 1- one I introduce myself (without any help from anyone) where >> smp_call_function() calls all online CPUs instead of calling all CPUs except >> itself. > > I'd be pretty surprised if one was able to write a bug like that. You mean > the CPU sends an IPI to itself and then loops around until it has serviced > that IPI? And this works? Wow. In fact, it works because __smp_call_function_mask() makes : cpus_and(mask, mask, allbutself); So it removes current cpu from the mask. BTW, I don't have to modify smp_call_function(): it is correct as it is written (except from a semantic point of view... do we care about semantic ?). > And on_each_cpu() can call the handler function twice? > > hm > >> 2- one in global_flush_tlb() which calls smp_call_function() with irqs >> disabled. > > That would be a big bug, and surely we would already have picked it up. > > > > argh, mainline's x86_64 smp_call_function() doesn't do the check. We've > had *heaps* of bugs in i386 where people were running smp_call_foo() under > local_irq_disable(). I wonder how many there are in x86_64? > >> I think I should at least correct #1 ? > > I think we should correct all bugs ;) > We don't live in a perfect world... ;-) Moreover, I'm not able to reproduce #2 ... Fengguang could you send me your .config and your qemu command line parameters ? Laurent PS: if semantic is important, you can apply attached patch... -- - [EMAIL PROTECTED] -- "Software is hard" - Donald Knuth 0001-smp_call_function-call-function-on-all-CPUs.patch Description: application/mbox signature.asc Description: OpenPGP digital signature
Re: WARNING: at arch/x86_64/kernel/smp.c:397 smp_call_function_mask()
On Fri, 28 Sep 2007 11:18:45 +0200 Laurent Vivier <[EMAIL PROTECTED]> wrote: > Andrew Morton wrote: > > On Fri, 28 Sep 2007 10:52:08 +0200 Laurent Vivier <[EMAIL PROTECTED]> wrote: > >> > >> Andi, is this correct ? > >> Andrew, should I send a patch implementing this change ? > > > > umm, I think all the smp_call_function fucntions are deadlocky if called > > with local interrupts disabled, regardless of whether the calling CPU is in > > the mask. > > > > If CPU A is sending a cross-cpu call to CPU B and CPU B is sending a > > cross-cpu call to CPU A, and they both have local interrupts disabled... > > OK, so there are two errors: > > 1- one I introduce myself (without any help from anyone) where > smp_call_function() calls all online CPUs instead of calling all CPUs except > itself. I'd be pretty surprised if one was able to write a bug like that. You mean the CPU sends an IPI to itself and then loops around until it has serviced that IPI? And this works? Wow. And on_each_cpu() can call the handler function twice? hm > 2- one in global_flush_tlb() which calls smp_call_function() with irqs > disabled. That would be a big bug, and surely we would already have picked it up. argh, mainline's x86_64 smp_call_function() doesn't do the check. We've had *heaps* of bugs in i386 where people were running smp_call_foo() under local_irq_disable(). I wonder how many there are in x86_64? > I think I should at least correct #1 ? I think we should correct all bugs ;) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: WARNING: at arch/x86_64/kernel/smp.c:397 smp_call_function_mask()
Andrew Morton wrote:
> On Fri, 28 Sep 2007 10:52:08 +0200 Laurent Vivier <[EMAIL PROTECTED]> wrote:
>
>> Fengguang Wu wrote:
>>> On Thu, Sep 27, 2007 at 02:22:20AM -0700, Andrew Morton wrote:
>>>> ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.23-rc8/2.6.23-rc8-mm2/
>>>
>>> Laurent,
>>>
>>> It triggered a WARNING on first run in qemu:
>> Thank you to report it.
>>
>>> [0.31] WARNING: at arch/x86_64/kernel/smp.c:397
>>> smp_call_function_mask()
>>> [0.31]
>>> [0.31] Call Trace:
>>> [0.31] [] dump_trace+0x3ee/0x4a0
>>> [0.31] [] show_trace+0x43/0x70
>>> [0.31] [] dump_stack+0x15/0x20
>>> [0.31] [] smp_call_function_mask+0x94/0xa0
>>> [0.31] [] smp_call_function+0x19/0x20
>>> [0.31] [] on_each_cpu+0x1f/0x50
>>> [0.31] [] global_flush_tlb+0x8c/0x110
>>> [0.31] [] free_init_pages+0xe5/0xf0
>>> [0.31] [] alternative_instructions+0x7e/0x150
>>> [0.31] [] check_bugs+0x1a/0x20
>>> [0.31] [] start_kernel+0x2da/0x380
>>> [0.31] [] _sinittext+0x132/0x140
>>
>> the reason is the WARN_ON():
>>
>> 390 int smp_call_function_mask(cpumask_t mask,
>> 391void (*func)(void *), void *info,
>> 392int wait)
>> 393 {
>> 394 int ret;
>> 395
>> 396 /* Can deadlock when called with interrupts disabled */
>> 397 WARN_ON(irqs_disabled());
>> 398
>> 399 spin_lock(_lock);
>> 400 ret = __smp_call_function_mask(mask, func, info, wait);
>> 401 spin_unlock(_lock);
>> 402 return ret;
>> 403 }
>>
>> The patch I sent to Andi didn't include this WARN_ON() and it's why I didn't
>> find this issue. (see http://lkml.org/lkml/2007/8/24/101)
>>
>> smp_call_function_mask() is called by smp_call_function() which calls a
>> function
>> on all CPU except current.
>> The comment of smp_call_function() specifies:
>> ...
>> * You must not call this function with disabled interrupts or from a
>> * hardware interrupt handler or from a bottom half handler.
>> * Actually there are a few legal cases, like panic.
>> */
>>
>> So this WARN_ON() is correct, and the caller (global_flush_tlb()) doesn't
>> follow
>> this rule.
>>
>> I guess this WARN_ON() is only needed when we have current CPU in provided
>> mask.
>> So I think we should change:
>>
>> int smp_call_function (void (*func) (void *info), void *info, int nonatomic,
>> int wait)
>> {
>> return smp_call_function_mask(cpu_online_map, func, info, wait);
>> }
>> ("cpu_online_map" is a bad choice, comment also specifies: "run a function on
>> all other CPU")
>>
>> to
>>
>> int smp_call_function (void (*func) (void *info), void *info, int nonatomic,
>> int wait)
>> {
>> int ret;
>> cpumask_t allbutself;
>>
>> allbutself = cpu_online_map;
>> cpu_clear(smp_processor_id(), allbutself);
>>
>> spin_lock(_lock);
>> ret = __smp_call_function_mask(allbutself, func, info, wait);
>> spin_unlock(_lock);
>> return ret;
>> }
>> (which is smp_call_function_mask() without the WARN_ON() and without current
>> cpu
>> in the mask)
>>
>> Andi, is this correct ?
>> Andrew, should I send a patch implementing this change ?
>
> umm, I think all the smp_call_function fucntions are deadlocky if called
> with local interrupts disabled, regardless of whether the calling CPU is in
> the mask.
>
> If CPU A is sending a cross-cpu call to CPU B and CPU B is sending a
> cross-cpu call to CPU A, and they both have local interrupts disabled...
OK, so there are two errors:
1- one I introduce myself (without any help from anyone) where
smp_call_function() calls all online CPUs instead of calling all CPUs except
itself.
2- one in global_flush_tlb() which calls smp_call_function() with irqs disabled.
I think I should at least correct #1 ?
Laurent
--
- [EMAIL PROTECTED] --
"Software is hard" - Donald Knuth
signature.asc
Description: OpenPGP digital signature
Re: WARNING: at arch/x86_64/kernel/smp.c:397 smp_call_function_mask()
On Fri, 28 Sep 2007 10:52:08 +0200 Laurent Vivier <[EMAIL PROTECTED]> wrote:
> Fengguang Wu wrote:
> > On Thu, Sep 27, 2007 at 02:22:20AM -0700, Andrew Morton wrote:
> >> ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.23-rc8/2.6.23-rc8-mm2/
> >
> > Laurent,
> >
> > It triggered a WARNING on first run in qemu:
>
> Thank you to report it.
>
> >
> > [0.31] WARNING: at arch/x86_64/kernel/smp.c:397
> > smp_call_function_mask()
> > [0.31]
> > [0.31] Call Trace:
> > [0.31] [] dump_trace+0x3ee/0x4a0
> > [0.31] [] show_trace+0x43/0x70
> > [0.31] [] dump_stack+0x15/0x20
> > [0.31] [] smp_call_function_mask+0x94/0xa0
> > [0.31] [] smp_call_function+0x19/0x20
> > [0.31] [] on_each_cpu+0x1f/0x50
> > [0.31] [] global_flush_tlb+0x8c/0x110
> > [0.31] [] free_init_pages+0xe5/0xf0
> > [0.31] [] alternative_instructions+0x7e/0x150
> > [0.31] [] check_bugs+0x1a/0x20
> > [0.31] [] start_kernel+0x2da/0x380
> > [0.31] [] _sinittext+0x132/0x140
>
>
> the reason is the WARN_ON():
>
> 390 int smp_call_function_mask(cpumask_t mask,
> 391void (*func)(void *), void *info,
> 392int wait)
> 393 {
> 394 int ret;
> 395
> 396 /* Can deadlock when called with interrupts disabled */
> 397 WARN_ON(irqs_disabled());
> 398
> 399 spin_lock(_lock);
> 400 ret = __smp_call_function_mask(mask, func, info, wait);
> 401 spin_unlock(_lock);
> 402 return ret;
> 403 }
>
> The patch I sent to Andi didn't include this WARN_ON() and it's why I didn't
> find this issue. (see http://lkml.org/lkml/2007/8/24/101)
>
> smp_call_function_mask() is called by smp_call_function() which calls a
> function
> on all CPU except current.
> The comment of smp_call_function() specifies:
> ...
> * You must not call this function with disabled interrupts or from a
> * hardware interrupt handler or from a bottom half handler.
> * Actually there are a few legal cases, like panic.
> */
>
> So this WARN_ON() is correct, and the caller (global_flush_tlb()) doesn't
> follow
> this rule.
>
> I guess this WARN_ON() is only needed when we have current CPU in provided
> mask.
> So I think we should change:
>
> int smp_call_function (void (*func) (void *info), void *info, int nonatomic,
> int wait)
> {
> return smp_call_function_mask(cpu_online_map, func, info, wait);
> }
> ("cpu_online_map" is a bad choice, comment also specifies: "run a function on
> all other CPU")
>
> to
>
> int smp_call_function (void (*func) (void *info), void *info, int nonatomic,
> int wait)
> {
> int ret;
> cpumask_t allbutself;
>
> allbutself = cpu_online_map;
> cpu_clear(smp_processor_id(), allbutself);
>
> spin_lock(_lock);
> ret = __smp_call_function_mask(allbutself, func, info, wait);
> spin_unlock(_lock);
> return ret;
> }
> (which is smp_call_function_mask() without the WARN_ON() and without current
> cpu
> in the mask)
>
> Andi, is this correct ?
> Andrew, should I send a patch implementing this change ?
umm, I think all the smp_call_function fucntions are deadlocky if called
with local interrupts disabled, regardless of whether the calling CPU is in
the mask.
If CPU A is sending a cross-cpu call to CPU B and CPU B is sending a
cross-cpu call to CPU A, and they both have local interrupts disabled...
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: WARNING: at arch/x86_64/kernel/smp.c:397 smp_call_function_mask()
Fengguang Wu wrote:
> On Thu, Sep 27, 2007 at 02:22:20AM -0700, Andrew Morton wrote:
>> ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.23-rc8/2.6.23-rc8-mm2/
>
> Laurent,
>
> It triggered a WARNING on first run in qemu:
Thank you to report it.
>
> [ 0.310000] WARNING: at arch/x86_64/kernel/smp.c:397
> smp_call_function_mask()
> [0.31]
> [0.31] Call Trace:
> [0.31] [] dump_trace+0x3ee/0x4a0
> [0.31] [] show_trace+0x43/0x70
> [0.31] [] dump_stack+0x15/0x20
> [0.31] [] smp_call_function_mask+0x94/0xa0
> [0.31] [] smp_call_function+0x19/0x20
> [0.31] [] on_each_cpu+0x1f/0x50
> [0.31] [] global_flush_tlb+0x8c/0x110
> [0.31] [] free_init_pages+0xe5/0xf0
> [0.31] [] alternative_instructions+0x7e/0x150
> [0.31] [] check_bugs+0x1a/0x20
> [0.31] [] start_kernel+0x2da/0x380
> [0.31] [] _sinittext+0x132/0x140
the reason is the WARN_ON():
390 int smp_call_function_mask(cpumask_t mask,
391void (*func)(void *), void *info,
392int wait)
393 {
394 int ret;
395
396 /* Can deadlock when called with interrupts disabled */
397 WARN_ON(irqs_disabled());
398
399 spin_lock(_lock);
400 ret = __smp_call_function_mask(mask, func, info, wait);
401 spin_unlock(_lock);
402 return ret;
403 }
The patch I sent to Andi didn't include this WARN_ON() and it's why I didn't
find this issue. (see http://lkml.org/lkml/2007/8/24/101)
smp_call_function_mask() is called by smp_call_function() which calls a function
on all CPU except current.
The comment of smp_call_function() specifies:
...
* You must not call this function with disabled interrupts or from a
* hardware interrupt handler or from a bottom half handler.
* Actually there are a few legal cases, like panic.
*/
So this WARN_ON() is correct, and the caller (global_flush_tlb()) doesn't follow
this rule.
I guess this WARN_ON() is only needed when we have current CPU in provided mask.
So I think we should change:
int smp_call_function (void (*func) (void *info), void *info, int nonatomic,
int wait)
{
return smp_call_function_mask(cpu_online_map, func, info, wait);
}
("cpu_online_map" is a bad choice, comment also specifies: "run a function on
all other CPU")
to
int smp_call_function (void (*func) (void *info), void *info, int nonatomic,
int wait)
{
int ret;
cpumask_t allbutself;
allbutself = cpu_online_map;
cpu_clear(smp_processor_id(), allbutself);
spin_lock(_lock);
ret = __smp_call_function_mask(allbutself, func, info, wait);
spin_unlock(_lock);
return ret;
}
(which is smp_call_function_mask() without the WARN_ON() and without current cpu
in the mask)
Andi, is this correct ?
Andrew, should I send a patch implementing this change ?
Regards,
Laurent
--
- [EMAIL PROTECTED] --
"Software is hard" - Donald Knuth
signature.asc
Description: OpenPGP digital signature
Re: WARNING: at arch/x86_64/kernel/smp.c:397 smp_call_function_mask()
Andrew Morton wrote:
On Fri, 28 Sep 2007 10:52:08 +0200 Laurent Vivier [EMAIL PROTECTED] wrote:
Fengguang Wu wrote:
On Thu, Sep 27, 2007 at 02:22:20AM -0700, Andrew Morton wrote:
ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.23-rc8/2.6.23-rc8-mm2/
Laurent,
It triggered a WARNING on first run in qemu:
Thank you to report it.
[0.31] WARNING: at arch/x86_64/kernel/smp.c:397
smp_call_function_mask()
[0.31]
[0.31] Call Trace:
[0.31] [8100dbde] dump_trace+0x3ee/0x4a0
[0.31] [8100dcd3] show_trace+0x43/0x70
[0.31] [8100dd15] dump_stack+0x15/0x20
[0.31] [8101cd44] smp_call_function_mask+0x94/0xa0
[0.31] [8101cd69] smp_call_function+0x19/0x20
[0.31] [8104277f] on_each_cpu+0x1f/0x50
[0.31] [81026eac] global_flush_tlb+0x8c/0x110
[0.31] [81025c85] free_init_pages+0xe5/0xf0
[0.31] [81549b5e] alternative_instructions+0x7e/0x150
[0.31] [8154a2ea] check_bugs+0x1a/0x20
[0.31] [81540c4a] start_kernel+0x2da/0x380
[0.31] [81540132] _sinittext+0x132/0x140
the reason is the WARN_ON():
390 int smp_call_function_mask(cpumask_t mask,
391void (*func)(void *), void *info,
392int wait)
393 {
394 int ret;
395
396 /* Can deadlock when called with interrupts disabled */
397 WARN_ON(irqs_disabled());
398
399 spin_lock(call_lock);
400 ret = __smp_call_function_mask(mask, func, info, wait);
401 spin_unlock(call_lock);
402 return ret;
403 }
The patch I sent to Andi didn't include this WARN_ON() and it's why I didn't
find this issue. (see http://lkml.org/lkml/2007/8/24/101)
smp_call_function_mask() is called by smp_call_function() which calls a
function
on all CPU except current.
The comment of smp_call_function() specifies:
...
* You must not call this function with disabled interrupts or from a
* hardware interrupt handler or from a bottom half handler.
* Actually there are a few legal cases, like panic.
*/
So this WARN_ON() is correct, and the caller (global_flush_tlb()) doesn't
follow
this rule.
I guess this WARN_ON() is only needed when we have current CPU in provided
mask.
So I think we should change:
int smp_call_function (void (*func) (void *info), void *info, int nonatomic,
int wait)
{
return smp_call_function_mask(cpu_online_map, func, info, wait);
}
(cpu_online_map is a bad choice, comment also specifies: run a function on
all other CPU)
to
int smp_call_function (void (*func) (void *info), void *info, int nonatomic,
int wait)
{
int ret;
cpumask_t allbutself;
allbutself = cpu_online_map;
cpu_clear(smp_processor_id(), allbutself);
spin_lock(call_lock);
ret = __smp_call_function_mask(allbutself, func, info, wait);
spin_unlock(call_lock);
return ret;
}
(which is smp_call_function_mask() without the WARN_ON() and without current
cpu
in the mask)
Andi, is this correct ?
Andrew, should I send a patch implementing this change ?
umm, I think all the smp_call_function fucntions are deadlocky if called
with local interrupts disabled, regardless of whether the calling CPU is in
the mask.
If CPU A is sending a cross-cpu call to CPU B and CPU B is sending a
cross-cpu call to CPU A, and they both have local interrupts disabled...
OK, so there are two errors:
1- one I introduce myself (without any help from anyone) where
smp_call_function() calls all online CPUs instead of calling all CPUs except
itself.
2- one in global_flush_tlb() which calls smp_call_function() with irqs disabled.
I think I should at least correct #1 ?
Laurent
--
- [EMAIL PROTECTED] --
Software is hard - Donald Knuth
signature.asc
Description: OpenPGP digital signature
Re: WARNING: at arch/x86_64/kernel/smp.c:397 smp_call_function_mask()
Andrew Morton wrote: On Fri, 28 Sep 2007 11:18:45 +0200 Laurent Vivier [EMAIL PROTECTED] wrote: Andrew Morton wrote: On Fri, 28 Sep 2007 10:52:08 +0200 Laurent Vivier [EMAIL PROTECTED] wrote: Andi, is this correct ? Andrew, should I send a patch implementing this change ? umm, I think all the smp_call_function fucntions are deadlocky if called with local interrupts disabled, regardless of whether the calling CPU is in the mask. If CPU A is sending a cross-cpu call to CPU B and CPU B is sending a cross-cpu call to CPU A, and they both have local interrupts disabled... OK, so there are two errors: 1- one I introduce myself (without any help from anyone) where smp_call_function() calls all online CPUs instead of calling all CPUs except itself. I'd be pretty surprised if one was able to write a bug like that. You mean the CPU sends an IPI to itself and then loops around until it has serviced that IPI? And this works? Wow. In fact, it works because __smp_call_function_mask() makes : cpus_and(mask, mask, allbutself); So it removes current cpu from the mask. BTW, I don't have to modify smp_call_function(): it is correct as it is written (except from a semantic point of view... do we care about semantic ?). And on_each_cpu() can call the handler function twice? hm 2- one in global_flush_tlb() which calls smp_call_function() with irqs disabled. That would be a big bug, and surely we would already have picked it up. looks argh, mainline's x86_64 smp_call_function() doesn't do the check. We've had *heaps* of bugs in i386 where people were running smp_call_foo() under local_irq_disable(). I wonder how many there are in x86_64? I think I should at least correct #1 ? I think we should correct all bugs ;) We don't live in a perfect world... ;-) Moreover, I'm not able to reproduce #2 ... Fengguang could you send me your .config and your qemu command line parameters ? Laurent PS: if semantic is important, you can apply attached patch... -- - [EMAIL PROTECTED] -- Software is hard - Donald Knuth 0001-smp_call_function-call-function-on-all-CPUs.patch Description: application/mbox signature.asc Description: OpenPGP digital signature
Re: WARNING: at arch/x86_64/kernel/smp.c:397 smp_call_function_mask()
Fengguang Wu wrote:
On Thu, Sep 27, 2007 at 02:22:20AM -0700, Andrew Morton wrote:
ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.23-rc8/2.6.23-rc8-mm2/
Laurent,
It triggered a WARNING on first run in qemu:
Thank you to report it.
[0.31] WARNING: at arch/x86_64/kernel/smp.c:397
smp_call_function_mask()
[0.31]
[0.31] Call Trace:
[0.31] [8100dbde] dump_trace+0x3ee/0x4a0
[0.31] [8100dcd3] show_trace+0x43/0x70
[0.31] [8100dd15] dump_stack+0x15/0x20
[0.31] [8101cd44] smp_call_function_mask+0x94/0xa0
[0.31] [8101cd69] smp_call_function+0x19/0x20
[0.31] [8104277f] on_each_cpu+0x1f/0x50
[0.31] [81026eac] global_flush_tlb+0x8c/0x110
[0.31] [81025c85] free_init_pages+0xe5/0xf0
[0.31] [81549b5e] alternative_instructions+0x7e/0x150
[0.31] [8154a2ea] check_bugs+0x1a/0x20
[0.31] [81540c4a] start_kernel+0x2da/0x380
[0.31] [81540132] _sinittext+0x132/0x140
the reason is the WARN_ON():
390 int smp_call_function_mask(cpumask_t mask,
391void (*func)(void *), void *info,
392int wait)
393 {
394 int ret;
395
396 /* Can deadlock when called with interrupts disabled */
397 WARN_ON(irqs_disabled());
398
399 spin_lock(call_lock);
400 ret = __smp_call_function_mask(mask, func, info, wait);
401 spin_unlock(call_lock);
402 return ret;
403 }
The patch I sent to Andi didn't include this WARN_ON() and it's why I didn't
find this issue. (see http://lkml.org/lkml/2007/8/24/101)
smp_call_function_mask() is called by smp_call_function() which calls a function
on all CPU except current.
The comment of smp_call_function() specifies:
...
* You must not call this function with disabled interrupts or from a
* hardware interrupt handler or from a bottom half handler.
* Actually there are a few legal cases, like panic.
*/
So this WARN_ON() is correct, and the caller (global_flush_tlb()) doesn't follow
this rule.
I guess this WARN_ON() is only needed when we have current CPU in provided mask.
So I think we should change:
int smp_call_function (void (*func) (void *info), void *info, int nonatomic,
int wait)
{
return smp_call_function_mask(cpu_online_map, func, info, wait);
}
(cpu_online_map is a bad choice, comment also specifies: run a function on
all other CPU)
to
int smp_call_function (void (*func) (void *info), void *info, int nonatomic,
int wait)
{
int ret;
cpumask_t allbutself;
allbutself = cpu_online_map;
cpu_clear(smp_processor_id(), allbutself);
spin_lock(call_lock);
ret = __smp_call_function_mask(allbutself, func, info, wait);
spin_unlock(call_lock);
return ret;
}
(which is smp_call_function_mask() without the WARN_ON() and without current cpu
in the mask)
Andi, is this correct ?
Andrew, should I send a patch implementing this change ?
Regards,
Laurent
--
- [EMAIL PROTECTED] --
Software is hard - Donald Knuth
signature.asc
Description: OpenPGP digital signature
Re: WARNING: at arch/x86_64/kernel/smp.c:397 smp_call_function_mask()
On Fri, 28 Sep 2007 10:52:08 +0200 Laurent Vivier [EMAIL PROTECTED] wrote:
Fengguang Wu wrote:
On Thu, Sep 27, 2007 at 02:22:20AM -0700, Andrew Morton wrote:
ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.23-rc8/2.6.23-rc8-mm2/
Laurent,
It triggered a WARNING on first run in qemu:
Thank you to report it.
[0.31] WARNING: at arch/x86_64/kernel/smp.c:397
smp_call_function_mask()
[0.31]
[0.31] Call Trace:
[0.31] [8100dbde] dump_trace+0x3ee/0x4a0
[0.31] [8100dcd3] show_trace+0x43/0x70
[0.31] [8100dd15] dump_stack+0x15/0x20
[0.31] [8101cd44] smp_call_function_mask+0x94/0xa0
[0.31] [8101cd69] smp_call_function+0x19/0x20
[0.31] [8104277f] on_each_cpu+0x1f/0x50
[0.31] [81026eac] global_flush_tlb+0x8c/0x110
[0.31] [81025c85] free_init_pages+0xe5/0xf0
[0.31] [81549b5e] alternative_instructions+0x7e/0x150
[0.31] [8154a2ea] check_bugs+0x1a/0x20
[0.31] [81540c4a] start_kernel+0x2da/0x380
[0.31] [81540132] _sinittext+0x132/0x140
the reason is the WARN_ON():
390 int smp_call_function_mask(cpumask_t mask,
391void (*func)(void *), void *info,
392int wait)
393 {
394 int ret;
395
396 /* Can deadlock when called with interrupts disabled */
397 WARN_ON(irqs_disabled());
398
399 spin_lock(call_lock);
400 ret = __smp_call_function_mask(mask, func, info, wait);
401 spin_unlock(call_lock);
402 return ret;
403 }
The patch I sent to Andi didn't include this WARN_ON() and it's why I didn't
find this issue. (see http://lkml.org/lkml/2007/8/24/101)
smp_call_function_mask() is called by smp_call_function() which calls a
function
on all CPU except current.
The comment of smp_call_function() specifies:
...
* You must not call this function with disabled interrupts or from a
* hardware interrupt handler or from a bottom half handler.
* Actually there are a few legal cases, like panic.
*/
So this WARN_ON() is correct, and the caller (global_flush_tlb()) doesn't
follow
this rule.
I guess this WARN_ON() is only needed when we have current CPU in provided
mask.
So I think we should change:
int smp_call_function (void (*func) (void *info), void *info, int nonatomic,
int wait)
{
return smp_call_function_mask(cpu_online_map, func, info, wait);
}
(cpu_online_map is a bad choice, comment also specifies: run a function on
all other CPU)
to
int smp_call_function (void (*func) (void *info), void *info, int nonatomic,
int wait)
{
int ret;
cpumask_t allbutself;
allbutself = cpu_online_map;
cpu_clear(smp_processor_id(), allbutself);
spin_lock(call_lock);
ret = __smp_call_function_mask(allbutself, func, info, wait);
spin_unlock(call_lock);
return ret;
}
(which is smp_call_function_mask() without the WARN_ON() and without current
cpu
in the mask)
Andi, is this correct ?
Andrew, should I send a patch implementing this change ?
umm, I think all the smp_call_function fucntions are deadlocky if called
with local interrupts disabled, regardless of whether the calling CPU is in
the mask.
If CPU A is sending a cross-cpu call to CPU B and CPU B is sending a
cross-cpu call to CPU A, and they both have local interrupts disabled...
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: WARNING: at arch/x86_64/kernel/smp.c:397 smp_call_function_mask()
On Fri, 28 Sep 2007 11:18:45 +0200 Laurent Vivier [EMAIL PROTECTED] wrote: Andrew Morton wrote: On Fri, 28 Sep 2007 10:52:08 +0200 Laurent Vivier [EMAIL PROTECTED] wrote: Andi, is this correct ? Andrew, should I send a patch implementing this change ? umm, I think all the smp_call_function fucntions are deadlocky if called with local interrupts disabled, regardless of whether the calling CPU is in the mask. If CPU A is sending a cross-cpu call to CPU B and CPU B is sending a cross-cpu call to CPU A, and they both have local interrupts disabled... OK, so there are two errors: 1- one I introduce myself (without any help from anyone) where smp_call_function() calls all online CPUs instead of calling all CPUs except itself. I'd be pretty surprised if one was able to write a bug like that. You mean the CPU sends an IPI to itself and then loops around until it has serviced that IPI? And this works? Wow. And on_each_cpu() can call the handler function twice? hm 2- one in global_flush_tlb() which calls smp_call_function() with irqs disabled. That would be a big bug, and surely we would already have picked it up. looks argh, mainline's x86_64 smp_call_function() doesn't do the check. We've had *heaps* of bugs in i386 where people were running smp_call_foo() under local_irq_disable(). I wonder how many there are in x86_64? I think I should at least correct #1 ? I think we should correct all bugs ;) - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
WARNING: at arch/x86_64/kernel/smp.c:397 smp_call_function_mask()
On Thu, Sep 27, 2007 at 02:22:20AM -0700, Andrew Morton wrote: > > ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.23-rc8/2.6.23-rc8-mm2/ Laurent, It triggered a WARNING on first run in qemu: [0.31] WARNING: at arch/x86_64/kernel/smp.c:397 smp_call_function_mask() [0.31] [0.31] Call Trace: [0.31] [] dump_trace+0x3ee/0x4a0 [0.31] [] show_trace+0x43/0x70 [0.31] [] dump_stack+0x15/0x20 [0.31] [] smp_call_function_mask+0x94/0xa0 [0.31] [] smp_call_function+0x19/0x20 [0.31] [] on_each_cpu+0x1f/0x50 [0.31] [] global_flush_tlb+0x8c/0x110 [0.31] [] free_init_pages+0xe5/0xf0 [0.31] [] alternative_instructions+0x7e/0x150 [0.31] [] check_bugs+0x1a/0x20 [0.31] [] start_kernel+0x2da/0x380 [0.31] [] _sinittext+0x132/0x140 Here is the more complete log: [0.00] Linux version 2.6.23-rc8-mm2 ([EMAIL PROTECTED]) (gcc version 4.2.1 (Debian 4.2.1-5)) #3 SMP Fri Sep 28 10:29:34 CST 2007 [0.00] Command line: root=/dev/hda rw console=ttyS0 clock=pit init=/bin/bash [0.00] BIOS-provided physical RAM map: [0.00] BIOS-e820: - 0009fc00 (usable) [0.00] BIOS-e820: 0009fc00 - 000a (reserved) [0.00] BIOS-e820: 000e8000 - 0010 (reserved) [0.00] BIOS-e820: 0010 - 39ff (usable) [0.00] BIOS-e820: 39ff - 3a00 (ACPI data) [0.00] BIOS-e820: fffc - 0001 (reserved) [0.00] end_pfn_map = 1048576 [0.00] DMI not present or invalid. [0.00] ACPI: RSDP 000FAA30, 0014 (r0 BOCHS ) [0.00] ACPI: RSDT 39FF, 002C (r0 BOCHS BXPCRSDT1 BXPC 1) [0.00] ACPI: FACP 39FF002C, 0074 (r0 BOCHS BXPCFACP1 BXPC 1) [0.00] ACPI: DSDT 39FF0100, 0832 (r1 BXPC BXDSDT1 INTL 20060912) [0.00] ACPI: FACS 39FF00C0, 0040 [0.00] ACPI: APIC 39FF0938, 0040 (r0 BOCHS BXPCAPIC1 BXPC 1) [0.00] No NUMA configuration found [0.00] Faking a node at -39ff [0.00] Bootmem setup node 0 -39ff [0.00] Zone PFN ranges: [0.00] DMA 0 -> 4096 [0.00] DMA324096 -> 1048576 [0.00] Normal1048576 -> 1048576 [0.00] Movable zone start PFN for each node [0.00] early_node_map[2] active PFN ranges [0.00] 0:0 -> 159 [0.00] 0: 256 -> 237552 [0.00] ACPI: PM-Timer IO Port: 0xb008 [0.00] ACPI: LAPIC (acpi_id[0x00] lapic_id[0x00] enabled) [0.00] Processor #0 (Bootup-CPU) [0.00] ACPI: IOAPIC (id[0x01] address[0xfec0] gsi_base[0]) [0.00] IOAPIC[0]: apic_id 1, address 0xfec0, GSI 0-23 [0.00] Setting APIC routing to flat [0.00] Using ACPI (MADT) for SMP configuration information [0.00] Allocating PCI resources starting at 4000 (gap: 3a00:c5fc) [0.00] .eh_frame_hdr for 'kernel' present but unusable [0.00] SMP: Allowing 1 CPUs, 0 hotplug CPUs [0.00] PERCPU: Allocating 429480 bytes of per cpu data [0.00] Built 1 zonelists in Node order, mobility grouping on. Total pages: 231879 [0.00] Policy zone: DMA32 [0.00] Kernel command line: root=/dev/hda rw console=ttyS0 clock=pit init=/bin/bash [0.00] Warning! clock= boot option is deprecated. Use clocksource=xyz [0.00] Initializing CPU#0 [0.00] PID hash table entries: 4096 (order: 12, 32768 bytes) [0.00] TSC calibrated against PM_TIMER [0.00] time.c: Detected 2932.892 MHz processor. [0.02] console [kgdb0] enabled [0.03] Console: colour VGA+ 80x25 [0.04] console [ttyS0] enabled [0.05] Lock dependency validator: Copyright (c) 2006 Red Hat, Inc., Ingo Molnar [0.05] ... MAX_LOCKDEP_SUBCLASSES:8 [0.05] ... MAX_LOCK_DEPTH: 30 [0.05] ... MAX_LOCKDEP_KEYS:2048 [0.05] ... CLASSHASH_SIZE: 1024 [0.05] ... MAX_LOCKDEP_ENTRIES: 8192 [0.05] ... MAX_LOCKDEP_CHAINS: 16384 [0.05] ... CHAINHASH_SIZE: 8192 [0.05] memory used by lock dependency info: 1712 kB [0.05] per task-struct memory footprint: 2160 bytes [0.05] Checking aperture... [0.10] Memory: 905832k/950208k available (3018k kernel code, 43988k reserved, 2171k data, 720k init) [0.10] SLUB: Genslabs=12, HWalign=64, Order=0-3, MinObjects=16, CPUs=1, Nodes=1 [0.25] Calibrating delay using timer specific routine.. 5880.64 BogoMIPS (lpj=29403242) [0.25] kswapd reclaim order set to 3 [0.25] Security Framework initialized [0.25] SELinux: Initia
WARNING: at arch/x86_64/kernel/smp.c:397 smp_call_function_mask()
On Thu, Sep 27, 2007 at 02:22:20AM -0700, Andrew Morton wrote: ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.23-rc8/2.6.23-rc8-mm2/ Laurent, It triggered a WARNING on first run in qemu: [0.31] WARNING: at arch/x86_64/kernel/smp.c:397 smp_call_function_mask() [0.31] [0.31] Call Trace: [0.31] [8100dbde] dump_trace+0x3ee/0x4a0 [0.31] [8100dcd3] show_trace+0x43/0x70 [0.31] [8100dd15] dump_stack+0x15/0x20 [0.31] [8101cd44] smp_call_function_mask+0x94/0xa0 [0.31] [8101cd69] smp_call_function+0x19/0x20 [0.31] [8104277f] on_each_cpu+0x1f/0x50 [0.31] [81026eac] global_flush_tlb+0x8c/0x110 [0.31] [81025c85] free_init_pages+0xe5/0xf0 [0.31] [81549b5e] alternative_instructions+0x7e/0x150 [0.31] [8154a2ea] check_bugs+0x1a/0x20 [0.31] [81540c4a] start_kernel+0x2da/0x380 [0.31] [81540132] _sinittext+0x132/0x140 Here is the more complete log: [0.00] Linux version 2.6.23-rc8-mm2 ([EMAIL PROTECTED]) (gcc version 4.2.1 (Debian 4.2.1-5)) #3 SMP Fri Sep 28 10:29:34 CST 2007 [0.00] Command line: root=/dev/hda rw console=ttyS0 clock=pit init=/bin/bash [0.00] BIOS-provided physical RAM map: [0.00] BIOS-e820: - 0009fc00 (usable) [0.00] BIOS-e820: 0009fc00 - 000a (reserved) [0.00] BIOS-e820: 000e8000 - 0010 (reserved) [0.00] BIOS-e820: 0010 - 39ff (usable) [0.00] BIOS-e820: 39ff - 3a00 (ACPI data) [0.00] BIOS-e820: fffc - 0001 (reserved) [0.00] end_pfn_map = 1048576 [0.00] DMI not present or invalid. [0.00] ACPI: RSDP 000FAA30, 0014 (r0 BOCHS ) [0.00] ACPI: RSDT 39FF, 002C (r0 BOCHS BXPCRSDT1 BXPC 1) [0.00] ACPI: FACP 39FF002C, 0074 (r0 BOCHS BXPCFACP1 BXPC 1) [0.00] ACPI: DSDT 39FF0100, 0832 (r1 BXPC BXDSDT1 INTL 20060912) [0.00] ACPI: FACS 39FF00C0, 0040 [0.00] ACPI: APIC 39FF0938, 0040 (r0 BOCHS BXPCAPIC1 BXPC 1) [0.00] No NUMA configuration found [0.00] Faking a node at -39ff [0.00] Bootmem setup node 0 -39ff [0.00] Zone PFN ranges: [0.00] DMA 0 - 4096 [0.00] DMA324096 - 1048576 [0.00] Normal1048576 - 1048576 [0.00] Movable zone start PFN for each node [0.00] early_node_map[2] active PFN ranges [0.00] 0:0 - 159 [0.00] 0: 256 - 237552 [0.00] ACPI: PM-Timer IO Port: 0xb008 [0.00] ACPI: LAPIC (acpi_id[0x00] lapic_id[0x00] enabled) [0.00] Processor #0 (Bootup-CPU) [0.00] ACPI: IOAPIC (id[0x01] address[0xfec0] gsi_base[0]) [0.00] IOAPIC[0]: apic_id 1, address 0xfec0, GSI 0-23 [0.00] Setting APIC routing to flat [0.00] Using ACPI (MADT) for SMP configuration information [0.00] Allocating PCI resources starting at 4000 (gap: 3a00:c5fc) [0.00] .eh_frame_hdr for 'kernel' present but unusable [0.00] SMP: Allowing 1 CPUs, 0 hotplug CPUs [0.00] PERCPU: Allocating 429480 bytes of per cpu data [0.00] Built 1 zonelists in Node order, mobility grouping on. Total pages: 231879 [0.00] Policy zone: DMA32 [0.00] Kernel command line: root=/dev/hda rw console=ttyS0 clock=pit init=/bin/bash [0.00] Warning! clock= boot option is deprecated. Use clocksource=xyz [0.00] Initializing CPU#0 [0.00] PID hash table entries: 4096 (order: 12, 32768 bytes) [0.00] TSC calibrated against PM_TIMER [0.00] time.c: Detected 2932.892 MHz processor. [0.02] console [kgdb0] enabled [0.03] Console: colour VGA+ 80x25 [0.04] console [ttyS0] enabled [0.05] Lock dependency validator: Copyright (c) 2006 Red Hat, Inc., Ingo Molnar [0.05] ... MAX_LOCKDEP_SUBCLASSES:8 [0.05] ... MAX_LOCK_DEPTH: 30 [0.05] ... MAX_LOCKDEP_KEYS:2048 [0.05] ... CLASSHASH_SIZE: 1024 [0.05] ... MAX_LOCKDEP_ENTRIES: 8192 [0.05] ... MAX_LOCKDEP_CHAINS: 16384 [0.05] ... CHAINHASH_SIZE: 8192 [0.05] memory used by lock dependency info: 1712 kB [0.05] per task-struct memory footprint: 2160 bytes [0.05] Checking aperture... [0.10] Memory: 905832k/950208k available (3018k kernel code, 43988k reserved, 2171k data, 720k init) [0.10] SLUB: Genslabs=12, HWalign=64, Order=0-3, MinObjects=16, CPUs=1, Nodes=1 [0.25] Calibrating delay using timer specific
