Re: [patch 1/4] Ignore stolen time in the softlockup watchdog
Andrew Morton wrote: On Tue, 24 Apr 2007 13:00:49 -0700 Jeremy Fitzhardinge [EMAIL PROTECTED] wrote: Andrew Morton wrote: Well, it _is_ mysterious. Did you try to locate the code which failed? I got lost in macros and include files, and gave up very very easily. Stop hiding, Ingo. OK, I've managed to reproduce it. Removing the local_irq_save/restore from sched_clock() makes it go away, as I'd expect (otherwise it would really be magic). erm, why do you expect that? A local_irq_save()/local_irq_restore() pair shouldn't be affecting anything? Well, yes. I have no idea why it causes a problem. But other than that, sched_clock does absolutely nothing which would affect lockdep state. But given that it never seems to touch the softlockup during testing, I have no idea what difference it makes... To what softlockup are you referring, and what does that have to do with anything? You dropped this patch, Ignore stolen time in the softlockup watchdog because its presence triggers the lock tester errors. The only thing this patch does is use sched_clock() rather than jiffies to measure lockup time. It therefore appears, for some reason, that using sched_clock() in the softlockup code is making the lock-test fail. Since the lock test doesn't explicitly do any softlockup stuff, the connection must be implicit via sched_lock - but how, I can't imagine. Since sched_clock() itself looks perfectly OK, and the softlockup watchdog seems fine too, I can only conclude its a bug in the lock testing stuff. But I don't know what. J ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [patch 1/4] Ignore stolen time in the softlockup watchdog
Andrew Morton wrote: It's weird. And I don't think the locking selftest code calls sched_clock() (or any other time-related thing) at all, does it? I guess it ends up going through the scheduler, which does use it. But... shrug J ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [patch 1/4] Ignore stolen time in the softlockup watchdog
On Tue, 2007-04-24 at 22:59 +0200, Ingo Molnar wrote: * Daniel Walker [EMAIL PROTECTED] wrote: On Tue, 2007-04-24 at 13:24 -0700, Jeremy Fitzhardinge wrote: And sched_clock's use of local_irq_save/restore appears to be absolutely correct, so I think it must be triggering a bug in either the self-tests or lockdep itself. Why does sched_clock need to disable interrupts? i concur. To me it appears not absolutely correct that someone apparently added local_irq_save/restore to sched_clock(), but absolute madness. sched_clock() is _very_ performance-sensitive for the scheduler, do not mess with it. It looks like it's used in some sort of warp check, but only when jiffies is used .. So I'm totally stumped why it's in there.. Daniel ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [patch 1/4] Ignore stolen time in the softlockup watchdog
On Tuesday 24 April 2007 22:52:27 Daniel Walker wrote: On Tue, 2007-04-24 at 13:24 -0700, Jeremy Fitzhardinge wrote: And sched_clock's use of local_irq_save/restore appears to be absolutely correct, so I think it must be triggering a bug in either the self-tests or lockdep itself. Why does sched_clock need to disable interrupts? It's only used in the instable path which is kind of i already threw up my hands anyways I use it because when you transition from stable (TSC) to instable (jiffies) the only way to avoid the clock jumping backwards is to remember and update the last value. To avoid races with parallel cpufreq handlers or timer interrupts this small section needs to run with interrupts disabled. The alternative would be a seqlock, but people have complained about this earlier too so i judged irq disabling better. -Andi ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [patch 1/4] Ignore stolen time in the softlockup watchdog
On Tue, 2007-04-24 at 23:20 +0200, Andi Kleen wrote: On Tuesday 24 April 2007 22:52:27 Daniel Walker wrote: On Tue, 2007-04-24 at 13:24 -0700, Jeremy Fitzhardinge wrote: And sched_clock's use of local_irq_save/restore appears to be absolutely correct, so I think it must be triggering a bug in either the self-tests or lockdep itself. Why does sched_clock need to disable interrupts? It's only used in the instable path which is kind of i already threw up my hands anyways I use it because when you transition from stable (TSC) to instable (jiffies) the only way to avoid the clock jumping backwards is to remember and update the last value. To avoid races with parallel cpufreq handlers or timer interrupts this small section needs to run with interrupts disabled. Preemption is already disabled with the get_cpu_var() , so it seems like the timer interrupt is the only worry? I find it confusing that the access of jiffies_64 isn't protected from interrupts, it's only the per_cpu data which should already be protected by the get_cpu_var()/put_cpu_var .. Daniel ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
