linux-kernel - Re: [PATCH 1/3] stop_machine: ensure that a queued callback will be called before cpu_stop

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: <20151014190356.GA8905@redhat.com>
Date:	Wed, 14 Oct 2015 21:03:56 +0200
From:	Oleg Nesterov <oleg@...hat.com>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	heiko.carstens@...ibm.com, Tejun Heo <tj@...nel.org>,
	Ingo Molnar <mingo@...nel.org>, Rik van Riel <riel@...hat.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] stop_machine: ensure that a queued callback will
	be called before cpu_stop_park()

On 10/14, Peter Zijlstra wrote:
>
> On Thu, Oct 08, 2015 at 04:51:31PM +0200, Oleg Nesterov wrote:
> > cpu_stop_queue_work() checks stopper->enabled before it queues the
> > work, but ->enabled == T can only guarantee cpu_stop_signal_done()
> > if we race with cpu_down().
> >
> > This is not enough for stop_two_cpus() or stop_machine(), they will
> > deadlock if multi_cpu_stop() won't be called by one of the target
> > CPU's. stop_machine/stop_cpus are fine, they rely on stop_cpus_mutex.
> > But stop_two_cpus() has to check cpu_active() to avoid the same race
> > with hotplug, and this check is very unobvious and probably not even
> > correct if we race with cpu_up().
> >
> > Change cpu_down() pass to clear ->enabled before cpu_stopper_thread()
> > flushes the pending ->works and returns with KTHREAD_SHOULD_PARK set.
> >
> > Note also that smpboot_thread_call() calls cpu_stop_unpark() which
> > sets enabled == T at CPU_ONLINE stage, so this CPU can't go away until
> > cpu_stopper_thread() is called at least once. This all means that if
> > cpu_stop_queue_work() succeeds, we know that work->fn() will be called.
>
> This hard relies on the fact that cpu_down uses stop machine, right?

Not really.

> IIRC part of the hotplug rework Thomas is doing is geared towards
> breaking away from stop machine. There is nothing fundamental about
> hot-unplug that requires stop machine.

cpu_down() should park/kill/whatever the percpu stopper thread anyway.
And this path should clear ->enabled, it can also flush the pending
works.

And we need this anyway even if cpu_down() won't use stop_machine(),
I think.

Oleg.

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