On Thu, 2013-08-08 at 07:50 +0200, leroy christophe wrote:
> Le 26/06/2013 01:04, Scott Wood a écrit :
> > What happens if there's a race? If another CPU updates wdt_last_ping in
> > parallel, then you could see wdt_last_ping greater than the value you
> > read for jiffies. Since this is an unsigned comparison, it will fail to
> > call keepalive. You might get saved by pinging it twice as often as
> > necessary, but you shouldn't rely on that.
> Euh ... This watchdog is integrated inside the CPU, so there is no
> chance that any external CPU get access to it.
Hmm, it looks like mpc8641d (which is the only multi-core SoC among
mpc8xx/mpc83xx/mpc86xx) does not have this watchdog, even though mpc8610 does.
So pretend I said "what if you get preempted?". :-)
> >> + mpc8xxx_wdt_keepalive();
> >> + /* We're pinging it twice faster than needed, to be sure. */
> >> + mod_timer(&wdt_timer, jiffies + HZ * hw_timo_sec / 2);
> >> + }
> >> +}
> >> +
> >> +static void mpc8xxx_wdt_sw_keepalive(void)
> >> +{
> >> + wdt_last_ping = jiffies;
> >> + mpc8xxx_wdt_timer_ping(0);
> >> }
> > This isn't new with this patch, but it looks like
> > mpc8xxx_wdt_keepalive() can be called either from timer context, or with
> > interrupts enabled... yet it uses a bare spin_lock() rather than an
> > irq-safe version. This should be fixed.
> Ok, I'll propose another patch for that. Indeed, is the spin_lock needed
> at all ? If we get two writes interleaved, it will make it anyway.
I suppose... I don't like relying on things like that, though.
-Scott
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
