On Mon, 2013-11-25 at 13:35 +0000, Vineet Gupta wrote: > Before reading ur email I was coding something like below: > > void arch_send_ipi(int cpu, int type) > { > u32 *pending_ptr = per_cpu_ptr(ipi_bits, cpu); > > while (cmpxchg(pending_ptr, 0, 1 << type) != 0) > cpu_relax(); > > raise_ipi(cpu); > } So you would have blocked the sender while there was already a pending IPI on the target ? Why ? The optimization proposed by Peter is actually the only interesting change here, without it the existing set_bit was perfectly fine. Remember that set_bit is atomic. Ben. > But obviously your version is nicer due to optimization, unless I'm over-analyzing it. > > > > 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. > > Not with your version of sender, since we need it as described above. > > >> 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. > > Right we need it with ur version of sender. Bit don't with my simplistic one. > > -Vineet > -- > To unsubscribe from this list: send the line "unsubscribe linux-arch" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-arch" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html