[lkml]   [2007]   [Jun]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [patch] i386/x86_64: smp_call_function locking inconsistency
    Hi Heiko,

    On 6/7/07, Heiko Carstens <> wrote:
    > > 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.

    I'll fix the necessary patch for x86_64, in that case.

    > > 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.

    Exactly. I was only mentioning that the smp_call_function*
    of x86{_64} were safe anyway (but the smp_processor_id()
    that would've preceded it need not have been, of course).

    > 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.

    Indeed -- this was doubly problematic because the un-safeness
    was because of smp_processor_id() as well as the (eventual)
    access of cpu_online_map (via smp_call_function() ->
    num_online_cpus()) ... thanks for letting me know about this.

    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to
    More majordomo info at
    Please read the FAQ at

     \ /
      Last update: 2007-06-07 19:07    [W:0.031 / U:12.216 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site