Re: [tip:x86/pat] generic-ipi: Allow cpus not yet online to call smp_call_function with irqs disabled

[Nick Piggin]

On Fri, Aug 21, 2009 at 11:51:50PM +0000, Suresh B wrote:
> Commit-ID:  269c861baa2fe7c114c3bc7831292758d29eb336
> Gitweb:     http://git.kernel.org/tip/269c861baa2fe7c114c3bc7831292758d29eb336
> Author:     Suresh Siddha <suresh.b.siddha@xxxxxxxxx>
> AuthorDate: Wed, 19 Aug 2009 18:05:35 -0700
> Committer:  H. Peter Anvin <hpa@xxxxxxxxx>
> CommitDate: Fri, 21 Aug 2009 16:25:43 -0700
> 
> generic-ipi: Allow cpus not yet online to call smp_call_function with irqs disabled
> 
> Because of deadlock possiblities smp_call_function() is not allowed to
> be called with interrupts disabled. Add an exception for the cpu not
> yet online, as no one else can send smp call function interrupt to this
> cpu that is not yet online and as such deadlock condition is not possible.
> 
> Signed-off-by: Suresh Siddha <suresh.b.siddha@xxxxxxxxx>
> Acked-by: Nick Piggin <npiggin@xxxxxxx>
> Signed-off-by: H. Peter Anvin <hpa@xxxxxxxxx>

I don't know if we should allow the use of smp_call_function here --
only call_function_single. CPU hotplug code is required to set up
some call_function data and if the cpu is offline then it might not
be set up correctly.

Also, I would say that we should just restrict this to wait==1 case
because in that case the stack can trivially be used for data. In
the wait==0 case, it is more complex. In the current implementation
it should be OK (it uses per-cpu data), but we've used kmalloc
there in the past, which probably wouldn't work either.


> ---
>  kernel/smp.c |   40 ++++++++++++++++++++++++++++++++++------
>  1 files changed, 34 insertions(+), 6 deletions(-)
> 
> diff --git a/kernel/smp.c b/kernel/smp.c
> index ad63d85..2accdf6 100644
> --- a/kernel/smp.c
> +++ b/kernel/smp.c
> @@ -177,6 +177,11 @@ void generic_smp_call_function_interrupt(void)
>  	int cpu = get_cpu();
>  
>  	/*
> +	 * Shouldn't receive this interrupt on a cpu that is not yet online.
> +	 */
> +	WARN_ON_ONCE(!cpu_online(cpu));
> +
> +	/*
>  	 * Ensure entry is visible on call_function_queue after we have
>  	 * entered the IPI. See comment in smp_call_function_many.
>  	 * If we don't have this, then we may miss an entry on the list
> @@ -230,6 +235,11 @@ void generic_smp_call_function_single_interrupt(void)
>  	unsigned int data_flags;
>  	LIST_HEAD(list);
>  
> +	/*
> +	 * Shouldn't receive this interrupt on a cpu that is not yet online.
> +	 */
> +	WARN_ON_ONCE(!cpu_online(smp_processor_id()));
> +
>  	spin_lock(&q->lock);
>  	list_replace_init(&q->list, &list);
>  	spin_unlock(&q->lock);
> @@ -285,8 +295,14 @@ int smp_call_function_single(int cpu, void (*func) (void *info), void *info,
>  	 */
>  	this_cpu = get_cpu();
>  
> -	/* Can deadlock when called with interrupts disabled */
> -	WARN_ON_ONCE(irqs_disabled() && !oops_in_progress);
> +	/*
> +	 * Can deadlock when called with interrupts disabled.
> +	 * We allow cpu's that are not yet online though, as no one else can
> +	 * send smp call function interrupt to this cpu and as such deadlocks
> +	 * can't happen.
> +	 */
> +	WARN_ON_ONCE(cpu_online(this_cpu) && irqs_disabled()
> +		     && !oops_in_progress);
>  
>  	if (cpu == this_cpu) {
>  		local_irq_save(flags);
> @@ -329,8 +345,14 @@ void __smp_call_function_single(int cpu, struct call_single_data *data,
>  {
>  	csd_lock(data);
>  
> -	/* Can deadlock when called with interrupts disabled */
> -	WARN_ON_ONCE(wait && irqs_disabled() && !oops_in_progress);
> +	/*
> +	 * Can deadlock when called with interrupts disabled.
> +	 * We allow cpu's that are not yet online though, as no one else can
> +	 * send smp call function interrupt to this cpu and as such deadlocks
> +	 * can't happen.
> +	 */
> +	WARN_ON_ONCE(cpu_online(smp_processor_id()) && wait && irqs_disabled()
> +		     && !oops_in_progress);
>  
>  	generic_exec_single(cpu, data, wait);
>  }
> @@ -365,8 +387,14 @@ void smp_call_function_many(const struct cpumask *mask,
>  	unsigned long flags;
>  	int cpu, next_cpu, this_cpu = smp_processor_id();
>  
> -	/* Can deadlock when called with interrupts disabled */
> -	WARN_ON_ONCE(irqs_disabled() && !oops_in_progress);
> +	/*
> +	 * Can deadlock when called with interrupts disabled.
> +	 * We allow cpu's that are not yet online though, as no one else can
> +	 * send smp call function interrupt to this cpu and as such deadlocks
> +	 * can't happen.
> +	 */
> +	WARN_ON_ONCE(cpu_online(this_cpu) && irqs_disabled()
> +		     && !oops_in_progress);
>  
>  	/* So, what's a CPU they want? Ignoring this one. */
>  	cpu = cpumask_first_and(mask, cpu_online_mask);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/