Re: [PATCH][2.5][8/14] smp_call_function_on_cpu - s390

[Ulrich Weigand]

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
  [e-mail]