Re: [patch 10/29] lockup_detector/perf: Prevent cpu hotplug deadlock

From	Don Zickus <dzickus@redhat.com>
Newsgroups	linux.kernel
Subject	Re: [patch 10/29] lockup_detector/perf: Prevent cpu hotplug deadlock
Date	2017-09-05 17:00 +0200
Message-ID	<umnuz-40l-29@gated-at.bofh.it> (permalink)
References	<uksf0-2Jn-3@gated-at.bofh.it> <uksf0-2Jn-21@gated-at.bofh.it> <ukZui-Os-15@gated-at.bofh.it> <ukZND-VC-1@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

On Fri, Sep 01, 2017 at 09:29:07PM +0200, Thomas Gleixner wrote:
> On Fri, 1 Sep 2017, Don Zickus wrote:
> > On Thu, Aug 31, 2017 at 09:16:08AM +0200, Thomas Gleixner wrote:
> > > The following deadlock is possible in the watchdog hotplug code:
> > > 
> > >   cpus_write_lock()
> > >     ...
> > >       takedown_cpu()
> > >         smpboot_park_threads()
> > >           smpboot_park_thread()
> > >             kthread_park()
> > >               ->park() := watchdog_disable()
> > >                 watchdog_nmi_disable()
> > >                   perf_event_release_kernel();
> > >                     put_event()
> > >                       _free_event()
> > >                         ->destroy() := hw_perf_event_destroy()
> > >                           x86_release_hardware()
> > >                             release_ds_buffers()
> > >                               get_online_cpus()
> > > 
> > > when a per cpu watchdog perf event is destroyed which drops the last
> > > reference to the PMU hardware. The cleanup code there invokes
> > > get_online_cpus() which instantly deadlocks because the hotplug percpu
> > > rwsem is write locked.
> > 
> > The main reason perf_event_release_kernel is in this path is because the
> > oprofile folks complained they couldn't use the perf counters when the
> > nmi_watchdog was disabled on the command line.
> 
> If the nmi watchdog is disabled on the command line then there are no
> counters claimed at all.

Ah, I see it now.  When you park all the threads, you clear the cpumask
which then calls the lockup detector_cleanup to release all perf_counters
that are no longer used by the cpumask.

Further reading this code, I see how the code covers various race conditions
we tried solving in the past.

Thanks!

Cheers,
Don

Thread

[patch 00/29] lockup_detector: Cure hotplug deadlocks and replace  duct tape Thomas Gleixner <tglx@linutronix.de> - 2017-08-31 09:40 +0200
  [patch 02/29] perf/x86/intel: Sanitize PMU HT bug workaround Thomas Gleixner <tglx@linutronix.de> - 2017-08-31 09:40 +0200
  [patch 25/29] lockup_detector: Implement init time detection of perf Thomas Gleixner <tglx@linutronix.de> - 2017-08-31 09:40 +0200
  [patch 19/29] lockup_detector: Cleanup header mess Thomas Gleixner <tglx@linutronix.de> - 2017-08-31 09:40 +0200
  [patch 26/29] lockup_detector/perf: Implement CPU enable replacement Thomas Gleixner <tglx@linutronix.de> - 2017-08-31 09:40 +0200
  [patch 24/29] lockup_detector/perf: Implement init time perf  validation Thomas Gleixner <tglx@linutronix.de> - 2017-08-31 09:40 +0200
    Re: [patch 24/29] lockup_detector/perf: Implement init time perf  validation Don Zickus <dzickus@redhat.com> - 2017-09-07 18:00 +0200
  [patch 10/29] lockup_detector/perf: Prevent cpu hotplug deadlock Thomas Gleixner <tglx@linutronix.de> - 2017-08-31 09:40 +0200
    Re: [patch 10/29] lockup_detector/perf: Prevent cpu hotplug deadlock Don Zickus <dzickus@redhat.com> - 2017-09-01 21:10 +0200
      Re: [patch 10/29] lockup_detector/perf: Prevent cpu hotplug  deadlock Thomas Gleixner <tglx@linutronix.de> - 2017-09-01 21:30 +0200
        Re: [patch 10/29] lockup_detector/perf: Prevent cpu hotplug deadlock Don Zickus <dzickus@redhat.com> - 2017-09-05 17:00 +0200
  [patch 21/29] lockup_detector: Cleanup sysctl variable name space Thomas Gleixner <tglx@linutronix.de> - 2017-08-31 09:40 +0200
  [patch 18/29] lockup_detector: Further simplify sysctl handling Thomas Gleixner <tglx@linutronix.de> - 2017-08-31 09:40 +0200
  [patch 23/29] lockup_detector: Get rid of the racy update loop Thomas Gleixner <tglx@linutronix.de> - 2017-08-31 09:40 +0200
  [patch 27/29] lockup_detector: Use new perf CPU enable mechanism Thomas Gleixner <tglx@linutronix.de> - 2017-08-31 09:40 +0200
  [patch 13/29] lockup_detector: Cleanup the ifdef maze Thomas Gleixner <tglx@linutronix.de> - 2017-08-31 09:40 +0200
  [patch 06/29] lockup_detector: Rework cpu hotplug locking Thomas Gleixner <tglx@linutronix.de> - 2017-08-31 09:40 +0200
  [patch 14/29] lockup_detector: Split out cpumask write function Thomas Gleixner <tglx@linutronix.de> - 2017-08-31 09:40 +0200
  [patch 11/29] lockup_detector: Remove park_in_progress hackery Thomas Gleixner <tglx@linutronix.de> - 2017-08-31 09:40 +0200
    Re: [patch 11/29] lockup_detector: Remove park_in_progress hackery Peter Zijlstra <peterz@infradead.org> - 2017-09-04 14:20 +0200
      Re: [patch 11/29] lockup_detector: Remove park_in_progress hackery Don Zickus <dzickus@redhat.com> - 2017-09-05 17:20 +0200
        Re: [patch 11/29] lockup_detector: Remove park_in_progress hackery Thomas Gleixner <tglx@linutronix.de> - 2017-09-05 17:50 +0200
    Re: [patch 11/29] lockup_detector: Remove park_in_progress hackery Thomas Gleixner <tglx@linutronix.de> - 2017-09-05 16:00 +0200
      [patch V2 11/29] lockup_detector: Remove park_in_progress  obfuscation Thomas Gleixner <tglx@linutronix.de> - 2017-09-05 21:20 +0200
  [patch 04/29] parisc: Use lockup_detector_stop() Thomas Gleixner <tglx@linutronix.de> - 2017-08-31 09:40 +0200
  [patch 29/29] lockup_detector: Cleanup hotplug locking mess Thomas Gleixner <tglx@linutronix.de> - 2017-08-31 09:40 +0200
  [patch 12/29] lockup_detector: Cleanup stub functions Thomas Gleixner <tglx@linutronix.de> - 2017-08-31 09:40 +0200
  [patch 03/29] lockup_detector: Provide interface to stop from  poweroff() Thomas Gleixner <tglx@linutronix.de> - 2017-08-31 09:40 +0200
  [patch 20/29] lockup_detector/sysctl: Get rid of the ifdeffery Thomas Gleixner <tglx@linutronix.de> - 2017-08-31 09:40 +0200
  [patch 08/29] lockup_detector: Mark hardlockup_detector_disable()  __init Thomas Gleixner <tglx@linutronix.de> - 2017-08-31 09:40 +0200
  [patch 17/29] lockup_detector: Get rid of the thread teardown/setup  dance Thomas Gleixner <tglx@linutronix.de> - 2017-08-31 09:40 +0200
    Re: [patch 17/29] lockup_detector: Get rid of the thread  teardown/setup dance Don Zickus <dzickus@redhat.com> - 2017-09-01 21:10 +0200
      Re: [patch 17/29] lockup_detector: Get rid of the thread teardown/setup  dance Thomas Gleixner <tglx@linutronix.de> - 2017-09-01 21:50 +0200
  [patch 15/29] smpboot/threads: Avoid runtime allocation Thomas Gleixner <tglx@linutronix.de> - 2017-08-31 09:40 +0200
  [patch 07/29] lockup_detector: Rename watchdog_proc_mutex Thomas Gleixner <tglx@linutronix.de> - 2017-08-31 09:40 +0200
  [patch 16/29] lockup_detector: Create new thread handling  infrastructure Thomas Gleixner <tglx@linutronix.de> - 2017-08-31 09:40 +0200
  [patch 22/29] lockup_detector: Make watchdog_nmi_reconfigure() two  stage Thomas Gleixner <tglx@linutronix.de> - 2017-08-31 09:40 +0200
  [patch 01/29] hardlockup_detector: Provide interface to stop/restart  perf events Thomas Gleixner <tglx@linutronix.de> - 2017-08-31 09:40 +0200
    Re: [patch 01/29] hardlockup_detector: Provide interface to  stop/restart perf events Borislav Petkov <bp@alien8.de> - 2017-09-06 18:20 +0200
  [patch 05/29] lockup_detector: Remove broken suspend/resume interfaces Thomas Gleixner <tglx@linutronix.de> - 2017-08-31 09:50 +0200
  Re: [patch 00/29] lockup_detector: Cure hotplug deadlocks and  replace duct tape Don Zickus <dzickus@redhat.com> - 2017-09-01 00:20 +0200
    Re: [patch 00/29] lockup_detector: Cure hotplug deadlocks and  replace duct tape Nicholas Piggin <npiggin@gmail.com> - 2017-09-01 06:50 +0200
    Re: [patch 00/29] lockup_detector: Cure hotplug deadlocks and replace  duct tape Thomas Gleixner <tglx@linutronix.de> - 2017-09-01 11:20 +0200
  Re: [patch 00/29] lockup_detector: Cure hotplug deadlocks and  replace duct tape Don Zickus <dzickus@redhat.com> - 2017-09-07 18:10 +0200

csiph-web