linux-kernel - Re: [PATCH] Remove WARN_ON_ONCE in __smp_call_function_single and smp_call_function

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sat, 05 Jan 2013 16:31:05 +0800
From:	Li Zhong <zhong@...ux.vnet.ibm.com>
To:	Hui Zhu <teawater@...il.com>
Cc:	Peter Zijlstra <peterz@...radead.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Michal Nazarewicz <mina86@...a86.org>,
	"Srivatsa S. Bhat" <srivatsa.bhat@...ux.vnet.ibm.com>,
	linux-kernel@...r.kernel.org, Zhu Yanhai <gaoyang.zyh@...bao.com>,
	yan@...ux.vnet.ibm.com,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
Subject: Re: [PATCH] Remove WARN_ON_ONCE in __smp_call_function_single and
 smp_call_function_single

On Sat, 2013-01-05 at 12:26 +0800, Hui Zhu wrote:
> The comments of these WARN_ON_ONCE said "Can deadlock when called with
> interrupts disabled".
> But I didn't find anything that will block this function.

It seems that if two cpus try to IPI each other with wait equals true,
they will deadlock if interrupts are disabled, because neither of them
can take the interrupt. 

If wait equals false, it seems deadlock is still possible, for example,
if smp_call_function_single() is called from interrupt context, then one
cpu might
	-->call smp_call_function_single()
	-->csd_lock()
	-->be interrupted before sending the ipi out
	-->in interrupt context, call smp_call_function_single() again
	-->csd_lock_wait() forever in csd_lock() 

I'm not sure whether it is possible that irqs be enabled in interrupt
context? If so, maybe we also need check that if wait equals false?

Thanks, Zhong

> So I think maybe this WARN_ON_ONCE is not need now.  So I post a patch for that.
> 
> Signed-off-by: Hui Zhu <teawater@...il.com>
> Signed-off-by: Zhu Yanhai <gaoyang.zyh@...bao.com>
> 
> --- a/kernel/smp.c
> +++ b/kernel/smp.c
> @@ -303,25 +303,13 @@ int smp_call_function_single(int cpu, sm
>  		.flags = 0,
>  	};
>  	unsigned long flags;
> -	int this_cpu;
>  	int err = 0;
> 
>  	/*
>  	 * prevent preemption and reschedule on another processor,
>  	 * as well as CPU removal
>  	 */
> -	this_cpu = get_cpu();
> -
> -	/*
> -	 * 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) {
> +	if (cpu == get_cpu()) {
>  		local_irq_save(flags);
>  		func(info);
>  		local_irq_restore(flags);
> @@ -409,17 +397,7 @@ void __smp_call_function_single(int cpu,
>  	unsigned int this_cpu;
>  	unsigned long flags;
> 
> -	this_cpu = get_cpu();
> -	/*
> -	 * 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);
> -
> -	if (cpu == this_cpu) {
> +	if (cpu == get_cpu()) {
>  		local_irq_save(flags);
>  		data->func(data->info);
>  		local_irq_restore(flags);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/