On Tue, 27 Mar 2007 00:12:53 -0700
Jeremy Fitzhardinge <jeremy@xxxxxxxx> 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 :)
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/virtualization
