[2.5] fix preempt-issues with smp_call_function()
Subsystems (6)
PATCH 1/1 [2.5] fix preempt-issues with smp_call_function()
Hello,
here is a patch to solve all (I hope I missed none) possible problems that could occur on SMP machines running a preemptible kernel when smp_call_function() calls a function which should be also executed on the current processor.
This patch is based on the one Dave Jones sent to the LKML last friday and applies to the linux kernel version 2.5.63.
Thank you for any response...
Thomas Schlichter
arch/alpha/kernel/process.c +1 -4
@@ -155,10 +155,7 @@struct halt_info args; args.mode = mode; args.restart_cmd = restart_cmd; -#ifdef CONFIG_SMP -smp_call_function (common_shutdown_1, &args,1 ,0 ); -#endif -common_shutdown_1 (&args); +on_each_cpu (common_shutdown_1, &args,1 ,0 ); }void
arch/alpha/kernel/smp.c +17 -6
@@ -899,10 +899,8 @@smp_imb (void) {/* Must wait other processors to flush their icache before continue. */ -if (smp_call_function (ipi_imb,NULL ,1 ,1 )) +if (on_each_cpu (ipi_imb,NULL ,1 ,1 ))printk (KERN_CRIT "smp_imb: timed out\n" ); - -imb (); } static void
@@ -916,11 +914,9 @@ {/* Although we don't have any data to pass, we do want to synchronize with the other processors. */ -if (smp_call_function (ipi_flush_tlb_all,NULL ,1 ,1 )) { +if (on_each_cpu (ipi_flush_tlb_all,NULL ,1 ,1 )) {printk (KERN_CRIT "flush_tlb_all: timed out\n" ); } - -tbia (); }#define asn_locked () (cpu_data[smp_processor_id()].asn_lock)
@@ -938,6 +934,8 @@void flush_tlb_mm (struct mm_struct *mm) { +preempt_disable (); +if (mm == current->active_mm) {flush_tlb_current (mm);if (atomic_read (&mm->mm_users) <=1 ) {
@@ -948,6 +946,7 @@if (mm->context[cpu]) mm->context[cpu] =0 ; } +preempt_enable ();return ; } }
@@ -955,6 +954,8 @@if (smp_call_function (ipi_flush_tlb_mm, mm,1 ,1 )) {printk (KERN_CRIT "flush_tlb_mm: timed out\n" ); } + +preempt_enable (); }struct flush_tlb_page_struct {
@@ -981,6 +982,8 @@struct flush_tlb_page_struct data;struct mm_struct *mm = vma->vm_mm; +preempt_disable (); +if (mm == current->active_mm) {flush_tlb_current_page (mm, vma, addr);if (atomic_read (&mm->mm_users) <=1 ) {
@@ -991,6 +994,7 @@if (mm->context[cpu]) mm->context[cpu] =0 ; } +preempt_enable ();return ; } }
@@ -1002,6 +1006,8 @@if (smp_call_function (ipi_flush_tlb_page, &data,1 ,1 )) {printk (KERN_CRIT "flush_tlb_page: timed out\n" ); } + +preempt_enable (); }void
@@ -1030,6 +1036,8 @@if ((vma->vm_flags &VM_EXEC ) ==0 )return ; +preempt_disable (); +if (mm == current->active_mm) {__load_new_mm_context (mm);if (atomic_read (&mm->mm_users) <=1 ) {
@@ -1040,6 +1048,7 @@if (mm->context[cpu]) mm->context[cpu] =0 ; } +preempt_enable ();return ; } }
@@ -1047,6 +1056,8 @@if (smp_call_function (ipi_flush_icache_page, mm,1 ,1 )) {printk (KERN_CRIT "flush_icache_page: timed out\n" ); } + +preempt_enable (); }#ifdef CONFIG_DEBUG_SPINLOCK
arch/i386/kernel/io_apic.c +2 -4
@@ -1376,8 +1376,7 @@void print_all_local_APICs (void ) { -smp_call_function (print_local_APIC,NULL ,1 ,1 ); -print_local_APIC (NULL ); +on_each_cpu (print_local_APIC,NULL ,1 ,1 ); }void /*__init*/ print_PIC (void )
@@ -1843,8 +1842,7 @@*/ printk (KERN_INFO "activating NMI Watchdog ..." ); -smp_call_function (enable_NMI_through_LVT0,NULL ,1 ,1 ); -enable_NMI_through_LVT0 (NULL ); +on_each_cpu (enable_NMI_through_LVT0,NULL ,1 ,1 );printk (" done.\n" ); }
arch/i386/kernel/ldt.c +2 -2
@@ -55,13 +55,13 @@wmb ();if (reload) { +preempt_disable ();load_LDT (pc);#ifdef CONFIG_SMP -preempt_disable ();if (current->mm->cpu_vm_mask != (1 <<smp_processor_id ()))smp_call_function (flush_ldt,0 ,1 ,1 ); -preempt_enable ();#endif +preempt_enable (); }if (oldsize) {if (oldsize*LDT_ENTRY_SIZE >PAGE_SIZE )
arch/i386/kernel/microcode.c +1 -2
@@ -183,11 +183,10 @@int i, error =0 , err;struct microcode *m; -if (smp_call_function (do_update_one,NULL ,1 ,1 ) !=0 ) { +if (on_each_cpu (do_update_one,NULL ,1 ,1 ) !=0 ) {printk (KERN_ERR "microcode: IPI timeout, giving up\n" );return -EIO ; } -do_update_one (NULL );for (i=0 ; i<NR_CPUS ; i++) { err = update_req[i].err;
arch/i386/kernel/smp.c +2 -11
@@ -436,7 +436,7 @@preempt_enable (); } -static inline void do_flush_tlb_all_local (void ) +static void do_flush_tlb_all (void * info) {unsigned long cpu =smp_processor_id ();
@@ -445,18 +445,9 @@leave_mm (cpu); } -static void flush_tlb_all_ipi (void * info) -{ -do_flush_tlb_all_local (); -} -void flush_tlb_all (void ) { -preempt_disable (); -smp_call_function (flush_tlb_all_ipi,0 ,1 ,1 ); - -do_flush_tlb_all_local (); -preempt_enable (); +on_each_cpu (do_flush_tlb_all,0 ,1 ,1 ); }/*
arch/i386/kernel/sysenter.c +1 -2
@@ -95,8 +95,7 @@return 0 ;memcpy ((void *) page, sysent,sizeof (sysent)); -enable_sep_cpu (NULL ); -smp_call_function (enable_sep_cpu,NULL ,1 ,1 ); +on_each_cpu (enable_sep_cpu,NULL ,1 ,1 );return 0 ; }
arch/i386/mach-voyager/voyager_smp.c +3 -11
@@ -1209,8 +1209,8 @@smp_call_function_interrupt (); } -static inline void -do_flush_tlb_all_local (void ) +static void +do_flush_tlb_all (void * info) {unsigned long cpu =smp_processor_id ();
@@ -1220,19 +1220,11 @@ } -static void -flush_tlb_all_function (void * info) -{ -do_flush_tlb_all_local (); -} -/* flush the TLB of every active CPU in the system */ void flush_tlb_all (void ) { -smp_call_function (flush_tlb_all_function,0 ,1 ,1 ); - -do_flush_tlb_all_local (); +on_each_cpu (do_flush_tlb_all,0 ,1 ,1 ); }/* used to set up the trampoline for other CPUs when the memory manager
arch/i386/mm/pageattr.c +2 -5
@@ -130,11 +130,8 @@ }static inline void flush_map (void ) -{ -#ifdef CONFIG_SMP -smp_call_function (flush_kernel_map,NULL ,1 ,1 ); -#endif -flush_kernel_map (NULL ); +{ +on_each_cpu (flush_kernel_map,NULL ,1 ,1 ); }struct deferred_page {
arch/i386/oprofile/nmi_int.c +4 -8
@@ -95,8 +95,7 @@* without actually triggering any NMIs as this will * break the core code horrifically. */ -smp_call_function (nmi_cpu_setup,NULL ,0 ,1 ); -nmi_cpu_setup (0 ); +on_each_cpu (nmi_cpu_setup,NULL ,0 ,1 );set_nmi_callback (nmi_callback); oprofile_pmdev =set_nmi_pm_callback (oprofile_pm_callback);return 0 ;
@@ -148,8 +147,7 @@ {unset_nmi_pm_callback (oprofile_pmdev);unset_nmi_callback (); -smp_call_function (nmi_cpu_shutdown,NULL ,0 ,1 ); -nmi_cpu_shutdown (0 ); +on_each_cpu (nmi_cpu_shutdown,NULL ,0 ,1 ); }
@@ -162,8 +160,7 @@static int nmi_start (void ) { -smp_call_function (nmi_cpu_start,NULL ,0 ,1 ); -nmi_cpu_start (0 ); +on_each_cpu (nmi_cpu_start,NULL ,0 ,1 );return 0 ; }
@@ -177,8 +174,7 @@static void nmi_stop (void ) { -smp_call_function (nmi_cpu_stop,NULL ,0 ,1 ); -nmi_cpu_stop (0 ); +on_each_cpu (nmi_cpu_stop,NULL ,0 ,1 ); }
arch/ia64/kernel/smp.c +5 -5
@@ -206,18 +206,18 @@void smp_flush_tlb_all (void ) { -smp_call_function ((void (*)(void *))local_flush_tlb_all,0 ,1 ,1 ); -local_flush_tlb_all (); +on_each_cpu ((void (*)(void *))local_flush_tlb_all,0 ,1 ,1 ); }void smp_flush_tlb_mm (struct mm_struct *mm) { -local_finish_flush_tlb_mm (mm); -/* this happens for the common case of a single-threaded fork(): */ if (likely (mm == current->active_mm &&atomic_read (&mm->mm_users) ==1 )) + { +local_finish_flush_tlb_mm (mm);return ; + }/* * We could optimize this further by using mm->cpu_vm_mask to track which CPUs
@@ -226,7 +226,7 @@* anyhow, and once a CPU is interrupted, the cost of local_flush_tlb_all() is * rather trivial. */ -smp_call_function ((void (*)(void *))local_finish_flush_tlb_mm, mm,1 ,1 ); +on_each_cpu ((void (*)(void *))local_finish_flush_tlb_mm, mm,1 ,1 ); }/*
arch/mips64/kernel/smp.c +13 -2
@@ -195,8 +195,7 @@void flush_tlb_all (void ) { -smp_call_function (flush_tlb_all_ipi,0 ,1 ,1 ); -_flush_tlb_all (); +on_each_cpu (flush_tlb_all_ipi,0 ,1 ,1 ); }static void flush_tlb_mm_ipi (void *mm)
@@ -219,6 +218,8 @@void flush_tlb_mm (struct mm_struct *mm) { +preempt_disable (); +if ((atomic_read (&mm->mm_users) !=1 ) || (current->mm != mm)) {smp_call_function (flush_tlb_mm_ipi, (void *)mm,1 ,1 ); }else {
@@ -228,6 +229,8 @@CPU_CONTEXT (i, mm) =0 ; }_flush_tlb_mm (mm); + +preempt_enable (); }struct flush_tlb_data {
@@ -246,6 +249,8 @@void flush_tlb_range (struct vm_area_struct *vma,unsigned long start,unsigned long end) { +preempt_disable (); +if ((atomic_read (&mm->mm_users) !=1 ) || (current->mm != mm)) {struct flush_tlb_data fd;
@@ -260,6 +265,8 @@CPU_CONTEXT (i, mm) =0 ; }_flush_tlb_range (mm, start, end); + +preempt_enable (); }static void flush_tlb_page_ipi (void *info)
@@ -271,6 +278,8 @@void flush_tlb_page (struct vm_area_struct *vma,unsigned long page) { +preempt_disable (); +if ((atomic_read (&vma->vm_mm->mm_users) !=1 ) || (current->mm != vma->vm_mm)) {struct flush_tlb_data fd;
@@ -284,5 +293,7 @@CPU_CONTEXT (i, vma->vm_mm) =0 ; }_flush_tlb_page (vma, page); + +preempt_enable (); }
arch/parisc/kernel/cache.c +1 -2
@@ -39,8 +39,7 @@void flush_data_cache (void ) { -smp_call_function ((void (*)(void *))flush_data_cache_local,NULL ,1 ,1 ); -flush_data_cache_local (); +on_each_cpu ((void (*)(void *))flush_data_cache_local,NULL ,1 ,1 ); }#endif
arch/parisc/kernel/irq.c +3 -8
@@ -61,20 +61,17 @@static spinlock_t irq_lock =SPIN_LOCK_UNLOCKED ;/* protect IRQ regions */ -#ifdef CONFIG_SMP static void cpu_set_eiem (void *info) {set_eiem ((unsigned long ) info); } -#endif static inline void disable_cpu_irq (void *unused,int irq) {unsigned long eirr_bit =EIEM_MASK (irq); cpu_eiem &= ~eirr_bit; -set_eiem (cpu_eiem); -smp_call_function (cpu_set_eiem, (void *) cpu_eiem,1 ,1 ); +on_each_cpu (cpu_set_eiem, (void *) cpu_eiem,1 ,1 ); }static void enable_cpu_irq (void *unused,int irq)
@@ -83,8 +80,7 @@mtctl (eirr_bit,23 );/* clear EIRR bit before unmasking */ cpu_eiem |= eirr_bit; -smp_call_function (cpu_set_eiem, (void *) cpu_eiem,1 ,1 ); -set_eiem (cpu_eiem); +on_each_cpu (cpu_set_eiem, (void *) cpu_eiem,1 ,1 ); }/* mask and disable are the same at the CPU level
@@ -100,8 +96,7 @@** handle *any* unmasked pending interrupts. ** ie We don't need to check for pending interrupts here. */ -smp_call_function (cpu_set_eiem, (void *) cpu_eiem,1 ,1 ); -set_eiem (cpu_eiem); +on_each_cpu (cpu_set_eiem, (void *) cpu_eiem,1 ,1 ); }/*
arch/parisc/kernel/smp.c +2 -3
@@ -401,7 +401,7 @@__setup ("maxcpus=" , maxcpus);/* -* Flush all other CPU's tlb and then mine. Do this with smp_call_function() +* Flush all other CPU's tlb and then mine. Do this with on_each_cpu() * as we want to ensure all TLB's flushed before proceeding. */
@@ -410,8 +410,7 @@void smp_flush_tlb_all (void ) { -smp_call_function ((void (*)(void *))flush_tlb_all_local,NULL ,1 ,1 ); -flush_tlb_all_local (); +on_each_cpu ((void (*)(void *))flush_tlb_all_local,NULL ,1 ,1 ); }
arch/parisc/mm/init.c +1 -2
@@ -974,8 +974,7 @@ do_recycle++; }spin_unlock (&sid_lock); -smp_call_function ((void (*)(void *))flush_tlb_all_local,NULL ,1 ,1 ); -flush_tlb_all_local (); +on_each_cpu ((void (*)(void *))flush_tlb_all_local,NULL ,1 ,1 );if (do_recycle) {spin_lock (&sid_lock);recycle_sids (recycle_ndirty,recycle_dirty_array);
arch/ppc/kernel/temp.c +2 -8
@@ -194,10 +194,7 @@/* schedule ourselves to be run again */ mod_timer (&tau_timer, jiffies + shrink_timer) ; -#ifdef CONFIG_SMP -smp_call_function (tau_timeout,NULL ,1 ,0 ); -#endif -tau_timeout (NULL ); +on_each_cpu (tau_timeout,NULL ,1 ,0 ); }/*
@@ -239,10 +236,7 @@ tau_timer.expires = jiffies + shrink_timer;add_timer (&tau_timer); -#ifdef CONFIG_SMP -smp_call_function (TAU_init_smp,NULL ,1 ,0 ); -#endif -TAU_init_smp (NULL ); +on_each_cpu (TAU_init_smp,NULL ,1 ,0 );printk ("Thermal assist unit " );#ifdef CONFIG_TAU_INT
arch/s390/kernel/smp.c +6 -12
@@ -228,8 +228,7 @@void machine_restart_smp (char * __unused) { cpu_restart_map = cpu_online_map; -smp_call_function (do_machine_restart,NULL ,0 ,0 ); -do_machine_restart (NULL ); +on_each_cpu (do_machine_restart,NULL ,0 ,0 ); }static void do_machine_halt (void * __unused)
@@ -247,8 +246,7 @@void machine_halt_smp (void ) { -smp_call_function (do_machine_halt,NULL ,0 ,0 ); -do_machine_halt (NULL ); +on_each_cpu (do_machine_halt,NULL ,0 ,0 ); }static void do_machine_power_off (void * __unused)
@@ -266,8 +264,7 @@void machine_power_off_smp (void ) { -smp_call_function (do_machine_power_off,NULL ,0 ,0 ); -do_machine_power_off (NULL ); +on_each_cpu (do_machine_power_off,NULL ,0 ,0 ); }/*
@@ -339,8 +336,7 @@void smp_ptlb_all (void ) { -smp_call_function (smp_ptlb_callback,NULL ,0 ,1 ); -local_flush_tlb (); +on_each_cpu (smp_ptlb_callback,NULL ,0 ,1 ); }/*
@@ -400,8 +396,7 @@ parms.end_ctl = cr; parms.orvals[cr] =1 << bit; parms.andvals[cr] =0xFFFFFFFF ; -smp_call_function (smp_ctl_bit_callback, &parms,0 ,1 ); -__ctl_set_bit (cr, bit); +on_each_cpu (smp_ctl_bit_callback, &parms,0 ,1 ); }/*
@@ -414,8 +409,7 @@ parms.end_ctl = cr; parms.orvals[cr] =0x00000000 ; parms.andvals[cr] = ~(1 << bit); -smp_call_function (smp_ctl_bit_callback, &parms,0 ,1 ); -__ctl_clear_bit (cr, bit); +on_each_cpu (smp_ctl_bit_callback, &parms,0 ,1 ); }/*
arch/s390x/kernel/smp.c +5 -10
@@ -227,8 +227,7 @@void machine_restart_smp (char * __unused) { cpu_restart_map = cpu_online_map; -smp_call_function (do_machine_restart,NULL ,0 ,0 ); -do_machine_restart (NULL ); +on_each_cpu (do_machine_restart,NULL ,0 ,0 ); }static void do_machine_halt (void * __unused)
@@ -246,8 +245,7 @@void machine_halt_smp (void ) { -smp_call_function (do_machine_halt,NULL ,0 ,0 ); -do_machine_halt (NULL ); +on_each_cpu (do_machine_halt,NULL ,0 ,0 ); }static void do_machine_power_off (void * __unused)
@@ -265,8 +263,7 @@void machine_power_off_smp (void ) { -smp_call_function (do_machine_power_off,NULL ,0 ,0 ); -do_machine_power_off (NULL ); +on_each_cpu (do_machine_power_off,NULL ,0 ,0 ); }/*
@@ -383,8 +380,7 @@ parms.end_ctl = cr; parms.orvals[cr] =1 << bit; parms.andvals[cr] =-1L ; -smp_call_function (smp_ctl_bit_callback, &parms,0 ,1 ); -__ctl_set_bit (cr, bit); +on_each_cpu (smp_ctl_bit_callback, &parms,0 ,1 ); }/*
@@ -397,8 +393,7 @@ parms.end_ctl = cr; parms.orvals[cr] =0 ; parms.andvals[cr] = ~(1L << bit); -smp_call_function (smp_ctl_bit_callback, &parms,0 ,1 ); -__ctl_clear_bit (cr, bit); +on_each_cpu (smp_ctl_bit_callback, &parms,0 ,1 ); }
arch/x86_64/kernel/bluesmoke.c +1 -13
@@ -111,11 +111,7 @@ {u32 low, high;int i; -unsigned int *cpu = info; -BUG_ON (*cpu !=smp_processor_id ()); - -preempt_disable ();for (i=0 ; i<banks; i++) {rdmsr (MSR_IA32_MC0_STATUS +i*4 , low, high);
@@ -130,20 +126,12 @@wmb (); } } -preempt_enable (); }static void mce_timerfunc (unsigned long data) { -unsigned int i; - -for (i=0 ; i<smp_num_cpus; i++) { -if (i ==smp_processor_id ()) -mce_checkregs (&i); -else -smp_call_function (mce_checkregs, &i,1 ,1 ); - } +on_each_cpu (mce_checkregs,NULL ,1 ,1 );/* Refresh the timer. */ mce_timer.expires = jiffies +MCE_RATE ;
arch/x86_64/kernel/io_apic.c +1 -2
@@ -928,8 +928,7 @@void print_all_local_APICs (void ) { -smp_call_function (print_local_APIC,NULL ,1 ,1 ); -print_local_APIC (NULL ); +on_each_cpu (print_local_APIC,NULL ,1 ,1 ); }void /*__init*/ print_PIC (void )
arch/x86_64/kernel/ldt.c +2 -2
@@ -60,13 +60,13 @@ pc->size = mincount;wmb ();if (reload) { +preempt_disable ();load_LDT (pc);#ifdef CONFIG_SMP -preempt_disable ();if (current->mm->cpu_vm_mask != (1 <<smp_processor_id ()))smp_call_function (flush_ldt,0 ,1 ,1 ); -preempt_enable ();#endif +preempt_enable (); }if (oldsize) {if (oldsize*LDT_ENTRY_SIZE >PAGE_SIZE )
arch/x86_64/kernel/smp.c +2 -11
@@ -328,7 +328,7 @@preempt_enable (); } -static inline void do_flush_tlb_all_local (void ) +static void do_flush_tlb_all (void * info) {unsigned long cpu =smp_processor_id ();
@@ -337,18 +337,9 @@leave_mm (cpu); } -static void flush_tlb_all_ipi (void * info) -{ -do_flush_tlb_all_local (); -} -void flush_tlb_all (void ) { -preempt_disable (); -smp_call_function (flush_tlb_all_ipi,0 ,1 ,1 ); - -do_flush_tlb_all_local (); -preempt_enable (); +on_each_cpu (do_flush_tlb_all,0 ,1 ,1 ); }void smp_kdb_stop (void )
arch/x86_64/mm/pageattr.c +1 -6
@@ -123,12 +123,7 @@static inline void flush_map (unsigned long address) { -preempt_disable (); -#ifdef CONFIG_SMP -smp_call_function (flush_kernel_map, (void *)address,1 ,1 ); -#endif -flush_kernel_map ((void *)address); -preempt_enable (); +on_each_cpu (flush_kernel_map, (void *)address,1 ,1 ); }struct deferred_page {
drivers/char/agp/agp.h +1 -2
@@ -42,9 +42,8 @@static void __attribute__((unused))global_cache_flush (void ) { -if (smp_call_function (ipi_handler,NULL ,1 ,1 ) !=0 ) +if (on_each_cpu (ipi_handler,NULL ,1 ,1 ) !=0 )panic (PFX "timed out waiting for the other CPUs!\n" ); -flush_agp_cache (); }#else static inline void global_cache_flush (void )
drivers/s390/char/sclp.c +1 -2
@@ -481,8 +481,7 @@do_machine_quiesce (void) { cpu_quiesce_map = cpu_online_map; -smp_call_function (do_load_quiesce_psw,NULL ,0 ,0 ); -do_load_quiesce_psw (NULL ); +on_each_cpu (do_load_quiesce_psw,NULL ,0 ,0 ); }#else static void
fs/buffer.c +1 -4
@@ -1403,10 +1403,7 @@static void invalidate_bh_lrus (void ) { -preempt_disable (); -invalidate_bh_lru (NULL ); -smp_call_function (invalidate_bh_lru,NULL ,1 ,1 ); -preempt_enable (); +on_each_cpu (invalidate_bh_lru,NULL ,1 ,1 ); } void set_bh_page(struct buffer_head *bh,
include/asm-parisc/cacheflush.h +1 -7
@@ -25,16 +25,10 @@extern void flush_cache_all_local (void ); -#ifdef CONFIG_SMP static inline void flush_cache_all (void ) { -smp_call_function ((void (*)(void *))flush_cache_all_local,NULL ,1 ,1 ); -flush_cache_all_local (); +on_each_cpu ((void (*)(void *))flush_cache_all_local,NULL ,1 ,1 ); } -#else -#define flush_cache_all flush_cache_all_local -#endif -/* The following value needs to be tuned and probably scaled with the * cache size.
include/linux/smp.h +22 -1
@@ -10,9 +10,10 @@#ifdef CONFIG_SMP +#include <linux/preempt.h> #include <linux/kernel.h> #include <linux/compiler.h> -#include <linux/threads.h> +#include <linux/thread_info.h> #include <asm/smp.h> #include <asm/bug.h>
@@ -54,6 +55,25 @@ int retry, int wait);/* +* Call a function on all processors +*/ +static inline int on_each_cpu (void (*func) (void *info),void *info, +int retry,int wait) +{ +int ret =0 ; + +preempt_disable (); + +if (num_online_cpus () >1 ) + ret =smp_call_function (func, info, retry, wait); +func (info); + +preempt_enable (); + +return ret; +} + +/* * True once the per process idle is forked */ extern int smp_threads_ready;
@@ -96,6 +116,7 @@#define hard_smp_processor_id () 0#define smp_threads_ready 1#define smp_call_function (func,info,retry,wait) ({ 0; }) +#define on_each_cpu (func,info,retry,wait) ({ func(info); 0; })static inline void smp_send_reschedule (int cpu) { }static inline void smp_send_reschedule_all (void ) { }#define cpu_online_map 1
mm/slab.c +4 -0
@@ -1116,12 +1116,16 @@static void smp_call_function_all_cpus (void (*func) (void *arg),void *arg) {check_irq_on (); +preempt_disable (); +local_irq_disable ();func (arg);local_irq_enable ();if (smp_call_function (func, arg,1 ,1 ))BUG (); + +preempt_enable (); }static void free_block (kmem_cache_t * cachep,void ** objpp,int len);
Quoted 8 lines · Thomas Schlichter
Hello,
here is a patch to solve all (I hope I missed none) possible problems that could occur on SMP machines running a preemptible kernel when
smp_call_function()calls a function which should be also executed on the current processor.
Patch looks pretty good, thanks. Fixes a real bug.
I worry a little about the s390/s390x change. smp_ctl_set_bit() and smp_ctl_clear_bit(). The functions which are being called on local and remote are fairly different. I'm sure it's OK, but... I changed that bit to open-code the preempt_disable/enable.
The arch/x86_64/kernel/bluesmoke.c change looks right. Seems that someone didn't understand the smp_call_function() API in there.
Quoted 7 lines · Thomas Schlichter
here is a patch to solve all (I hope I missed none) possible problems that could occur on SMP machines running a preemptible kernel when
smp_call_function()calls a function which should be also executed on the current processor.This patch is based on the one Dave Jones sent to the LKML last friday and applies to the linux kernel version 2.5.63.
Just one comment. You moved quite a few of the preempt_disable/enable pairs outside of the CONFIG_SMP checks. The issue we're working against here is to try and prevent preemption and ending up on a different CPU. As this cannot happen if CONFIG_SMP=n, I don't see why you've done this.
Dave
Quoted 8 lines · Dave Jones
Quoted 5 lines · Thomas Schlichter
here is a patch to solve all (I hope I missed none) possible problems that could occur on SMP machines running a preemptible kernel when
smp_call_function()calls a function which should be also executed on the current processor.
Quoted 8 lines
Quoted 2 lines
This patch is based on the one Dave Jones sent to the LKML last friday and applies to the linux kernel version 2.5.63.
Just one comment. You moved quite a few of the preempt_disable/enable pairs outside of the
CONFIG_SMPchecks. The issue we're working against here is to try and prevent preemption and ending up on a different CPU. As this cannot happen ifCONFIG_SMP=n, I don't see why you've done this.
Just in two places.
arch/i386/kernel/ldt.c | 6 ++++-- arch/x86_64/kernel/ldt.c | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-)
arch/i386/kernel/ldt.c renamed from arch/i386/kernel/ldt.c~on_each_cpu-ldt-cleanup +4 -2
@@ -55,13 +55,15 @@ static int alloc_ldt(mm_context_t *pc, iwmb ();if (reload) { +#ifdef CONFIG_SMP preempt_disable ();load_LDT (pc); -#ifdef CONFIG_SMP if (current->mm->cpu_vm_mask != (1 <<smp_processor_id ()))smp_call_function (flush_ldt,0 ,1 ,1 ); -#endif preempt_enable (); +#else +load_LDT (pc); +#endif }if (oldsize) {if (oldsize*LDT_ENTRY_SIZE >PAGE_SIZE )
arch/x86_64/kernel/ldt.c renamed from arch/x86_64/kernel/ldt.c~on_each_cpu-ldt-cleanup +4 -2
@@ -60,13 +60,15 @@ static int alloc_ldt(mm_context_t *pc, i pc->size = mincount;wmb ();if (reload) { +#ifdef CONFIG_SMP preempt_disable ();load_LDT (pc); -#ifdef CONFIG_SMP if (current->mm->cpu_vm_mask != (1 <<smp_processor_id ()))smp_call_function (flush_ldt,0 ,1 ,1 ); -#endif preempt_enable (); +#else +load_LDT (pc); +#endif }if (oldsize) {if (oldsize*LDT_ENTRY_SIZE >PAGE_SIZE )
_
Quoted 4 lines · Dave Jones
Just one comment. You moved quite a few of the preempt_disable/enable pairs outside of the
CONFIG_SMPchecks. The issue we're working against here is to try and prevent preemption and ending up on a different CPU. As this cannot happen ifCONFIG_SMP=n, I don't see why you've done this.
Quoted 1 line · Andrew Morton
Just in two places.
[snip]
Yes, thanks for delivering this better patch!
My approach wanted just to be the most simple possibility... ;-)
Thomas
Quoted 5 lines · Andrew Morton
Quoted 4 lines
Just one comment. You moved quite a few of the preempt_disable/enable pairs outside of the
CONFIG_SMPchecks. The issue we're working against here is to try and prevent preemption and ending up on a different CPU. As this cannot happen ifCONFIG_SMP=n, I don't see why you've done this.Just in two places.
Ok, slight exaggeration 8-) Looks good to me.
btw, (unrelated) shouldn't smp_call_function be doing magick checks with cpu_online() ?
Dave
Quoted 3 lines · Dave Jones
btw, (unrelated) shouldn't
smp_call_functionbe doing magick checks withcpu_online()?
Looks OK? It sprays the IPI out to all the other CPUs in cpu_online_map, and waits for num_online_cpus()-1 CPUs to answer.
All very racy in the presence of CPUs going offline, but that's all over the place. Depends how the offlining will be done I guess.
Quoted 4 lines · Andrew Morton
Quoted 2 lines
btw, (unrelated) shouldn't
smp_call_functionbe doing magick checks withcpu_online()?Looks OK? It sprays the IPI out to all the other CPUs in
cpu_online_map, and waits for num_online_cpus()-1 CPUs to answer.
Doh, of course.
Ugh, mornings.
Dave
Quoted 2 lines · Dave Jones
btw, (unrelated) shouldn't
smp_call_functionbe doing magick checks withcpu_online()?
Quoted 5 lines · Andrew Morton
Looks OK? It sprays the IPI out to all the other CPUs in
cpu_online_map, and waits for num_online_cpus()-1 CPUs to answer.All very racy in the presence of CPUs going offline, but that's all over the place. Depends how the offlining will be done I guess.
Well, now I see the check for num_online_cpus() != 1 in smp_call_function(), too, so the check in on_each_cpu() is not needed and possibly better this patch should apply to the include/linux/smp.h file...
Thomas
include/linux/smp.h Wed Feb 26 11:41:45 2003 renamed from include/linux/smp.h.orig Mon Feb 24 20:05:33 2003 +21 -1
@@ -10,9 +10,10 @@ #ifdef CONFIG_SMP +#include <linux/preempt.h> #include <linux/kernel.h> #include <linux/compiler.h> -#include <linux/threads.h> +#include <linux/thread_info.h> #include <asm/smp.h> #include <asm/bug.h>
@@ -54,6 +55,24 @@ int retry, int wait); /* + * Call a function on all processors + */ +static inline int on_each_cpu(void (*func) (void *info), void *info, + int retry, int wait) +{ + int ret; + + preempt_disable(); + + ret = smp_call_function(func, info, retry, wait); + func(info); + + preempt_enable(); + + return ret; +} + +/* * True once the per process idle is forked */ extern int smp_threads_ready;
@@ -96,6 +115,7 @@ #define hard_smp_processor_id() 0 #define smp_threads_ready 1 #define smp_call_function(func,info,retry,wait) ({ 0; }) +#define on_each_cpu(func,info,retry,wait) ({ func(info); 0; }) static inline void smp_send_reschedule(int cpu) { } static inline void smp_send_reschedule_all(void) { } #define cpu_online_map 1
Quoted 7 lines · Andrew Morton
Quoted 3 lines · Dave Jones
btw, (unrelated) shouldn't
smp_call_functionbe doing magick checks withcpu_online()?Looks OK? It sprays the IPI out to all the other CPUs in
cpu_online_map, and waits for num_online_cpus()-1 CPUs to answer.
You cannot do that by counting without a lot of care. IPI messages do not have guaranteed "once only" semantics. On an error a resend can and has been observed to cause a reissue of an IPI on PII/PIII setups
Quoted 7 lines · Alan Cox
Quoted 5 lines · Andrew Morton
Quoted 3 lines · Dave Jones
btw, (unrelated) shouldn't
smp_call_functionbe doing magick checks withcpu_online()?
Quoted 6 lines
Quoted 2 lines
Looks OK? It sprays the IPI out to all the other CPUs in
cpu_online_map, and waits for num_online_cpus()-1 CPUs to answer.You cannot do that by counting without a lot of care. IPI messages do not have guaranteed "once only" semantics. On an error a resend can and has been observed to cause a reissue of an IPI on PII/PIII setups
If that resend results in delivery of an actual extra interrupt, the resent-to CPU can start playing with stuff which used to be on the sender's stack and the box goes splat.
Didn't sct have a fix for that?
Quoted 5 lines · Andrew Morton
If that resend results in delivery of an actual extra interrupt, the resent-to CPU can start playing with stuff which used to be on the sender's stack and the box goes splat.
Didn't sct have a fix for that?
Yes but it was never merged mainstream for some reason. I think it kind of got away
Quoted 5 lines
Quoted 2 lines
btw, (unrelated) shouldn't
smp_call_functionbe doing magick checks withcpu_online()?Looks OK? It sprays the IPI out to all the other CPUs in
cpu_online_map, and waits for num_online_cpus()-1 CPUs to answer.
Incidentally, would be nice if this thing had a timeout and bugged out when one didn't reply, rather than just wedging the whole CPU for ever. Yes, it's normally some other bug that makes the other CPU not reply, but would make diags a damned sight easier.
M.