| Message ID | 20120727214024.GA10249@mudshark.cambridge.arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jul 27, 2012 at 10:40:24PM +0100, Will Deacon wrote: > On Fri, Jul 27, 2012 at 10:06:37PM +0100, Russell King - ARM Linux wrote: > > We support booting a kernel on systems with or without SMP support, even > > with a SMP kernel. When the kernel is booted on such a system, it is > > undefined whether smp_cross_call() is a valid function pointer. > > So let's define it to point at a dummy function which explodes with a BUG if > the cpumask passed in isn't empty. That allows SMP kernels to do things like > `cross call to all other cores' without having to worry about whether there > are any other cores or not. We should not be even attempting to do any cross calls when there aren't any other CPUs in the system - that's rather the point of leaving it as a NULL pointer so it does explode on such systems. . the scheduler won't call smp_send_reschedule() when there are no other CPUs in the system. . timer ticks won't be broadcast to other CPUs if there are no other CPUs in the system. . function calls will not be issued to other CPUs if there are no other CPUs in the system. There is only one case where this doesn't happen, and that's the shutdown path. For instance, smp_call_function*() all check that the target CPUs are marked online before the arch code is requested to issue an IPI to the target CPU. The only place which is missing this check is smp_send_stop(). Now, we could make machine_shutdown() do this instead: if (is_smp()) smp_send_stop(); but why bother calling into smp_send_stop(), and ultimately end up with the _only_ site which calls smp_cross_call() with an empty CPU mask when we can avoid the call when the CPU mask is empty - and prevent this special case of smp_cross_call() being called with an empty CPU mask ever occuring.
On Sat, Jul 28, 2012 at 11:08:31AM +0100, Russell King - ARM Linux wrote: > On Fri, Jul 27, 2012 at 10:40:24PM +0100, Will Deacon wrote: > > On Fri, Jul 27, 2012 at 10:06:37PM +0100, Russell King - ARM Linux wrote: > > > We support booting a kernel on systems with or without SMP support, even > > > with a SMP kernel. When the kernel is booted on such a system, it is > > > undefined whether smp_cross_call() is a valid function pointer. > > > > So let's define it to point at a dummy function which explodes with a BUG if > > the cpumask passed in isn't empty. That allows SMP kernels to do things like > > `cross call to all other cores' without having to worry about whether there > > are any other cores or not. > > We should not be even attempting to do any cross calls when there aren't > any other CPUs in the system - that's rather the point of leaving it as > a NULL pointer so it does explode on such systems. > > . the scheduler won't call smp_send_reschedule() when there are no other > CPUs in the system. > . timer ticks won't be broadcast to other CPUs if there are no other CPUs > in the system. > . function calls will not be issued to other CPUs if there are no other > CPUs in the system. > > There is only one case where this doesn't happen, and that's the shutdown > path. Ok, that's a pretty compelling argument you've got there :) > For instance, smp_call_function*() all check that the target CPUs are > marked online before the arch code is requested to issue an IPI to the > target CPU. Yes, thanks for making the case for the original patch. Consistency is the most important thing with these APIs, so I'll go full circle and offer my ack for the original patch: Acked-by: Will Deacon <will.deacon@arm.com> The next question is: who's putting this into the patch system? Will
On Sat, Jul 28, 2012 at 2:11 PM, Will Deacon <will.deacon@arm.com> wrote: > On Sat, Jul 28, 2012 at 11:08:31AM +0100, Russell King - ARM Linux wrote: >> On Fri, Jul 27, 2012 at 10:40:24PM +0100, Will Deacon wrote: >> > On Fri, Jul 27, 2012 at 10:06:37PM +0100, Russell King - ARM Linux wrote: >> > > We support booting a kernel on systems with or without SMP support, even >> > > with a SMP kernel. When the kernel is booted on such a system, it is >> > > undefined whether smp_cross_call() is a valid function pointer. >> > >> > So let's define it to point at a dummy function which explodes with a BUG if >> > the cpumask passed in isn't empty. That allows SMP kernels to do things like >> > `cross call to all other cores' without having to worry about whether there >> > are any other cores or not. >> >> We should not be even attempting to do any cross calls when there aren't >> any other CPUs in the system - that's rather the point of leaving it as >> a NULL pointer so it does explode on such systems. >> >> . the scheduler won't call smp_send_reschedule() when there are no other >> CPUs in the system. >> . timer ticks won't be broadcast to other CPUs if there are no other CPUs >> in the system. >> . function calls will not be issued to other CPUs if there are no other >> CPUs in the system. >> >> There is only one case where this doesn't happen, and that's the shutdown >> path. > > Ok, that's a pretty compelling argument you've got there :) > >> For instance, smp_call_function*() all check that the target CPUs are >> marked online before the arch code is requested to issue an IPI to the >> target CPU. > > Yes, thanks for making the case for the original patch. Consistency is the > most important thing with these APIs, so I'll go full circle and offer my > ack for the original patch: > > Acked-by: Will Deacon <will.deacon@arm.com> > > The next question is: who's putting this into the patch system? > > Will Hi, I just put the patch with Will's Acked-by on the patch system. It was accepted as patch 7480/1 [1]. The patch is on top of today's linux-next (next-20120727). Thanks a lot and best regards, Javier [1] http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=7480/1
On Sat, Jul 28, 2012 at 04:26:28PM +0200, Javier Martinez Canillas wrote: > On Sat, Jul 28, 2012 at 2:11 PM, Will Deacon <will.deacon@arm.com> wrote: > > Acked-by: Will Deacon <will.deacon@arm.com> > > > > The next question is: who's putting this into the patch system? > > > > Will > > Hi, > > I just put the patch with Will's Acked-by on the patch system. It was > accepted as patch 7480/1 [1]. > > The patch is on top of today's linux-next (next-20120727). > > Thanks a lot and best regards, > Javier > > [1] http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=7480/1 Isn't this a candidate for stable kernels?
On Sun, Jul 29, 2012 at 11:31 PM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Sat, Jul 28, 2012 at 04:26:28PM +0200, Javier Martinez Canillas wrote: >> On Sat, Jul 28, 2012 at 2:11 PM, Will Deacon <will.deacon@arm.com> wrote: >> > Acked-by: Will Deacon <will.deacon@arm.com> >> > >> > The next question is: who's putting this into the patch system? >> > >> > Will >> >> Hi, >> >> I just put the patch with Will's Acked-by on the patch system. It was >> accepted as patch 7480/1 [1]. >> >> The patch is on top of today's linux-next (next-20120727). >> >> Thanks a lot and best regards, >> Javier >> >> [1] http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=7480/1 > > Isn't this a candidate for stable kernels? Yes, I forgot it, sorry for that. Please let me know if I need to resubmit the patch cc'ing stable. Best regards, Javier
On Mon, Jul 30, 2012 at 11:22:51AM +0200, Javier Martinez Canillas wrote: > Yes, I forgot it, sorry for that. > > Please let me know if I need to resubmit the patch cc'ing stable. I can just add it to the commit.
On Mon, Jul 30, 2012 at 4:35 PM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Mon, Jul 30, 2012 at 11:22:51AM +0200, Javier Martinez Canillas wrote: >> Yes, I forgot it, sorry for that. >> >> Please let me know if I need to resubmit the patch cc'ing stable. > > I can just add it to the commit. Perfect, thanks a lot and sorry for missing that. Best regards, Javier
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c index 2c7217d..ffa411f 100644 --- a/arch/arm/kernel/smp.c +++ b/arch/arm/kernel/smp.c @@ -329,7 +329,13 @@ void __init smp_prepare_cpus(unsigned int max_cpus) } } -static void (*smp_cross_call)(const struct cpumask *, unsigned int); +static void dummy_smp_cross_call(const struct cpumask *mask, unsigned int irq) +{ + BUG_ON(!cpumask_empty(mask)); +} + +static void (*smp_cross_call)(const struct cpumask *, unsigned int) = + dummy_smp_cross_call; void __init set_smp_cross_call(void (*fn)(const struct cpumask *, unsigned int)) {