From: Randy Dunlap <[email protected]>
smp_call_function_single() needs to be visible in non-SMP builds, to fix:
arch/x86_64/kernel/vsyscall.c:283: warning: implicit declaration of function 'smp_call_function_single'
The (other/trivial) fix (instead of this one) is to add:
#include <asm/smp.h>
to linux-2.6.19-rc6-mm2/arch/x86_64/kernel/vsyscall.c
Signed-off-by: Randy Dunlap <[email protected]>
---
include/asm-x86_64/smp.h | 7 -------
include/linux/smp.h | 7 +++++++
2 files changed, 7 insertions(+), 7 deletions(-)
--- linux-2.6.19-rc6-mm2.orig/include/asm-x86_64/smp.h
+++ linux-2.6.19-rc6-mm2/include/asm-x86_64/smp.h
@@ -113,13 +113,6 @@ static __inline int logical_smp_processo
#define cpu_physical_id(cpu) x86_cpu_to_apicid[cpu]
#else
#define cpu_physical_id(cpu) boot_cpu_id
-static inline int smp_call_function_single(int cpuid, void (*func) (void *info),
- void *info, int retry, int wait)
-{
- /* Disable interrupts here? */
- func(info);
- return 0;
-}
#endif /* !CONFIG_SMP */
#endif
--- linux-2.6.19-rc6-mm2.orig/include/linux/smp.h
+++ linux-2.6.19-rc6-mm2/include/linux/smp.h
@@ -99,6 +99,13 @@ static inline int up_smp_call_function(v
static inline void smp_send_reschedule(int cpu) { }
#define num_booting_cpus() 1
#define smp_prepare_boot_cpu() do {} while (0)
+static inline int smp_call_function_single(int cpuid, void (*func) (void *info),
+ void *info, int retry, int wait)
+{
+ /* Disable interrupts here? */
+ func(info);
+ return 0;
+}
#endif /* !SMP */
---
On Wed, 29 Nov 2006 17:01:11 -0800
Randy Dunlap <[email protected]> wrote:
> From: Randy Dunlap <[email protected]>
>
> smp_call_function_single() needs to be visible in non-SMP builds, to fix:
>
> arch/x86_64/kernel/vsyscall.c:283: warning: implicit declaration of function 'smp_call_function_single'
>
> The (other/trivial) fix (instead of this one) is to add:
> #include <asm/smp.h>
> to linux-2.6.19-rc6-mm2/arch/x86_64/kernel/vsyscall.c
>
> Signed-off-by: Randy Dunlap <[email protected]>
> ---
> include/asm-x86_64/smp.h | 7 -------
> include/linux/smp.h | 7 +++++++
> 2 files changed, 7 insertions(+), 7 deletions(-)
>
> --- linux-2.6.19-rc6-mm2.orig/include/asm-x86_64/smp.h
> +++ linux-2.6.19-rc6-mm2/include/asm-x86_64/smp.h
> @@ -113,13 +113,6 @@ static __inline int logical_smp_processo
> #define cpu_physical_id(cpu) x86_cpu_to_apicid[cpu]
> #else
> #define cpu_physical_id(cpu) boot_cpu_id
> -static inline int smp_call_function_single(int cpuid, void (*func) (void *info),
> - void *info, int retry, int wait)
> -{
> - /* Disable interrupts here? */
> - func(info);
> - return 0;
> -}
> #endif /* !CONFIG_SMP */
> #endif
>
> --- linux-2.6.19-rc6-mm2.orig/include/linux/smp.h
> +++ linux-2.6.19-rc6-mm2/include/linux/smp.h
> @@ -99,6 +99,13 @@ static inline int up_smp_call_function(v
> static inline void smp_send_reschedule(int cpu) { }
> #define num_booting_cpus() 1
> #define smp_prepare_boot_cpu() do {} while (0)
> +static inline int smp_call_function_single(int cpuid, void (*func) (void *info),
> + void *info, int retry, int wait)
> +{
> + /* Disable interrupts here? */
> + func(info);
> + return 0;
> +}
>
> #endif /* !SMP */
>
No, I think this patch is right - the declaration of the CONFIG_SMP
smp_call_function_single() is in linux/smp.h so the !CONFIG_SMP declaration
or definition should be there too.
It's still buggy though. It should disable local interrupts around the
call to match the SMP version. I'll fix that separately.
On Wed, 2006-11-29 at 17:45 -0800, Andrew Morton wrote:
> No, I think this patch is right - the declaration of the CONFIG_SMP
> smp_call_function_single() is in linux/smp.h so the !CONFIG_SMP
> declaration
> or definition should be there too.
>
> It's still buggy though. It should disable local interrupts around
> the
> call to match the SMP version. I'll fix that separately.
hm, didnt i send an updated patch for that already? See the patch below,
from many days ago. I sent it after the tsc-sync-rewrite patch.
Ingo
--------------->
Subject: x86_64: build fixes
From: Ingo Molnar <[email protected]>
x86_64 does not build cleanly on UP:
arch/x86_64/kernel/vsyscall.c: In function 'cpu_vsyscall_notifier':
arch/x86_64/kernel/vsyscall.c:282: warning: implicit declaration of
function 'smp_call_function_single'
arch/x86_64/kernel/vsyscall.c: At top level:
arch/x86_64/kernel/vsyscall.c:279: warning: 'cpu_vsyscall_notifier'
defined but not used
this patch fixes it by making smp_call_function_single() globally
available.
Signed-off-by: Ingo Molnar <[email protected]>
---
include/asm-x86_64/smp.h | 11 ++---------
include/linux/smp.h | 10 +++++++---
kernel/sched.c | 19 +++++++++++++++++++
3 files changed, 28 insertions(+), 12 deletions(-)
Index: linux/include/asm-x86_64/smp.h
===================================================================
--- linux.orig/include/asm-x86_64/smp.h
+++ linux/include/asm-x86_64/smp.h
@@ -115,16 +115,9 @@ static __inline int logical_smp_processo
}
#ifdef CONFIG_SMP
-#define cpu_physical_id(cpu) x86_cpu_to_apicid[cpu]
+# define cpu_physical_id(cpu) x86_cpu_to_apicid[cpu]
#else
-#define cpu_physical_id(cpu) boot_cpu_id
-static inline int smp_call_function_single(int cpuid, void (*func)
(void *info),
- void *info, int retry, int wait)
-{
- /* Disable interrupts here? */
- func(info);
- return 0;
-}
+# define cpu_physical_id(cpu) boot_cpu_id
#endif /* !CONFIG_SMP */
#endif
Index: linux/include/linux/smp.h
===================================================================
--- linux.orig/include/linux/smp.h
+++ linux/include/linux/smp.h
@@ -53,9 +53,6 @@ extern void smp_cpus_done(unsigned int m
*/
int smp_call_function(void(*func)(void *info), void *info, int retry,
int wait);
-int smp_call_function_single(int cpuid, void (*func) (void *info), void
*info,
- int retry, int wait);
-
/*
* Call a function on all processors
*/
@@ -103,6 +100,13 @@ static inline void smp_send_reschedule(i
#endif /* !SMP */
/*
+ * Call a function on a specific CPU (on UP the function gets executed
+ * on the current CPU, immediately):
+ */
+int smp_call_function_single(int cpuid, void (*func) (void *info), void
*info,
+ int retry, int wait);
+
+/*
* smp_processor_id(): get the current CPU ID.
*
* if DEBUG_PREEMPT is enabled the we check whether it is
Index: linux/kernel/sched.c
===================================================================
--- linux.orig/kernel/sched.c
+++ linux/kernel/sched.c
@@ -1110,6 +1110,25 @@ repeat:
task_rq_unlock(rq, &flags);
}
+#ifndef CONFIG_SMP
+/*
+ * Call a function on a specific CPU (on UP the function gets executed
+ * on the current CPU, immediately):
+ */
+int smp_call_function_single(int cpuid, void (*func) (void *info), void
*info,
+ int retry, int wait)
+{
+ unsigned long flags;
+
+ local_irq_save(flags);
+ func(info);
+ local_irq_restore(flags);
+
+ return 0;
+}
+EXPORT_SYMBOL(smp_call_function_single);
+#endif
+
/***
* kick_process - kick a running thread to enter/exit the kernel
* @p: the to-be-kicked thread
On Thu, 30 Nov 2006 08:00:00 +0100
Ingo Molnar <[email protected]> wrote:
> On Wed, 2006-11-29 at 17:45 -0800, Andrew Morton wrote:
> > No, I think this patch is right - the declaration of the CONFIG_SMP
> > smp_call_function_single() is in linux/smp.h so the !CONFIG_SMP
> > declaration
> > or definition should be there too.
> >
> > It's still buggy though. It should disable local interrupts around
> > the
> > call to match the SMP version. I'll fix that separately.
>
> hm, didnt i send an updated patch for that already? See the patch below,
> from many days ago. I sent it after the tsc-sync-rewrite patch.
>
Might have got lost.
> --------------->
> Subject: x86_64: build fixes
> From: Ingo Molnar <[email protected]>
>
> x86_64 does not build cleanly on UP:
>
> arch/x86_64/kernel/vsyscall.c: In function 'cpu_vsyscall_notifier':
> arch/x86_64/kernel/vsyscall.c:282: warning: implicit declaration of
> function 'smp_call_function_single'
> arch/x86_64/kernel/vsyscall.c: At top level:
> arch/x86_64/kernel/vsyscall.c:279: warning: 'cpu_vsyscall_notifier'
> defined but not used
>
> this patch fixes it by making smp_call_function_single() globally
> available.
>
> Signed-off-by: Ingo Molnar <[email protected]>
> ---
> include/asm-x86_64/smp.h | 11 ++---------
> include/linux/smp.h | 10 +++++++---
> kernel/sched.c | 19 +++++++++++++++++++
> 3 files changed, 28 insertions(+), 12 deletions(-)
>
> Index: linux/include/asm-x86_64/smp.h
> ===================================================================
> --- linux.orig/include/asm-x86_64/smp.h
> +++ linux/include/asm-x86_64/smp.h
> @@ -115,16 +115,9 @@ static __inline int logical_smp_processo
> }
>
> #ifdef CONFIG_SMP
> -#define cpu_physical_id(cpu) x86_cpu_to_apicid[cpu]
> +# define cpu_physical_id(cpu) x86_cpu_to_apicid[cpu]
> #else
> -#define cpu_physical_id(cpu) boot_cpu_id
> -static inline int smp_call_function_single(int cpuid, void (*func)
> (void *info),
congratulations-your-first-wordwrapped-patch ;)
> --- linux.orig/kernel/sched.c
> +++ linux/kernel/sched.c
> @@ -1110,6 +1110,25 @@ repeat:
> task_rq_unlock(rq, &flags);
> }
>
> +#ifndef CONFIG_SMP
> +/*
> + * Call a function on a specific CPU (on UP the function gets executed
> + * on the current CPU, immediately):
> + */
> +int smp_call_function_single(int cpuid, void (*func) (void *info), void
> *info,
> + int retry, int wait)
> +{
> + unsigned long flags;
> +
> + local_irq_save(flags);
> + func(info);
> + local_irq_restore(flags);
> +
> + return 0;
> +}
yes, but a) calling the SMP version with local interrupts disabled is a
bug, so we can use bare local_irq_disable() here and b) only two
archictures call or use this function, so all the others don't want a copy
of it.
So I did:
--- a/include/linux/smp.h~up-smp_call_function_single-should-disable-interrupts
+++ a/include/linux/smp.h
@@ -15,6 +15,7 @@ extern void cpu_idle(void);
#include <linux/kernel.h>
#include <linux/compiler.h>
#include <linux/thread_info.h>
+#include <linux/irqflags.h>
#include <asm/smp.h>
/*
@@ -102,8 +103,9 @@ static inline void smp_send_reschedule(i
static inline int smp_call_function_single(int cpuid, void (*func) (void *info),
void *info, int retry, int wait)
{
- /* Disable interrupts here? */
+ local_irq_disable(); /* Match the SMP call environment */
func(info);
+ local_irq_enable();
return 0;
}
_
which is somewhat unpleasant. I added a WARN_ON(irqs_disabled()) to the
out-of-line SMP version.
btw, does anyone know why the SMP versions of this function use
spin_lock_bh(&call_lock)?
On Wed, 2006-11-29 at 23:54 -0800, Andrew Morton wrote:
> which is somewhat unpleasant. I added a WARN_ON(irqs_disabled()) to
> the
> out-of-line SMP version.
ok.
>
> btw, does anyone know why the SMP versions of this function use
> spin_lock_bh(&call_lock)?
that makes no sense (neither the get_cpu()/put_cpu() gymnastics) if this
is called with irqs disabled all the time.
Ingo
>
On Thu, 30 Nov 2006 10:22:20 +0100
Ingo Molnar <[email protected]> wrote:
> >
> > btw, does anyone know why the SMP versions of this function use
> > spin_lock_bh(&call_lock)?
>
> that makes no sense (neither the get_cpu()/put_cpu() gymnastics) if this
> is called with irqs disabled all the time.
smp_call_function_single() must be called with local interrupts ENabled.
But why isn't it just spin_lock()?
<looks>
Eric simply copied that code from ia64, which added the spin_lock_bh()
in 2.4.8. Ho-hum.
On Thu, 30 Nov 2006 08:00:00 +0100 Ingo Molnar wrote:
> On Wed, 2006-11-29 at 17:45 -0800, Andrew Morton wrote:
> > No, I think this patch is right - the declaration of the CONFIG_SMP
> > smp_call_function_single() is in linux/smp.h so the !CONFIG_SMP
> > declaration
> > or definition should be there too.
> >
> > It's still buggy though. It should disable local interrupts around
> > the
> > call to match the SMP version. I'll fix that separately.
>
> hm, didnt i send an updated patch for that already? See the patch below,
> from many days ago. I sent it after the tsc-sync-rewrite patch.
Hi Ingo,
Has there been a patch for this one? (UP again, not SMP)
drivers/input/ff-memless.c:384: warning: implicit declaration of function 'local_bh_disable'
drivers/input/ff-memless.c:393: warning: implicit declaration of function 'local_bh_enable'
Thanks,
---
~Randy
config: http://oss.oracle.com/~rdunlap/configs/config-input-up-header
On Thu, 30 Nov 2006 14:11:40 -0800
Randy Dunlap <[email protected]> wrote:
> On Thu, 30 Nov 2006 08:00:00 +0100 Ingo Molnar wrote:
>
> > On Wed, 2006-11-29 at 17:45 -0800, Andrew Morton wrote:
> > > No, I think this patch is right - the declaration of the CONFIG_SMP
> > > smp_call_function_single() is in linux/smp.h so the !CONFIG_SMP
> > > declaration
> > > or definition should be there too.
> > >
> > > It's still buggy though. It should disable local interrupts around
> > > the
> > > call to match the SMP version. I'll fix that separately.
> >
> > hm, didnt i send an updated patch for that already? See the patch below,
> > from many days ago. I sent it after the tsc-sync-rewrite patch.
>
> Hi Ingo,
>
> Has there been a patch for this one? (UP again, not SMP)
>
> drivers/input/ff-memless.c:384: warning: implicit declaration of function 'local_bh_disable'
> drivers/input/ff-memless.c:393: warning: implicit declaration of function 'local_bh_enable'
>
> Thanks,
> ---
> ~Randy
> config: http://oss.oracle.com/~rdunlap/configs/config-input-up-header
eww.. I guess linux/spinlock.h should really include linux/interrupt.h.
But interrupt.h includes stuff like sched.h which will want spinlock.h.
This, maybe?
include/linux/bottom_half.h | 5 +++++
include/linux/interrupt.h | 7 +------
include/linux/spinlock.h | 1 +
3 files changed, 7 insertions(+), 6 deletions(-)
diff -puN /dev/null include/linux/bottom_half.h
--- /dev/null
+++ a/include/linux/bottom_half.h
@@ -0,0 +1,5 @@
+extern void local_bh_disable(void);
+extern void __local_bh_enable(void);
+extern void _local_bh_enable(void);
+extern void local_bh_enable(void);
+extern void local_bh_enable_ip(unsigned long ip);
diff -puN include/linux/interrupt.h~add-bottom_half.h include/linux/interrupt.h
--- a/include/linux/interrupt.h~add-bottom_half.h
+++ a/include/linux/interrupt.h
@@ -11,6 +11,7 @@
#include <linux/hardirq.h>
#include <linux/sched.h>
#include <linux/irqflags.h>
+#include <linux/bottom_half.h>
#include <asm/atomic.h>
#include <asm/ptrace.h>
#include <asm/system.h>
@@ -217,12 +218,6 @@ static inline void __deprecated save_and
#define save_and_cli(x) save_and_cli(&x)
#endif /* CONFIG_SMP */
-extern void local_bh_disable(void);
-extern void __local_bh_enable(void);
-extern void _local_bh_enable(void);
-extern void local_bh_enable(void);
-extern void local_bh_enable_ip(unsigned long ip);
-
/* PLEASE, avoid to allocate new softirqs, if you need not _really_ high
frequency threaded job scheduling. For almost all the purposes
tasklets are more than enough. F.e. all serial device BHs et
diff -puN include/linux/spinlock.h~add-bottom_half.h include/linux/spinlock.h
--- a/include/linux/spinlock.h~add-bottom_half.h
+++ a/include/linux/spinlock.h
@@ -52,6 +52,7 @@
#include <linux/thread_info.h>
#include <linux/kernel.h>
#include <linux/stringify.h>
+#include <linux/bottom_half.h>
#include <asm/system.h>
_
Andrew Morton wrote:
> On Thu, 30 Nov 2006 14:11:40 -0800
> Randy Dunlap <[email protected]> wrote:
>
>> On Thu, 30 Nov 2006 08:00:00 +0100 Ingo Molnar wrote:
>>
>>> On Wed, 2006-11-29 at 17:45 -0800, Andrew Morton wrote:
>>>> No, I think this patch is right - the declaration of the CONFIG_SMP
>>>> smp_call_function_single() is in linux/smp.h so the !CONFIG_SMP
>>>> declaration
>>>> or definition should be there too.
>>>>
>>>> It's still buggy though. It should disable local interrupts around
>>>> the
>>>> call to match the SMP version. I'll fix that separately.
>>> hm, didnt i send an updated patch for that already? See the patch below,
>>> from many days ago. I sent it after the tsc-sync-rewrite patch.
>> Hi Ingo,
>>
>> Has there been a patch for this one? (UP again, not SMP)
>>
>> drivers/input/ff-memless.c:384: warning: implicit declaration of function 'local_bh_disable'
>> drivers/input/ff-memless.c:393: warning: implicit declaration of function 'local_bh_enable'
>>
>> Thanks,
>> ---
>> ~Randy
>> config: http://oss.oracle.com/~rdunlap/configs/config-input-up-header
>
> eww.. I guess linux/spinlock.h should really include linux/interrupt.h.
> But interrupt.h includes stuff like sched.h which will want spinlock.h.
>
> This, maybe?
Looks good. I had already tried (and failed) adding interrupt.h to spinlock.h --
what a mess.
--
~Randy
On Thu, 30 Nov 2006 14:27:19 -0800 Andrew Morton wrote:
> On Thu, 30 Nov 2006 14:11:40 -0800
> Randy Dunlap <[email protected]> wrote:
>
> > On Thu, 30 Nov 2006 08:00:00 +0100 Ingo Molnar wrote:
> >
> > > On Wed, 2006-11-29 at 17:45 -0800, Andrew Morton wrote:
> > > > No, I think this patch is right - the declaration of the CONFIG_SMP
> > > > smp_call_function_single() is in linux/smp.h so the !CONFIG_SMP
> > > > declaration
> > > > or definition should be there too.
> > > >
> > > > It's still buggy though. It should disable local interrupts around
> > > > the
> > > > call to match the SMP version. I'll fix that separately.
> > >
> > > hm, didnt i send an updated patch for that already? See the patch below,
> > > from many days ago. I sent it after the tsc-sync-rewrite patch.
> >
> > Hi Ingo,
> >
> > Has there been a patch for this one? (UP again, not SMP)
> >
> > drivers/input/ff-memless.c:384: warning: implicit declaration of function 'local_bh_disable'
> > drivers/input/ff-memless.c:393: warning: implicit declaration of function 'local_bh_enable'
> >
> > Thanks,
> > ---
> > ~Randy
> > config: http://oss.oracle.com/~rdunlap/configs/config-input-up-header
>
> eww.. I guess linux/spinlock.h should really include linux/interrupt.h.
> But interrupt.h includes stuff like sched.h which will want spinlock.h.
>
> This, maybe?
Ack. Tested on UP and SMP x86_64.
> include/linux/bottom_half.h | 5 +++++
> include/linux/interrupt.h | 7 +------
> include/linux/spinlock.h | 1 +
> 3 files changed, 7 insertions(+), 6 deletions(-)
>
> diff -puN /dev/null include/linux/bottom_half.h
> --- /dev/null
> +++ a/include/linux/bottom_half.h
> @@ -0,0 +1,5 @@
> +extern void local_bh_disable(void);
> +extern void __local_bh_enable(void);
> +extern void _local_bh_enable(void);
> +extern void local_bh_enable(void);
> +extern void local_bh_enable_ip(unsigned long ip);
> diff -puN include/linux/interrupt.h~add-bottom_half.h include/linux/interrupt.h
> --- a/include/linux/interrupt.h~add-bottom_half.h
> +++ a/include/linux/interrupt.h
> @@ -11,6 +11,7 @@
> #include <linux/hardirq.h>
> #include <linux/sched.h>
> #include <linux/irqflags.h>
> +#include <linux/bottom_half.h>
> #include <asm/atomic.h>
> #include <asm/ptrace.h>
> #include <asm/system.h>
> @@ -217,12 +218,6 @@ static inline void __deprecated save_and
> #define save_and_cli(x) save_and_cli(&x)
> #endif /* CONFIG_SMP */
>
> -extern void local_bh_disable(void);
> -extern void __local_bh_enable(void);
> -extern void _local_bh_enable(void);
> -extern void local_bh_enable(void);
> -extern void local_bh_enable_ip(unsigned long ip);
> -
> /* PLEASE, avoid to allocate new softirqs, if you need not _really_ high
> frequency threaded job scheduling. For almost all the purposes
> tasklets are more than enough. F.e. all serial device BHs et
> diff -puN include/linux/spinlock.h~add-bottom_half.h include/linux/spinlock.h
> --- a/include/linux/spinlock.h~add-bottom_half.h
> +++ a/include/linux/spinlock.h
> @@ -52,6 +52,7 @@
> #include <linux/thread_info.h>
> #include <linux/kernel.h>
> #include <linux/stringify.h>
> +#include <linux/bottom_half.h>
>
> #include <asm/system.h>
---
~Randy