On Tue, 2008-06-10 at 10:44 -0500, James Bottomley wrote:

On Tue, 2008-06-10 at 15:51 +0100, Catalin Marinas wrote:

quoted

I was thinking whether this condition can be removed and allow the
smp_call_function*() to be called with IRQs disabled. At a quick look,
it seems to be possible if the csd_flag_wait() function calls the IPI
handlers directly when the IRQs are disabled (see the patch below).

[...]

quoted

Please let me know what you think or whether deadlocks are still
possible (or any other solution apart from hardware fixes :-)). Thanks.

I don't see how your proposal fixes the deadlocks.  The problem is that
on a lot of arch's IPIs are normal interrupts.  If interrupts are
disabled, you don't see them.

ARM uses normal interrupts for IPIs as well.

The deadlock scenario is CPU1 enters smp_call_function() with IRQ's
disabled as CPU2 does the same thing and spins on the call_lock.  Now
CPU1 is waiting for an ack for its IPI to CPU2, but CPU2 will never see
the IPI until it enables interrupts.

I can see in the generic IPI patches that the call_function_lock is only
held for list_add_tail_rcu and not while waiting for the other CPU to
complete (both arch_send_call_function_ipi and csd_flag_wait are outside
the locking region).

The patch I posted polls for an incoming IPI in the csd_flag_wait()
function if the interrupts are disabled so that it clears the wait flag
even if it doesn't get the IPI. The disadvantage might be a spurious IPI
(but I can leave with this). If interrupts are enabled, there is no
drawback, apart from a call to irq_disabled().

One way to mitigate the effects of this is to enable interrupts if the
architecture code finds the call_lock (x86 implementation) held against
it, then re-disable before trying to get the lock again.  But really, in
order to make smp_call_function work in interrupt disabled sections, the
interrupt handler has to be modified to bar all non-IPI interrupts until
the critical section is over (otherwise there's no point allowing it
with disabled interrupts because an smp_call_function becomes a de facto
interrupt enable again).  If you really want to see how something like
this works, the voyager code has it (because interrupt disabling in the
VIC is expensive).  But it's quite a lot of code ...

I still think it can be less complicated that this. I'll look at Paul's
post to get some ideas. As I said, I need this functionality on current
ARM SMP systems, even if it means implementing it separately.

Thanks.

-- 
Catalin