linux-toradex.git/include/linux/nmi.h, branch v4.14-rc7

powerpc/watchdog: Make use of watchdog_nmi_probe()

2017-10-04T08:53:54+00:00

The rework of the core hotplug code triggers the WARN_ON in start_wd_cpu()
on powerpc because it is called multiple times for the boot CPU.

The first call is via:

  start_wd_on_cpu+0x80/0x2f0
  watchdog_nmi_reconfigure+0x124/0x170
  softlockup_reconfigure_threads+0x110/0x130
  lockup_detector_init+0xbc/0xe0
  kernel_init_freeable+0x18c/0x37c
  kernel_init+0x2c/0x160
  ret_from_kernel_thread+0x5c/0xbc

And then again via the CPU hotplug registration:

  start_wd_on_cpu+0x80/0x2f0
  cpuhp_invoke_callback+0x194/0x620
  cpuhp_thread_fun+0x7c/0x1b0
  smpboot_thread_fn+0x290/0x2a0
  kthread+0x168/0x1b0
  ret_from_kernel_thread+0x5c/0xbc

This can be avoided by setting up the cpu hotplug state with nocalls and
move the initialization to the watchdog_nmi_probe() function. That
initializes the hotplug callbacks without invoking the callback and the
following core initialization function then configures the watchdog for the
online CPUs (in this case CPU0) via softlockup_reconfigure_threads().

Reported-and-tested-by: Michael Ellerman 
Signed-off-by: Thomas Gleixner 
Acked-by: Michael Ellerman 
Cc: Benjamin Herrenschmidt 
Cc: Nicholas Piggin 
Cc: linuxppc-dev@lists.ozlabs.org

watchdog/core, powerpc: Replace watchdog_nmi_reconfigure()

2017-10-04T08:53:53+00:00

The recent cleanup of the watchdog code split watchdog_nmi_reconfigure()
into two stages. One to stop the NMI and one to restart it after
reconfiguration. That was done by adding a boolean 'run' argument to the
code, which is functionally correct but not necessarily a piece of art.

Replace it by two explicit functions: watchdog_nmi_stop() and
watchdog_nmi_start().

Fixes: 6592ad2fcc8f ("watchdog/core, powerpc: Make watchdog_nmi_reconfigure() two stage")
Requested-by: Linus 'Nursing his pet-peeve' Torvalds 
Signed-off-by: Thomas 'Mopping up garbage' Gleixner 
Acked-by: Michael Ellerman 
Cc: Peter Zijlstra 
Cc: Don Zickus 
Cc: Benjamin Herrenschmidt 
Cc: Nicholas Piggin 
Cc: linuxppc-dev@lists.ozlabs.org
Link: http://lkml.kernel.org/r/alpine.DEB.2.20.1710021957480.2114@nanos

watchdog/hardlockup/perf: Implement CPU enable replacement

2017-09-14T09:41:08+00:00

watchdog_nmi_enable() is an unparseable mess, Provide a clean perf specific
implementation, which will be used when the existing setup/teardown mess is
replaced.

Signed-off-by: Thomas Gleixner 
Reviewed-by: Don Zickus 
Cc: Andrew Morton 
Cc: Borislav Petkov 
Cc: Chris Metcalf 
Cc: Linus Torvalds 
Cc: Nicholas Piggin 
Cc: Peter Zijlstra 
Cc: Sebastian Siewior 
Cc: Ulrich Obergfell 
Link: http://lkml.kernel.org/r/20170912194148.180215498@linutronix.de
Signed-off-by: Ingo Molnar

watchdog/hardlockup/perf: Implement init time perf validation

2017-09-14T09:41:08+00:00

The watchdog tries to create perf events even after it figured out that
perf is not functional or the requested event is not supported.

That's braindead as this can be done once at init time and if not supported
the NMI watchdog can be turned off unconditonally.

Implement the perf hardlockup detector functionality for that. This creates
a new event create function, which will replace the unholy mess of the
existing one in later patches.

Signed-off-by: Thomas Gleixner 
Reviewed-by: Don Zickus 
Cc: Andrew Morton 
Cc: Borislav Petkov 
Cc: Chris Metcalf 
Cc: Linus Torvalds 
Cc: Nicholas Piggin 
Cc: Peter Zijlstra 
Cc: Sebastian Siewior 
Cc: Ulrich Obergfell 
Link: http://lkml.kernel.org/r/20170912194148.019090547@linutronix.de
Signed-off-by: Ingo Molnar

watchdog/core, powerpc: Make watchdog_nmi_reconfigure() two stage

2017-09-14T09:41:07+00:00

Both the perf reconfiguration and the powerpc watchdog_nmi_reconfigure()
need to be done in two steps.

     1) Stop all NMIs
     2) Read the new parameters and start NMIs

Right now watchdog_nmi_reconfigure() is a combination of both. To allow a
clean reconfiguration add a 'run' argument and split the functionality in
powerpc.

Signed-off-by: Thomas Gleixner 
Reviewed-by: Don Zickus 
Cc: Andrew Morton 
Cc: Benjamin Herrenschmidt 
Cc: Borislav Petkov 
Cc: Chris Metcalf 
Cc: Linus Torvalds 
Cc: Michael Ellerman 
Cc: Nicholas Piggin 
Cc: Peter Zijlstra 
Cc: Sebastian Siewior 
Cc: Ulrich Obergfell 
Cc: linuxppc-dev@lists.ozlabs.org
Link: http://lkml.kernel.org/r/20170912194147.862865570@linutronix.de
Signed-off-by: Ingo Molnar

watchdog/sysctl: Clean up sysctl variable name space

2017-09-14T09:41:07+00:00

Reflect that these variables are user interface related and remove the
whitespace damage in the sysctl table while at it.

Signed-off-by: Thomas Gleixner 
Reviewed-by: Don Zickus 
Cc: Andrew Morton 
Cc: Borislav Petkov 
Cc: Chris Metcalf 
Cc: Linus Torvalds 
Cc: Nicholas Piggin 
Cc: Peter Zijlstra 
Cc: Sebastian Siewior 
Cc: Ulrich Obergfell 
Link: http://lkml.kernel.org/r/20170912194147.783210221@linutronix.de
Signed-off-by: Ingo Molnar

watchdog/sysctl: Get rid of the #ifdeffery

2017-09-14T09:41:07+00:00

The sysctl of the nmi_watchdog file prevents writes by setting:

    min = max = 0

if none of the users is enabled. That involves ifdeffery and is competely
non obvious.

If none of the facilities is enabeld, then the file can simply be made read
only. Move the ifdeffery into the header and use a constant for file
permissions.

Signed-off-by: Thomas Gleixner 
Reviewed-by: Don Zickus 
Cc: Andrew Morton 
Cc: Borislav Petkov 
Cc: Chris Metcalf 
Cc: Linus Torvalds 
Cc: Nicholas Piggin 
Cc: Peter Zijlstra 
Cc: Sebastian Siewior 
Cc: Ulrich Obergfell 
Link: http://lkml.kernel.org/r/20170912194147.706073616@linutronix.de
Signed-off-by: Ingo Molnar

watchdog/core: Clean up header mess

2017-09-14T09:41:07+00:00

Having the same #ifdef in various places does not make it more
readable. Collect stuff into one place.

Signed-off-by: Thomas Gleixner 
Reviewed-by: Don Zickus 
Cc: Andrew Morton 
Cc: Borislav Petkov 
Cc: Chris Metcalf 
Cc: Linus Torvalds 
Cc: Nicholas Piggin 
Cc: Peter Zijlstra 
Cc: Sebastian Siewior 
Cc: Ulrich Obergfell 
Link: http://lkml.kernel.org/r/20170912194147.627096864@linutronix.de
Signed-off-by: Ingo Molnar

watchdog/core: Remove the park_in_progress obfuscation

2017-09-14T09:41:05+00:00

Commit:

  b94f51183b06 ("kernel/watchdog: prevent false hardlockup on overloaded system")

tries to fix the following issue:

proc_write()
   set_sample_period()    <--- New sample period becoms visible
			  <----- Broken starts
   proc_watchdog_update()
     watchdog_enable_all_cpus()		watchdog_hrtimer_fn()
     update_watchdog_all_cpus()		   restart_timer(sample_period)
        watchdog_park_threads()

					thread->park()
					  disable_nmi()
			  <----- Broken ends

The reason why this is broken is that the update of the watchdog threshold
becomes immediately effective and visible for the hrtimer function which
uses that value to rearm the timer. But the NMI/perf side still uses the
old value up to the point where it is disabled. If the rate has been
lowered then the NMI can run fast enough to 'detect' a hard lockup because
the timer has not fired due to the longer period.

The patch 'fixed' this by adding a variable:

proc_write()
   set_sample_period()
					<----- Broken starts
   proc_watchdog_update()
     watchdog_enable_all_cpus()		watchdog_hrtimer_fn()
     update_watchdog_all_cpus()		   restart_timer(sample_period)
         watchdog_park_threads()
	  park_in_progress = 1
					<----- Broken ends
				        nmi_watchdog()
					  if (park_in_progress)
					     return;

The only effect of this variable was to make the window where the breakage
can hit small enough that it was not longer observable in testing. From a
correctness point of view it is a pointless bandaid which merily papers
over the root cause: the unsychronized update of the variable.

Looking deeper into the related code pathes unearthed similar problems in
the watchdog_start()/stop() functions.

 watchdog_start()
	perf_nmi_event_start()
	hrtimer_start()

 watchdog_stop()
	hrtimer_cancel()
	perf_nmi_event_stop()

In both cases the call order is wrong because if the tasks gets preempted
or the VM gets scheduled out long enough after the first call, then there is
a chance that the next NMI will see a stale hrtimer interrupt count and
trigger a false positive hard lockup splat.

Get rid of park_in_progress so the code can be gradually deobfuscated and
pruned from several layers of duct tape papering over the root cause,
which has been either ignored or not understood at all.

Once this is removed the underlying problem will be fixed by rewriting the
proc interface to do a proper synchronized update.

Address the start/stop() ordering problem as well by reverting the call
order, so this part is at least correct now.

Signed-off-by: Thomas Gleixner 
Reviewed-by: Don Zickus 
Cc: Andrew Morton 
Cc: Borislav Petkov 
Cc: Chris Metcalf 
Cc: Linus Torvalds 
Cc: Nicholas Piggin 
Cc: Peter Zijlstra 
Cc: Sebastian Siewior 
Cc: Ulrich Obergfell 
Link: http://lkml.kernel.org/r/alpine.DEB.2.20.1709052038270.2393@nanos
Signed-off-by: Ingo Molnar

watchdog/hardlockup/perf: Prevent CPU hotplug deadlock

2017-09-14T09:41:05+00:00

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.

To solve this add a deferring mechanism:

  cpus_write_lock()
			   kthread_park()
			    watchdog_nmi_disable(deferred)
			      perf_event_disable(event);
			      move_event_to_deferred(event);
			   ....
  cpus_write_unlock()
  cleaup_deferred_events()
    perf_event_release_kernel()

This is still properly serialized against concurrent hotplug via the
cpu_add_remove_lock, which is held by the task which initiated the hotplug
event.

This is also used to handle event destruction when the watchdog threads are
parked via other mechanisms than CPU hotplug.

Analyzed-by: Peter Zijlstra 

Reported-by: Borislav Petkov 
Signed-off-by: Thomas Gleixner 
Reviewed-by: Don Zickus 
Cc: Andrew Morton 
Cc: Chris Metcalf 
Cc: Linus Torvalds 
Cc: Nicholas Piggin 
Cc: Peter Zijlstra 
Cc: Sebastian Siewior 
Cc: Ulrich Obergfell 
Link: http://lkml.kernel.org/r/20170912194146.884469246@linutronix.de
Signed-off-by: Ingo Molnar