Re: [PATCH] fix percpu heartbeats

[akuster <akuster@dslextreme.com>]

[-- Attachment #1: Type: text/plain, Size: 3631 bytes --]

Milton Miller wrote:
> The changeset of Cort's patch that Troy checked in is broken in several ways:
>
> 1) The slave cpus no longer call heartbeats at all because there was a
> previous continue in the loop. (reordered and used remembered cpu number)
>
> 2) The rtas call rate is per-minute not per-second.  (correct the rate)
>
> 3) All the rtas calls will occur at the same time, modulo the contended
> rtas lock.  (wait to divide the rate until smp_kick_cpus).
>
> 4) It added NRCPUS*3 words to ppc_md.  It also pointed out we were modifing
> this structre repeatedly at runtime.  [moved the counts to the cache hot
> and dirty per-cpu irqstat and allow only one choice of function, just per
> cpu countdown and period.  The count is always decremented, but only when
> it reaches zero do we decide to call the function by checking the reset
> value for non-zero.  The period is reset after the call, so aperiodic
> users (like 4xx watchdog maybe?) could clear the count during their callback.]
>
> I know there are other alternatives, but the cost to non-users is minimal
> (an extra branch and compare every 2*32 jiffies), and the the periodic
> call usage is set it up and forget it.
>
>
> I'll read comments through the lists, so no need to cc me.
>
> milton
>
> ===== arch/ppc/kernel/ppc4xx_setup.c 1.46 vs 1.47 =====
> --- 1.46/arch/ppc/kernel/ppc4xx_setup.c	Fri Aug  2 11:30:01 2002
> +++ 1.47/arch/ppc/kernel/ppc4xx_setup.c	Sun Aug 18 21:03:03 2002
> @@ -383,6 +383,8 @@
>  		strcpy(cmd_line, (char *) (r6 + KERNELBASE));
>  	}
>  #if defined(CONFIG_PPC405_WDT)
> +	ppc_md.heartbeat = ppc4xx_wdt_heartbeat;
> +
>  /* Look for wdt= option on command line */
>  	if (strstr(cmd_line, "wdt=")) {
>  		int valid_wdt = 0;
> @@ -396,6 +398,26 @@
>  			++q;
>  		}
>  		wdt_enable = valid_wdt;
> +
> +		/* XXX why is the wdt functoin modifing count and not
> +		 * using reset ??  If this code wants kernel heartbeat
> +		 * calls then:
> +		 *
> +		 * #include <asm/hwirq.h>
> +		 * heartbeat_reset(cpu) = period_in_jiffies;
> +		 * heartbeat_count(cpu) = period_in_jiffies;
> +		 *
> +		 * The function will be called from the timer interrupt
> +		 * when the counts reaches 0, afterwards the count will
> +		 * be set to the heartbeat_reset(cpu) value (which
> +		 * could be cleard during call for one-shot mode).
> +		 *
> +		 * If the code doesn't want the count manageed it
> +		 * should not use the kernel callbacks (or set reset
> +		 * to 1 and use its own counters).
> +		 */
> +#error sanitize wdt and kernel heartbeat interactions
> +
>  	}
>  #endif
>
> @@ -411,11 +433,6 @@
>  	ppc_md.halt = ppc4xx_halt;
>
>  	ppc_md.calibrate_decr = ppc4xx_calibrate_decr;
> -
> -#ifdef CONFIG_PPC405_WDT
> -	ppc_md.heartbeat = ppc4xx_wdt_heartbeat;
> -#endif
> -	ppc_md.heartbeat_count = 0;
>
>  	ppc_md.find_end_of_memory = ppc4xx_find_end_of_memory;
>  	ppc_md.setup_io_mappings = m4xx_map_io;

To all

Attcahed is a patch for 40x to get wdt working on top of Milton's
orginal patch

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux 2.4 for PowerPC development tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
#
            ChangeSet	1.1138  -> 1.1139
#
drivers/char/ppc405_wdt.c
1.3     -> 1.4
#
arch/ppc/kernel/ppc4xx_setup.c
1.46    -> 1.47
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 02/08/20	armin@essen.mvista.com	1.1139
# changes to make driver work w/ heartbeat_* changes
# changes to setup on top of mintons patch to make 40x to work


Armin



[-- Attachment #2: wdt_0820.patch.gz --]
[-- Type: application/x-gunzip, Size: 1747 bytes --]