linux-kernel - Re: tick/broadcast: Make movement of broadcast hrtimer robust against hotplug

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: <20150120060913.A5EF31402DE@ozlabs.org>
Date:	Tue, 20 Jan 2015 17:09:13 +1100 (AEDT)
From:	Michael Ellerman <mpe@...erman.id.au>
To:	Preeti U Murthy <preeti@...ux.vnet.ibm.com>, tglx@...utronix.de
Cc:	aik@...abs.ru, shreyas@...ux.vnet.ibm.com,
	linux-kernel@...r.kernel.org, anton@...ba.org,
	linuxppc-dev@...ts.ozlabs.org
Subject: Re: tick/broadcast: Make movement of broadcast hrtimer robust against hotplug

On Mon, 2015-19-01 at 10:26:48 UTC, Preeti U Murthy wrote:
> Today if a cpu handling broadcasting of wakeups goes offline, it hands over

It's *the* cpu handling broadcasting of wakeups right? ie. there's only ever
one at a time.

> the job of broadcasting to another cpu in the CPU_DEAD phase. 

I think that would be clearer as "to another cpu, when the cpu going offline
reaches the CPU_DEAD state."

Otherwise it can read as "another cpu (which is) in the CPU_DEAD phase", which
is not what you mean - I think.

> The CPU_DEAD notifiers are run only after the offline cpu sets its state as
> CPU_DEAD. Meanwhile, the kthread doing the offline is scheduled out while

The kthread which is running on a different cpu from either of the first two
cpus you've mentioned.

> waiting for this transition by queuing a timer. This is fatal because if the
> cpu on which this kthread was running has no other work queued on it, it can
> re-enter deep idle state, since it sees that a broadcast cpu still exists.
> However the broadcast wakeup will never come since the cpu which was handling
> it is offline, and this cpu never wakes up to see this because its in deep
> idle state.

Which cpu is "this cpu"? I think you mean the one running the kthread which is
doing the offline, but it's not 100% clear.

> Fix this by setting the broadcast timer to a max value so as to force the cpus
> entering deep idle states henceforth to freshly nominate the broadcast cpu. More
> importantly this has to be done in the CPU_DYING phase so that it is visible to
> all cpus right after exiting stop_machine, which is when they can re-enter idle.
> This ensures that handover of the broadcast duty falls in place on offline, without
> having to do it explicitly.

OK, I don't know the code well enough to say if that's the right fix.

You say:

+	/* This allows fresh nomination of broadcast cpu */
+	bc->next_event.tv64 = KTIME_MAX;

Is that all it does? I see that check in several places in the code.


I assume we're expecting Thomas to merge this?

If so it's probably worth mentioning that it fixes a bug we are seeing on
machines in the wild. So it'd be nice if it went into 3.19 and/or gets sent to
stable.

cheers
--
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/