linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] fix heartbeat
@ 2002-02-09  1:46 Val Henson
  2002-02-09  6:50 ` Paul Mackerras
  0 siblings, 1 reply; 4+ messages in thread
From: Val Henson @ 2002-02-09  1:46 UTC (permalink / raw)
  To: linuxppc-dev


The heartbeat function will not execute properly on Gemini without
this patch. (Check for cpu 0 removed from gemini_heartbeat.)

-VAL

diff -Nru a/arch/ppc/kernel/time.c b/arch/ppc/kernel/time.c
--- a/arch/ppc/kernel/time.c	Fri Feb  8 15:12:57 2002
+++ b/arch/ppc/kernel/time.c	Fri Feb  8 15:12:57 2002
@@ -208,7 +208,8 @@
 	smp_local_timer_interrupt(regs);
 #endif /* CONFIG_SMP */

-	if (ppc_md.heartbeat && !ppc_md.heartbeat_count--)
+	if (ppc_md.heartbeat && !smp_processor_id() &&
+	    !ppc_md.heartbeat_count--)
 		ppc_md.heartbeat();

 	hardirq_exit(cpu);

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] fix heartbeat
  2002-02-09  1:46 [PATCH] fix heartbeat Val Henson
@ 2002-02-09  6:50 ` Paul Mackerras
  2002-02-10  0:23   ` Val Henson
  0 siblings, 1 reply; 4+ messages in thread
From: Paul Mackerras @ 2002-02-09  6:50 UTC (permalink / raw)
  To: Val Henson; +Cc: linuxppc-dev


Val Henson writes:
>
> The heartbeat function will not execute properly on Gemini without
> this patch. (Check for cpu 0 removed from gemini_heartbeat.)

CHRP needs the heartbeat function called on all cpus, IIRC.

Clearly the heartbeat_count needs to be per-cpu or else pushed into
the heartbeat function.  How about we make it just

	if (ppc_md.heartbeat)
		ppc_md.heartbeat();

and let the heartbeat function do the if (!smp_processor_id()) and the
if (!heartbeat_count--) if it wants?

Paul.

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] fix heartbeat
  2002-02-09  6:50 ` Paul Mackerras
@ 2002-02-10  0:23   ` Val Henson
  2002-02-10 21:43     ` Val Henson
  0 siblings, 1 reply; 4+ messages in thread
From: Val Henson @ 2002-02-10  0:23 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: linuxppc-dev


On Sat, Feb 09, 2002 at 05:50:47PM +1100, Paul Mackerras wrote:
> Val Henson writes:
> >
> > The heartbeat function will not execute properly on Gemini without
> > this patch. (Check for cpu 0 removed from gemini_heartbeat.)
>
> CHRP needs the heartbeat function called on all cpus, IIRC.
>
> Clearly the heartbeat_count needs to be per-cpu or else pushed into
> the heartbeat function.  How about we make it just
>
> 	if (ppc_md.heartbeat)
> 		ppc_md.heartbeat();
>
> and let the heartbeat function do the if (!smp_processor_id()) and the
> if (!heartbeat_count--) if it wants?

Ah, I see, the heartbeat_count is now being decremented once per timer
interrupt, instead of smp_nr_cpus times per timer interrupt.  Last
time I checked, it was still possible for the heartbeat count to go
negative.

It's fine the way it is.  Sorry for the unnecessary patch.

-VAL

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] fix heartbeat
  2002-02-10  0:23   ` Val Henson
@ 2002-02-10 21:43     ` Val Henson
  0 siblings, 0 replies; 4+ messages in thread
From: Val Henson @ 2002-02-10 21:43 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: linuxppc-dev


On Sat, Feb 09, 2002 at 05:23:53PM -0700, Val Henson wrote:
>
> On Sat, Feb 09, 2002 at 05:50:47PM +1100, Paul Mackerras wrote:
> > Val Henson writes:
> > >
> > > The heartbeat function will not execute properly on Gemini without
> > > this patch. (Check for cpu 0 removed from gemini_heartbeat.)
> >
> > CHRP needs the heartbeat function called on all cpus, IIRC.
> >
> > Clearly the heartbeat_count needs to be per-cpu or else pushed into
> > the heartbeat function.  How about we make it just
> >
> > 	if (ppc_md.heartbeat)
> > 		ppc_md.heartbeat();
> >
> > and let the heartbeat function do the if (!smp_processor_id()) and the
> > if (!heartbeat_count--) if it wants?
>
> Ah, I see, the heartbeat_count is now being decremented once per timer
> interrupt, instead of smp_nr_cpus times per timer interrupt.  Last
> time I checked, it was still possible for the heartbeat count to go
> negative.
>
> It's fine the way it is.  Sorry for the unnecessary patch.

Tsk, shouldn't read patches on the weekend... Here's the current code:

        if (ppc_md.heartbeat && !ppc_md.heartbeat_count--)
                ppc_md.heartbeat();

This decrements the count on every processor.  If cpu 0 turns off
interrupts interrupts for a while, we get two effects: heartbeat_count
is now decremented smp_nr_cpus - 1 times per timer interrupt, and if
your heartbeat function doesn't explicitly prevent it, your heartbeat
count could go negative.

An alternative that allows you to run the heartbeat function on
whatever cpus you like but decrements the heartbeat once per timer
interrupt is:

        if (ppc_md.heartbeat && !smp_processor_id()
	         ppc_md.heartbeat_count--)

        if (!ppc_md.heartbeat_count)
                ppc_md.heartbeat();

Howz that?

-VAL

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2002-02-10 21:43 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2002-02-09  1:46 [PATCH] fix heartbeat Val Henson
2002-02-09  6:50 ` Paul Mackerras
2002-02-10  0:23   ` Val Henson
2002-02-10 21:43     ` Val Henson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).