linux-kernel - Re: Preventing IPI sending races in arch code

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20131125135725.GA10022@twins.programming.kicks-ass.net>
Date:	Mon, 25 Nov 2013 14:57:25 +0100
From:	Peter Zijlstra <peterz@...radead.org>
To:	Vineet Gupta <Vineet.Gupta1@...opsys.com>
Cc:	"linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>,
	Gilad Ben-Yossef <gilad@...yossef.com>,
	Noam Camus <noamc@...hip.com>,
	David Daney <david.daney@...ium.com>,
	James Hogan <james.hogan@...tec.com>,
	thomas Gleixner <tglx@...utronix.de>,
	lkml <linux-kernel@...r.kernel.org>,
	Richard Kuo <rkuo@...eaurora.org>
Subject: Re: Preventing IPI sending races in arch code

On Mon, Nov 25, 2013 at 01:35:38PM +0000, Vineet Gupta wrote:
> On 11/25/2013 05:57 PM, Peter Zijlstra wrote:

> > 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?
> 
> I see your point. So in receiver, it is OK to de-assert the IPI before processing
> the msg itself.
> 
> Actually your code seems to be optimizing away asserting an IPI, if sender already
> had a pending msg (assuming we retain the xchg loop in receiver). Was that an
> intended optimization - or just a side effect of your code ;-)

No, full intention. As you mentioned you wanted to avoid sending IPIs
where none were needed.

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

No, even with my code; the receiving end should look like:

void handle_ipi(struct pt_regs *regs)
{
	u32 pending;

	ipi_clear(irq);

	pending = xchg(this_cpu_ptr(ipi_bits), 0);

	while (pending) {
		bit = ffs(pending);

		/* handle bit */

		pending &= ~(1U << bit);
	}
}

So while it does have a while() loop, it only does a single xchg().

The version you showed before had the xchg() in the loop, that is not
required.
--
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/