linux-kernel - Re: [patch 1/2] Ignore stolen time in the softlockup watchdog

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 27 Mar 2007 09:50:57 +0200
From:	Eric Dumazet <dada1@...mosbay.com>
To:	Jeremy Fitzhardinge <jeremy@...p.org>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Linux Kernel <linux-kernel@...r.kernel.org>,
	virtualization@...ts.osdl.org, Ingo Molnar <mingo@...e.hu>,
	Thomas Gleixner <tglx@...utronix.de>,
	john stultz <johnstul@...ibm.com>,
	Zachary Amsden <zach@...are.com>,
	James Morris <jmorris@...ei.org>,
	Dan Hecht <dhecht@...are.com>,
	Paul Mackerras <paulus@...ba.org>,
	Martin Schwidefsky <schwidefsky@...ibm.com>,
	Prarit Bhargava <prarit@...hat.com>,
	Chris Lalancette <clalance@...hat.com>,
	Rick Lindsley <ricklind@...ibm.com>
Subject: Re: [patch 1/2] Ignore stolen time in the softlockup watchdog

On Tue, 27 Mar 2007 00:12:53 -0700
Jeremy Fitzhardinge <jeremy@...p.org> 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 majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/