lkml.org 
[lkml]   [2013]   [Nov]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: Preventing IPI sending races in arch code
On Mon, Nov 25, 2013 at 05:00:18PM +0530, Vineet Gupta wrote:
> While we are at it, I wanted to confirm another potential race (ARC/blackfin..)
> The IPI handler clears the interrupt before atomically-read-n-clear the msg word.
>
> do_IPI
> plat_smp_ops.ipi_clear(irq);
> while ((pending = xchg(&ipi_data->bits, 0) != 0)
> find_next_bit(....)
> switch(next-msg)
>
> Depending on arch this could lead to an immediate IPI interrupt, and again
> ipi_data->bits could get out of syn with IPI senders.

I'm obviously lacking in platform knowledge here, what does that
ipi_clear() actually do? Tell the platform the interrupt has arrived and
it can stop asserting the line?

So sure, then someone can again assert the interrupt, but given we just
established a protocol for raising the thing; namely something like
this:

void arch_send_ipi(int cpu, int type)
{
u32 *pending_ptr = per_cpu_ptr(ipi_bits, cpu);
u32 new, old;

do {
new = old = *pending_ptr;
new |= 1U << type;
} while (cmpxchg(pending_ptr, old, new) != old)

if (!old) /* only raise the actual IPI if we set the first bit */
raise_ipi(cpu);
}

Who would re-assert it if we have !0 pending?

Also, the above can be thought of as a memory ordering issue:

STORE pending
MB /* implied by cmpxchg */
STORE ipi /* raise the actual thing */

In that case the other end must be:

LOAD ipi
MB /* implied by xchg */
LOAD pending

Which is what your code seems to do.

> IMO the while loop is
> completely useless specially if IPIs are not coalesced in h/w.

Agreed, the while loops seems superfluous.

> And we need to move
> the xchg ahead of ACK'ing the IPI
>
> do_IPI
> pending = xchg(&ipi_data->bits, 0);
> plat_smp_ops.ipi_clear(irq);
> while (ffs....)
> switch(next-msg)
> ...
>
> Does that look sane to you.

This I'm not at all certain of; continuing with the memory order analogy
this would allow for the case where we see 0 pending, set a bit, try and
raise the interrupt but then do not because its already assert.

And since you just removed the while() loop, we'll be left with a !0
pending vector and nobody processing it.


\
 
 \ /
  Last update: 2013-11-25 14:01    [W:0.052 / U:2.664 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site