• Petr Mladek's avatar
    watchdog: reliable handling of timestamps · 0f90b88d
    Petr Mladek authored
    Commit 9bf3bc94 ("watchdog: cleanup handling of false positives")
    tried to handle a virtual host stopped by the host a more
    straightforward and cleaner way.
    
    But it introduced a risk of false softlockup reports.  The virtual host
    might be stopped at any time, for example between
    kvm_check_and_clear_guest_paused() and is_softlockup().  As a result,
    is_softlockup() might read the updated jiffies and detects a softlockup.
    
    A solution might be to put back kvm_check_and_clear_guest_paused() after
    is_softlockup() and detect it.  But it would put back the cycle that
    complicates the logic.
    
    In fact, the handling of all the timestamps is not reliable.  The code
    does not guarantee when and how many times the timestamps are read.  For
    example, "period_ts" might be touched anytime also from NMI and re-read in
    is_softlockup().  It works just by chance.
    
    Fix all the problems by making the code even more explicit.
    
    1. Make sure that "now" and "period_ts" timestamps are read only once.
       They might be changed at anytime by NMI or when the virtual guest is
       stopped by the host.  Note that "now" timestamp does this implicitly
       because "jiffies" is marked volatile.
    
    2. "now" time must be read first.  The state of "period_ts" will
       decide whether it will be used or the period will get restarted.
    
    3. kvm_check_and_clear_guest_paused() must be called before reading
       "period_ts".  It touches the variable when the guest was stopped.
    
    As a result, "now" timestamp is used only when the watchdog was not
    touched and the guest not stopped in the meantime.  "period_ts" is
    restarted in all other situations.
    
    Link: https://lkml.kernel.org/r/YKT55gw+RZfyoFf7@alley
    Fixes: 9bf3bc94 ("watchdog: cleanup handling of false positives")
    Signed-off-by: default avatarPetr Mladek <pmladek@suse.com>
    Reported-by: default avatarSergey Senozhatsky <senozhatsky@chromium.org>
    Reviewed-by: default avatarSergey Senozhatsky <senozhatsky@chromium.org>
    Cc: Peter Zijlstra <peterz@infradead.org>
    Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
    Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
    0f90b88d
watchdog.c 20.4 KB