Jeremy Fitzhardinge wrote:

Prarit Bhargava wrote:
  

quoted

Jeremy Fitzhardinge wrote:
  
    

quoted

Prarit Bhargava wrote:
  
    
      

quoted

I'd like to see this patch implement/fix touch_cpu_softlockup_watchdog
and touch_softlockup_watchdog to mimic touch_nmi_watchdog's behaviour.
    
      
        

Why?  Is that more correct?  It seems to me that you're interested in
whether a specific CPU has gone and locked up.  If touching the watchdog
  
makes it update all CPU timestamps, then you'll hide the fact that other
CPUs have locked up, won't it?

  
    
      

In case of misuse, yes.  But there are cases where we know that all CPUs 
will have softlockup issues, such as when doing a "big" sysrq-t dump.  
When doing the sysrq-t we take the tasklist_lock which prevents all 
other CPUs from scheduling -- this leads to bogus softlockup messages, 
so we need to reset everyone's watchdog just before releasing the 
tasklist_lock.

Another question -- are you going to expose disable/enable_watchdog to 
other subsystems?  Or are you going to expose touch_softlockup_watchdog?
    

Well, it depends on who turns up. 

My first thought is to export both the global enable/disable interfaces
and touch_softlockup_watchdog.  But on second thoughts maybe
touch_softlockup_watchdog is completely redundant, since you'd only do
  

IMO, if you export enable/disable you should drop touch_softlockup_watchdog.

it if you're holding off timer interrupts, but the lockup only gets
reported if timer interrupts are enabled (in other words, the best it
can tell you is "you locked up for a while there", which isn't terribly
useful).  

I like to think of the softlockup watchdog letting me know that a cpu 
hasn't scheduled in a long time.

So perhaps this can just be dropped.  I haven't looked at the
users to see what they're really trying to achieve.
  

I've looked through much of that code for my previous patch ;)

AFAICT the uses appear to be cases where we _know_ that  we've gone away 
for a while and need to reset the timer.

But there were some exceptions:  touch_nmi_watchdog erroneously calls 
touch_softlockup_watchdog.  In fact, touch_nmi_watchdog is trying to 
touch all cpus softlockup watchdogs, not just one.

IIRC, There was an extra call to touch_softlockup_watchdog which wasn't 
necessary IIRC...

Look at my previous patch where I replaced touch_softlockup_watchdog 
with touch_cpu_softlockup_watchdog ...

The enable/disable interfaces are more generally useful in that you can
say "I *know* I'm going to go away for a while, so don't bother
reporting it".

    J