On Tue, 27 Mar 2007 00:12:53 -0700 Jeremy Fitzhardinge <[EMAIL PROTECTED]> wrote:
> Eric Dumazet wrote: > > Jeremy Fitzhardinge a écrit : > > > >> +static DEFINE_PER_CPU(unsigned long long, touch_timestamp); > > > > ... > > > >> void touch_softlockup_watchdog(void) > >> { > >> - __raw_get_cpu_var(touch_timestamp) = jiffies; > >> + __raw_get_cpu_var(touch_timestamp) = sched_clock(); > >> } > > > > Not very clear if this is safe on 32bit, since this is not anymore > > atomic. > > Hm, good point. Don't think it matters very much. These values are > per-cpu, and if an interrupt happens between the word updates and the > intermediate values causes a timeout, then it was pretty marginal > anyway. I guess the worst case is if the low-word gets written first, > and it goes from a high value to low, then it could be sampled as if > time had gone back by up to ~4 seconds. > > I'll give it another look. OK thanks. I noticed another 'not clear' bit in your second patch : void softlockup_enable(void) { touch_softlockup_watchdog(); wmb(); /* update timestamp before enable */ __get_cpu_var(enabled) = 1; } Are you sure wmb() is needed here ? I think a barrier() (compiler barrier) should be enough. If not, a nice comment would help too :) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/