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
| ||
|
Date: Thu, 7 Jun 2007 18:27:43 +0200 From: Heiko Carstens <heiko.carstens@...ibm.com> To: Satyam Sharma <satyam.sharma@...il.com> Cc: Jan Glauber <jan.glauber@...ibm.com>, David Miller <davem@...emloft.net>, akpm@...l.org, mingo@...e.hu, ak@...e.de, schwidefsky@...ibm.com, linux-kernel@...r.kernel.org, Alan Cox <alan@...hat.com> Subject: Re: [patch] i386/x86_64: smp_call_function locking inconsistency > >So either all spin_lock_bh's should be converted to spin_lock, > >which would limit smp_call_function()/smp_call_function_single() > >to process context & irqs enabled. > >Or the spin_lock's could be converted to spin_lock_bh which would > >make it possible to call these two functions even if in softirq > >context. AFAICS this should be safe. > > Actually, I agree with David and Andi here: > > On 2/9/07, David Miller <davem@...emloft.net> wrote: > >I think it's logically simpler if we disallow smp_call_function*() > >from any kind of asynchronous context. But I'm sure your driver > >has a true need for this for some reason. > > and > > On 2/9/07, Andi Kleen <ak@...e.de> wrote: > >I'm not so sure. Perhaps drop _bh in both and stick a WARN_ON_ONCE in > >to catch the cases? > > Replacing the _bh variants and making smp_call_function{_single} > illegal from all contexts but process is fine for x86_64, as we > don't really have any driver that needs to use this from softirq > context in the x86_64 tree. This means it becomes dissimilar to > s390, but similar to powerpc, mips, alpha, sparc64 semantics. > I'll prepare and submit a patch for the same, shortly. Calling an smp_call_* function from any context but process context is a bug. We didn't notice this initially when we used smp_call_function from softirq context... until we deadlocked ;) So s390 is the same as any other architecture wrt this. > On 2/9/07, Heiko Carstens <heiko.carstens@...ibm.com> wrote: > >Another thing that comes into my mind is smp_call_function together > >with cpu hotplug. Who is responsible that preemption and with that > >cpu hotplug is disabled? > >Is it the caller or smp_call_function itself? > >If it's smp_call_function then s390 would be broken, since > >then we would have > >int cpus = num_online_cpus()-1; > >in preemptible context... I agree: what a mess :) > > and > > On 2/9/07, Jan Glauber <jan.glauber@...ibm.com> wrote: > >If preemption must be disabled before smp_call_function() we should have > >the same semantics for all smp_call_function_* variants. > > I don't see any CPU hotplug / preemption disabling issues here. > Note that both smp_call_function() and smp_call_function_single() > on x86_64 acquire the call_lock spinlock before using cpu_online_map > via num_online_cpus(). And spin_lock() does preempt_disable() on both > SMP and !SMP, so we're safe. [ But we're not explicitly disabling > preemption and depending on spin_lock() instead, so a comment would > be in order? ] Calling smp_call_function_single() with preemption enabled is pointless. You might be scheduled on the cpu you want to send an IPI to and get -EBUSY as return... If cpu hotplug is enabled the target cpu might even be gone when smp_call_function_single() gets executed. Avi Kivity has already a patch which introduces an on_cpu() function which looks quite like on_each_cpu(). That way you don't have to open code this stuff over and over again: preempt_disable(); if (cpu == smp_processor_id()) func(); else smp_call_function_single(...); preempt_enable(); There are already quite a few of these around. - 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/
Powered by blists - more mailing lists