smp.c | 49 ++++++++++++++++++++++++++++++++-----------------
1 files changed, 32 insertions(+), 17 deletions(-)
Index: linux-2.5.60/arch/s390/kernel/smp.c
===================================================================
RCS file: /build/cvsroot/linux-2.5.60/arch/s390/kernel/smp.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 smp.c
--- linux-2.5.60/arch/s390/kernel/smp.c 10 Feb 2003 22:15:54 -0000 1.1.1.1
+++ linux-2.5.60/arch/s390/kernel/smp.c 14 Feb 2003 07:31:47 -0000
@@ -102,27 +102,34 @@
* in the system.
*/
-int smp_call_function (void (*func) (void *info), void *info, int nonatomic,
- int wait)
/*
- * [SUMMARY] Run a function on all other CPUs.
- * <func> The function to run. This must be fast and non-blocking.
- * <info> An arbitrary pointer to pass to the function.
- * <nonatomic> currently unused.
- * <wait> If true, wait (atomically) until function has completed on other CPUs.
- * [RETURNS] 0 on success, else a negative status code. Does not return until
- * remote CPUs are nearly ready to execute <<func>> or are or have executed.
+ * smp_call_function_on_cpu - Runs func on all processors in the mask
+ *
+ * @func: The function to run. This must be fast and non-blocking.
+ * @info: An arbitrary pointer to pass to the function.
+ * @wait: If true, wait (atomically) until function has completed on other CPUs.
+ * @mask: The bitmask of CPUs to call the function
+ *
+ * Returns 0 on success, else a negative status code. Does not return until
+ * remote CPUs are nearly ready to execute func or have executed it.
*
* You must not call this function with disabled interrupts or from a
* hardware interrupt handler or from a bottom half handler.
*/
+
+int smp_call_function_on_cpu (void (*func) (void *info), void *info, int wait,
+ unsigned long mask)
{
struct call_data_struct data;
- int cpus = num_online_cpus()-1;
+ int i, cpu, num_cpus;
- /* FIXME: get cpu lock -hc */
- if (cpus <= 0)
- return 0;
+ cpu = get_cpu();
+ mask &= (1UL << cpu);
+ num_cpus = hweight32(mask);
+ if (num_cpus == 0) {
+ put_cpu_no_resched();
+ return -EINVAL;
+ }
data.func = func;
data.info = info;
@@ -134,18 +141,26 @@
spin_lock(&call_lock);
call_data = &data;
/* Send a message to all other CPUs and wait for them to respond */
- smp_ext_bitcall_others(ec_call_function);
+ for (i = 0; i < NR_CPUS; i++) {
+ if (cpu_online(i) && ((1UL << cpu) && mask))
+ smp_ext_bitcall(i, ec_call_function);
+ }
/* Wait for response */
- while (atomic_read(&data.started) != cpus)
+ while (atomic_read(&data.started) != num_cpus)
barrier();
if (wait)
- while (atomic_read(&data.finished) != cpus)
+ while (atomic_read(&data.finished) != num_cpus)
barrier();
spin_unlock(&call_lock);
-
+ put_cpu_no_resched();
return 0;
+}
+
+int smp_call_function (void (*func) (void *info), void *info, int nonatomic, int wait)
+{
+ return smp_call_function_on_cpu(func, info, wait, cpu_online_map);
}
static inline void do_send_stop(void)
One liner to fix num_cpus == 0 on SMP kernel w/ UP box
Index: linux-2.5.60/arch/s390/kernel/smp.c
===================================================================
RCS file: /build/cvsroot/linux-2.5.60/arch/s390/kernel/smp.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 smp.c
--- linux-2.5.60/arch/s390/kernel/smp.c 10 Feb 2003 22:15:54 -0000 1.1.1.1
+++ linux-2.5.60/arch/s390/kernel/smp.c 14 Feb 2003 12:23:17 -0000
@@ -102,27 +102,34 @@
* in the system.
*/
-int smp_call_function (void (*func) (void *info), void *info, int nonatomic,
- int wait)
/*
- * [SUMMARY] Run a function on all other CPUs.
- * <func> The function to run. This must be fast and non-blocking.
- * <info> An arbitrary pointer to pass to the function.
- * <nonatomic> currently unused.
- * <wait> If true, wait (atomically) until function has completed on other CPUs.
- * [RETURNS] 0 on success, else a negative status code. Does not return until
- * remote CPUs are nearly ready to execute <<func>> or are or have executed.
+ * smp_call_function_on_cpu - Runs func on all processors in the mask
+ *
+ * @func: The function to run. This must be fast and non-blocking.
+ * @info: An arbitrary pointer to pass to the function.
+ * @wait: If true, wait (atomically) until function has completed on other CPUs.
+ * @mask: The bitmask of CPUs to call the function
+ *
+ * Returns 0 on success, else a negative status code. Does not return until
+ * remote CPUs are nearly ready to execute func or have executed it.
*
* You must not call this function with disabled interrupts or from a
* hardware interrupt handler or from a bottom half handler.
*/
+
+int smp_call_function_on_cpu (void (*func) (void *info), void *info, int wait,
+ unsigned long mask)
{
struct call_data_struct data;
- int cpus = num_online_cpus()-1;
+ int i, cpu, num_cpus;
- /* FIXME: get cpu lock -hc */
- if (cpus <= 0)
+ cpu = get_cpu();
+ mask &= (1UL << cpu);
+ num_cpus = hweight32(mask);
+ if (num_cpus == 0) {
+ put_cpu_no_resched();
return 0;
+ }
data.func = func;
data.info = info;
@@ -134,18 +141,26 @@
spin_lock(&call_lock);
call_data = &data;
/* Send a message to all other CPUs and wait for them to respond */
- smp_ext_bitcall_others(ec_call_function);
+ for (i = 0; i < NR_CPUS; i++) {
+ if (cpu_online(i) && ((1UL << cpu) && mask))
+ smp_ext_bitcall(i, ec_call_function);
+ }
/* Wait for response */
- while (atomic_read(&data.started) != cpus)
+ while (atomic_read(&data.started) != num_cpus)
barrier();
if (wait)
- while (atomic_read(&data.finished) != cpus)
+ while (atomic_read(&data.finished) != num_cpus)
barrier();
spin_unlock(&call_lock);
-
+ put_cpu_no_resched();
return 0;
+}
+
+int smp_call_function (void (*func) (void *info), void *info, int nonatomic, int wait)
+{
+ return smp_call_function_on_cpu(func, info, wait, cpu_online_map);
}
static inline void do_send_stop(void)
Zwane Mwaikambo wrote:
>+ cpu = get_cpu();
>+ mask &= (1UL << cpu);
>+ num_cpus = hweight32(mask);
I guess you mean
mask &= ~(1UL << cpu);
or else num_cpus is always either 0 or 1 ...
>+ for (i = 0; i < NR_CPUS; i++) {
>+ if (cpu_online(i) && ((1UL << cpu) && mask))
This should be
if (cpu_online(i) && ((1UL << i) & mask))
or else the message is sent to all online CPUs anyway ...
Bye,
Ulrich
--
Dr. Ulrich Weigand
[email protected]
On Fri, 14 Feb 2003, Ulrich Weigand wrote:
> Zwane Mwaikambo wrote:
>
> >+ cpu = get_cpu();
> >+ mask &= (1UL << cpu);
> >+ num_cpus = hweight32(mask);
>
> I guess you mean
> mask &= ~(1UL << cpu);
> or else num_cpus is always either 0 or 1 ...
Hmm correct
> >+ for (i = 0; i < NR_CPUS; i++) {
> >+ if (cpu_online(i) && ((1UL << cpu) && mask))
>
> This should be
> if (cpu_online(i) && ((1UL << i) & mask))
> or else the message is sent to all online CPUs anyway ...
Correct again, thanks i'll fix that.
Zwane
--
function.linuxpower.ca
On Fri, 14 Feb 2003, Zwane Mwaikambo wrote:
> Correct again, thanks i'll fix that.
Rediffed;
Index: linux-2.5.60/arch/s390/kernel/smp.c
===================================================================
RCS file: /build/cvsroot/linux-2.5.60/arch/s390/kernel/smp.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 smp.c
--- linux-2.5.60/arch/s390/kernel/smp.c 10 Feb 2003 22:15:54 -0000 1.1.1.1
+++ linux-2.5.60/arch/s390/kernel/smp.c 15 Feb 2003 12:26:50 -0000
@@ -102,27 +102,34 @@
* in the system.
*/
-int smp_call_function (void (*func) (void *info), void *info, int nonatomic,
- int wait)
/*
- * [SUMMARY] Run a function on all other CPUs.
- * <func> The function to run. This must be fast and non-blocking.
- * <info> An arbitrary pointer to pass to the function.
- * <nonatomic> currently unused.
- * <wait> If true, wait (atomically) until function has completed on other CPUs.
- * [RETURNS] 0 on success, else a negative status code. Does not return until
- * remote CPUs are nearly ready to execute <<func>> or are or have executed.
+ * smp_call_function_on_cpu - Runs func on all processors in the mask
+ *
+ * @func: The function to run. This must be fast and non-blocking.
+ * @info: An arbitrary pointer to pass to the function.
+ * @wait: If true, wait (atomically) until function has completed on other CPUs.
+ * @mask: The bitmask of CPUs to call the function
+ *
+ * Returns 0 on success, else a negative status code. Does not return until
+ * remote CPUs are nearly ready to execute func or have executed it.
*
* You must not call this function with disabled interrupts or from a
* hardware interrupt handler or from a bottom half handler.
*/
+
+int smp_call_function_on_cpu (void (*func) (void *info), void *info, int wait,
+ unsigned long mask)
{
struct call_data_struct data;
- int cpus = num_online_cpus()-1;
+ int i, cpu, num_cpus;
- /* FIXME: get cpu lock -hc */
- if (cpus <= 0)
+ cpu = get_cpu();
+ mask &= ~(1UL << cpu);
+ num_cpus = hweight32(mask);
+ if (num_cpus == 0) {
+ put_cpu_no_resched();
return 0;
+ }
data.func = func;
data.info = info;
@@ -134,18 +141,26 @@
spin_lock(&call_lock);
call_data = &data;
/* Send a message to all other CPUs and wait for them to respond */
- smp_ext_bitcall_others(ec_call_function);
+ for (i = 0; i < NR_CPUS; i++) {
+ if (cpu_online(i) && ((1UL << i) && mask))
+ smp_ext_bitcall(i, ec_call_function);
+ }
/* Wait for response */
- while (atomic_read(&data.started) != cpus)
+ while (atomic_read(&data.started) != num_cpus)
barrier();
if (wait)
- while (atomic_read(&data.finished) != cpus)
+ while (atomic_read(&data.finished) != num_cpus)
barrier();
spin_unlock(&call_lock);
-
+ put_cpu_no_resched();
return 0;
+}
+
+int smp_call_function (void (*func) (void *info), void *info, int nonatomic, int wait)
+{
+ return smp_call_function_on_cpu(func, info, wait, cpu_online_map);
}
static inline void do_send_stop(void)
Zwane Mwaikambo wrote:
> + if (cpu_online(i) && ((1UL << i) && mask))
That's still '&&' instead of '&'.
Bye,
Ulrich
--
Dr. Ulrich Weigand
[email protected]
On Sat, 15 Feb 2003, Ulrich Weigand wrote:
> Zwane Mwaikambo wrote:
>
> > + if (cpu_online(i) && ((1UL << i) && mask))
>
> That's still '&&' instead of '&'.
*sigh*
Index: linux-2.5.60/arch/s390/kernel/smp.c
===================================================================
RCS file: /build/cvsroot/linux-2.5.60/arch/s390/kernel/smp.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 smp.c
--- linux-2.5.60/arch/s390/kernel/smp.c 10 Feb 2003 22:15:54 -0000 1.1.1.1
+++ linux-2.5.60/arch/s390/kernel/smp.c 15 Feb 2003 15:36:07 -0000
@@ -102,27 +102,34 @@
* in the system.
*/
-int smp_call_function (void (*func) (void *info), void *info, int nonatomic,
- int wait)
/*
- * [SUMMARY] Run a function on all other CPUs.
- * <func> The function to run. This must be fast and non-blocking.
- * <info> An arbitrary pointer to pass to the function.
- * <nonatomic> currently unused.
- * <wait> If true, wait (atomically) until function has completed on other CPUs.
- * [RETURNS] 0 on success, else a negative status code. Does not return until
- * remote CPUs are nearly ready to execute <<func>> or are or have executed.
+ * smp_call_function_on_cpu - Runs func on all processors in the mask
+ *
+ * @func: The function to run. This must be fast and non-blocking.
+ * @info: An arbitrary pointer to pass to the function.
+ * @wait: If true, wait (atomically) until function has completed on other CPUs.
+ * @mask: The bitmask of CPUs to call the function
+ *
+ * Returns 0 on success, else a negative status code. Does not return until
+ * remote CPUs are nearly ready to execute func or have executed it.
*
* You must not call this function with disabled interrupts or from a
* hardware interrupt handler or from a bottom half handler.
*/
+
+int smp_call_function_on_cpu (void (*func) (void *info), void *info, int wait,
+ unsigned long mask)
{
struct call_data_struct data;
- int cpus = num_online_cpus()-1;
+ int i, cpu, num_cpus;
- /* FIXME: get cpu lock -hc */
- if (cpus <= 0)
+ cpu = get_cpu();
+ mask &= ~(1UL << cpu);
+ num_cpus = hweight32(mask);
+ if (num_cpus == 0) {
+ put_cpu_no_resched();
return 0;
+ }
data.func = func;
data.info = info;
@@ -134,18 +141,26 @@
spin_lock(&call_lock);
call_data = &data;
/* Send a message to all other CPUs and wait for them to respond */
- smp_ext_bitcall_others(ec_call_function);
+ for (i = 0; i < NR_CPUS; i++) {
+ if (cpu_online(i) && ((1UL << i) & mask))
+ smp_ext_bitcall(i, ec_call_function);
+ }
/* Wait for response */
- while (atomic_read(&data.started) != cpus)
+ while (atomic_read(&data.started) != num_cpus)
barrier();
if (wait)
- while (atomic_read(&data.finished) != cpus)
+ while (atomic_read(&data.finished) != num_cpus)
barrier();
spin_unlock(&call_lock);
-
+ put_cpu_no_resched();
return 0;
+}
+
+int smp_call_function (void (*func) (void *info), void *info, int nonatomic, int wait)
+{
+ return smp_call_function_on_cpu(func, info, wait, cpu_online_map);
}
static inline void do_send_stop(void)
Zwane Mwaikambo wrote:
> On Sat, 15 Feb 2003, Ulrich Weigand wrote:
> > That's still '&&' instead of '&'.
>
> *sigh*
Hmm. I think this code still has a problem. If the caller
passes in a mask containing bits for offline CPUs, those will
be counted here
> + num_cpus = hweight32(mask);
but there will be no external interrupt generated for those,
and thus this loop
> + while (atomic_read(&data.started) != num_cpus)
will never terminate ...
Bye,
Ulrich
--
Dr. Ulrich Weigand
[email protected]
On Sat, 15 Feb 2003, Ulrich Weigand wrote:
> Hmm. I think this code still has a problem. If the caller
> passes in a mask containing bits for offline CPUs, those will
> be counted here
It would be a bug in the caller, this is a primitive really. If the caller
is calling this with random bitmasks they are probably making errors
elsewhere too. This is also the behaviour of the Alpha version, which has
been around before this patch.
> > + num_cpus = hweight32(mask);
>
> but there will be no external interrupt generated for those,
> and thus this loop
>
> > + while (atomic_read(&data.started) != num_cpus)
>
> will never terminate ...
The following cpu_online call only goes as far as avoiding IPI'ing to
nonexistent cpus, anything more would be spoonfeeding the caller, i prefer
garbage in, garbage out.
for (i = 0; i < NR_CPUS; i++) {
if (cpu_online(i) && ((1UL << i) & mask))
smp_ext_bitcall(i, ec_call_function);
}
Thanks,
Zwane
--
function.linuxpower.ca
Zwane Mwaikambo wrote:
> It would be a bug in the caller, this is a primitive really. If the caller
> is calling this with random bitmasks they are probably making errors
> elsewhere too. This is also the behaviour of the Alpha version, which has
> been around before this patch.
Well, either this is a requirement on the caller or it isn't. I guess
it is fine to make this requirement, but then ...
> The following cpu_online call only goes as far as avoiding IPI'ing to
> nonexistent cpus, anything more would be spoonfeeding the caller, i prefer
> garbage in, garbage out.
>
> for (i = 0; i < NR_CPUS; i++) {
> if (cpu_online(i) && ((1UL << i) & mask))
> smp_ext_bitcall(i, ec_call_function);
> }
... this test is quite pointless as the routine will hang shortly
anyway. In fact is appears to be rather misleading as it can give
the casual reader the impression that offline CPUs are properly
cared for. I'd suggest to either
- make the routine really safe by doing something like
mask &= cpu_online_mask;
at the beginning
or else
- lose the cpu_online test
But apart from this cosmetic issue, there is still a real problem:
smp_ext_bitcall can fail due to SIGP returning a busy condition;
smp_ext_bitcall_others would have retried until the busy condition
is gone. This means your version can actually lose signals and
deadlock. You should do something like
while (smp_ext_bitcall(i, ec_call_function) == sigp_busy)
udelay(10);
B.t.w as you are removing the only caller of smp_ext_bitcall_others,
you might as well delete the function itself.
All those comments apply likewise to the s390x version.
Bye,
Ulrich
--
Dr. Ulrich Weigand
[email protected]
On Sat, 15 Feb 2003, Ulrich Weigand wrote:
> ... this test is quite pointless as the routine will hang shortly
> anyway. In fact is appears to be rather misleading as it can give
> the casual reader the impression that offline CPUs are properly
> cared for. I'd suggest to either
>
> - make the routine really safe by doing something like
> mask &= cpu_online_mask;
> at the beginning
If the caller is calling smp_call_function_on_cpu directly (in
contrast to calling it via smp_call_function) they probably
are targetting a specific group of processors so they have also probably
done a check of some sort for cpus online, or are explicitely targeting 1
cpu.
> or else
>
> - lose the cpu_online test
This could be achieved if s390 (or if we had a generic one, this is
another story...) had a for_each_cpu type iterator, which would also
cover aforementioned mask &= cpu_online_map issue, but as an aside, it is
harder to track down lockups from things like IPIs to invalid cpus than a busy loop
waiting for num_cpus.
> But apart from this cosmetic issue, there is still a real problem:
> smp_ext_bitcall can fail due to SIGP returning a busy condition;
> smp_ext_bitcall_others would have retried until the busy condition
> is gone. This means your version can actually lose signals and
> deadlock. You should do something like
>
> while (smp_ext_bitcall(i, ec_call_function) == sigp_busy)
> udelay(10);
Thanks i wasn't aware of that, i'll add that.
> B.t.w as you are removing the only caller of smp_ext_bitcall_others,
> you might as well delete the function itself.
>
> All those comments apply likewise to the s390x version.
Thanks,
Zwane
--
function.linuxpower.ca
Zwane Mwaikambo wrote:
> This could be achieved if s390 (or if we had a generic one, this is
> another story...) had a for_each_cpu type iterator, which would also
> cover aforementioned mask &= cpu_online_map issue, but as an aside, it is
> harder to track down lockups from things like IPIs to invalid cpus than a busy loop
> waiting for num_cpus.
I'm not sure I understand what you mean here. In any case, at least
on S/390, doing a SIGP to an invalid CPU will simply get you an
'not operational' indication (condition code 3), so in fact the
only problem *is* the busy loop on num_cpus ...
Bye,
Ulrich
--
Dr. Ulrich Weigand
[email protected]
On Sat, 15 Feb 2003, Ulrich Weigand wrote:
> I'm not sure I understand what you mean here. In any case, at least
> on S/390, doing a SIGP to an invalid CPU will simply get you an
> 'not operational' indication (condition code 3), so in fact the
> only problem *is* the busy loop on num_cpus ...
On other architectures it can get nasty if you send an interprocessor
interrupt out onto the bus for an invalid cpu destination.
Since not having the online map will be a problem for you I'll add the
mask = ... & cpu_online_map to cover the concerns you have.
Thanks,
Zwane
--
function.linuxpower.ca