Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1247139

Re: [PATCH 1/3] stop_machine: ensure that a queued callback will be called before cpu_stop_park()

Show all headers | View raw

On Wed, Oct 14, 2015 at 09:03:56PM +0200, Oleg Nesterov wrote:
> 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.

So the proposed patch does: ->enabled=false; park();, which can race
with if (->enabled) wake();

smpboot_thread_fn() will not call ->thread_fn() when should_park(), and
thus any pending work will not get flushed.

It only works now because the stopper task calls park(), which means
cpu_stopper_thread() will flush, but that very much relies on the
stopper thread calling park in itself.

Or I'm just terminally confused.. which is entirely possible.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[RFC][PATCH] sched: Start stopper early Peter Zijlstra <peterz@infradead.org> - 2015-10-07 10:50 +0200
  Re: [RFC][PATCH] sched: Start stopper early Oleg Nesterov <oleg@redhat.com> - 2015-10-07 14:40 +0200
    Re: [RFC][PATCH] sched: Start stopper early Peter Zijlstra <peterz@infradead.org> - 2015-10-07 14:40 +0200
      Re: [RFC][PATCH] sched: Start stopper early Oleg Nesterov <oleg@redhat.com> - 2015-10-07 15:30 +0200
        Re: [RFC][PATCH] sched: Start stopper early Oleg Nesterov <oleg@redhat.com> - 2015-10-07 15:30 +0200
        Re: Re: [RFC][PATCH] sched: Start stopper early kbuild test robot <lkp@intel.com> - 2015-10-07 15:40 +0200
  [PATCH 0/3] (Was: [RFC][PATCH] sched: Start stopper early) Oleg Nesterov <oleg@redhat.com> - 2015-10-08 17:00 +0200
    [PATCH 1/3] stop_machine: ensure that a queued callback will be  called before cpu_stop_park() Oleg Nesterov <oleg@redhat.com> - 2015-10-08 17:00 +0200
      Re: [PATCH 1/3] stop_machine: ensure that a queued callback will be  called before cpu_stop_park() Peter Zijlstra <peterz@infradead.org> - 2015-10-14 17:40 +0200
        Re: [PATCH 1/3] stop_machine: ensure that a queued callback will  be called before cpu_stop_park() Oleg Nesterov <oleg@redhat.com> - 2015-10-14 21:10 +0200
          Re: [PATCH 1/3] stop_machine: ensure that a queued callback will be  called before cpu_stop_park() Peter Zijlstra <peterz@infradead.org> - 2015-10-14 22:40 +0200
            Re: [PATCH 1/3] stop_machine: ensure that a queued callback will  be called before cpu_stop_park() Oleg Nesterov <oleg@redhat.com> - 2015-10-15 19:10 +0200
              Re: [PATCH 1/3] stop_machine: ensure that a queued callback will be  called before cpu_stop_park() Peter Zijlstra <peterz@infradead.org> - 2015-10-16 13:00 +0200
    [PATCH 3/3] stop_machine: change cpu_stop_queue_two_works() to  rely on stopper->enabled Oleg Nesterov <oleg@redhat.com> - 2015-10-08 17:00 +0200
      Re: [PATCH 3/3] stop_machine: change cpu_stop_queue_two_works() to  rely on stopper->enabled Peter Zijlstra <peterz@infradead.org> - 2015-10-08 17:10 +0200
        Re: [PATCH 3/3] stop_machine: change cpu_stop_queue_two_works() to  rely on stopper->enabled Oleg Nesterov <oleg@redhat.com> - 2015-10-08 18:10 +0200
          Re: [PATCH 3/3] stop_machine: change cpu_stop_queue_two_works() to  rely on stopper->enabled Oleg Nesterov <oleg@redhat.com> - 2015-10-08 18:20 +0200
      [PATCH v2 3/3] stop_machine: change cpu_stop_queue_two_works() to  rely on stopper->enabled Oleg Nesterov <oleg@redhat.com> - 2015-10-08 19:10 +0200
        Re: [PATCH v2 3/3] stop_machine: change cpu_stop_queue_two_works()  to rely on stopper->enabled Peter Zijlstra <peterz@infradead.org> - 2015-10-09 18:40 +0200
          Re: [PATCH v2 3/3] stop_machine: change cpu_stop_queue_two_works()  to rely on stopper->enabled Oleg Nesterov <oleg@redhat.com> - 2015-10-09 18:50 +0200
    [PATCH 2/3] stop_machine: introduce __cpu_stop_queue_work() and  cpu_stop_queue_two_works() Oleg Nesterov <oleg@redhat.com> - 2015-10-08 17:00 +0200
  Re: [RFC][PATCH] sched: Start stopper early Oleg Nesterov <oleg@redhat.com> - 2015-10-08 20:10 +0200
    Re: [RFC][PATCH] sched: Start stopper early Oleg Nesterov <oleg@redhat.com> - 2015-10-08 21:00 +0200
  [PATCH 0/3] make stopper threads more "selfparking" Oleg Nesterov <oleg@redhat.com> - 2015-10-09 18:10 +0200
    [PATCH 2/3] stop_machine: kill cpu_stop_threads->setup() and  cpu_stop_unpark() Oleg Nesterov <oleg@redhat.com> - 2015-10-09 18:10 +0200
    [PATCH 3/3] sched: start stopper early Oleg Nesterov <oleg@redhat.com> - 2015-10-09 18:10 +0200
      Re: [PATCH 3/3] sched: start stopper early Oleg Nesterov <oleg@redhat.com> - 2015-10-09 19:00 +0200
    [PATCH 1/3] stop_machine: kill smp_hotplug_thread->pre_unpark,  introduce stop_machine_unpark() Oleg Nesterov <oleg@redhat.com> - 2015-10-09 18:10 +0200
  Re: [RFC][PATCH] sched: Start stopper early Heiko Carstens <heiko.carstens@de.ibm.com> - 2015-10-16 10:30 +0200
    Re: [RFC][PATCH] sched: Start stopper early Peter Zijlstra <peterz@infradead.org> - 2015-10-16 12:00 +0200
      Re: [RFC][PATCH] sched: Start stopper early Heiko Carstens <heiko.carstens@de.ibm.com> - 2015-10-16 14:10 +0200

csiph-web