[PATCH 0/6] pseries: rtas & nvram cleanup/simpilification

[PATCH 0/6] pseries: rtas & nvram cleanup/simpilification

[Linas Vepstas]

Paul,

Please apply te following for 2.6.24

The following sequence of patches cleanup, simplify and shorten
the pseries code having to do with error logging. Several
global variables shared across directories are removed, and
the rtasd initialization sequence is rearranged and simplified,
making the flow of control clearer. A minor buglet is fixed,
and error mesages can now be logged earlier in the boot sequence.

--linas
 

[PATCH 1/6] pseries: avoid excess rtas calls

[Linas Vepstas]

We don't need to look up the rtas event token once per 
cpu per second. This avoids some misc string ops and 
rtas calls and provides some minor performance improvement.

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>

----
 arch/powerpc/platforms/pseries/rtasd.c |   15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

Index: linux-2.6.22-git2/arch/powerpc/platforms/pseries/rtasd.c
===================================================================
--- linux-2.6.22-git2.orig/arch/powerpc/platforms/pseries/rtasd.c	2007-07-08 18:32:17.000000000 -0500
+++ linux-2.6.22-git2/arch/powerpc/platforms/pseries/rtasd.c	2007-08-07 17:57:13.000000000 -0500
@@ -44,10 +44,13 @@ static unsigned long rtas_log_start;
 static unsigned long rtas_log_size;
 
 static int surveillance_timeout = -1;
-static unsigned int rtas_event_scan_rate;
 static unsigned int rtas_error_log_max;
 static unsigned int rtas_error_log_buffer_max;
 
+/* RTAS service tokens */
+static unsigned int event_scan;
+static unsigned int rtas_event_scan_rate;
+
 static int full_rtas_msgs = 0;
 
 extern int no_logging;
@@ -381,7 +384,7 @@ static int get_eventscan_parms(void)
 	return 0;
 }
 
-static void do_event_scan(int event_scan)
+static void do_event_scan(void)
 {
 	int error;
 	do {
@@ -408,7 +411,7 @@ static void do_event_scan_all_cpus(long 
 	cpu = first_cpu(cpu_online_map);
 	for (;;) {
 		set_cpus_allowed(current, cpumask_of_cpu(cpu));
-		do_event_scan(rtas_token("event-scan"));
+		do_event_scan();
 		set_cpus_allowed(current, CPU_MASK_ALL);
 
 		/* Drop hotplug lock, and sleep for the specified delay */
@@ -426,12 +429,11 @@ static void do_event_scan_all_cpus(long 
 static int rtasd(void *unused)
 {
 	unsigned int err_type;
-	int event_scan = rtas_token("event-scan");
 	int rc;
 
 	daemonize("rtasd");
 
-	if (event_scan == RTAS_UNKNOWN_SERVICE || get_eventscan_parms() == -1)
+	if (get_eventscan_parms() == -1)
 		goto error;
 
 	rtas_log_buf = vmalloc(rtas_error_log_buffer_max*LOG_NUMBER);
@@ -486,7 +488,8 @@ static int __init rtas_init(void)
 		return 0;
 
 	/* No RTAS */
-	if (rtas_token("event-scan") == RTAS_UNKNOWN_SERVICE) {
+	event_scan = rtas_token("event-scan");
+	if (event_scan == RTAS_UNKNOWN_SERVICE) {
 		printk(KERN_DEBUG "rtasd: no event-scan on system\n");
 		return -ENODEV;
 	}

[PATCH 2/6] pseries: use rtas_token instead of hand-rolled code

[Linas Vepstas]

The rtas_token() call does the same thing as this hand-rolled code.
This makes the code easier to read.

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>

----
 arch/powerpc/platforms/pseries/rtasd.c |   13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

Index: linux-2.6.22-git2/arch/powerpc/platforms/pseries/rtasd.c
===================================================================
--- linux-2.6.22-git2.orig/arch/powerpc/platforms/pseries/rtasd.c	2007-08-07 17:57:13.000000000 -0500
+++ linux-2.6.22-git2/arch/powerpc/platforms/pseries/rtasd.c	2007-08-07 17:57:49.000000000 -0500
@@ -361,26 +361,17 @@ static int enable_surveillance(int timeo
 
 static int get_eventscan_parms(void)
 {
-	struct device_node *node;
-	const int *ip;
-
-	node = of_find_node_by_path("/rtas");
-
-	ip = of_get_property(node, "rtas-event-scan-rate", NULL);
-	if (ip == NULL) {
+	rtas_event_scan_rate = rtas_token("rtas-event-scan-rate");
+	if (rtas_event_scan_rate == RTAS_UNKNOWN_SERVICE) {
 		printk(KERN_ERR "rtasd: no rtas-event-scan-rate\n");
-		of_node_put(node);
 		return -1;
 	}
-	rtas_event_scan_rate = *ip;
 	DEBUG("rtas-event-scan-rate %d\n", rtas_event_scan_rate);
 
 	/* Make room for the sequence number */
 	rtas_error_log_max = rtas_get_error_log_max();
 	rtas_error_log_buffer_max = rtas_error_log_max + sizeof(int);
 
-	of_node_put(node);
-
 	return 0;
 }
 

[PATCH 3/6] pseries: simplify rtasd initialization

[Linas Vepstas]

Simplify rtasd initialization code; this also fixes a buglet, 
where the /proc entries weren't being cleaned up in case of 
failure.

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>

----
 arch/powerpc/platforms/pseries/rtasd.c |   53 +++++++++++----------------------
 1 file changed, 19 insertions(+), 34 deletions(-)

Index: linux-2.6.22-git2/arch/powerpc/platforms/pseries/rtasd.c
===================================================================
--- linux-2.6.22-git2.orig/arch/powerpc/platforms/pseries/rtasd.c	2007-08-08 12:19:47.000000000 -0500
+++ linux-2.6.22-git2/arch/powerpc/platforms/pseries/rtasd.c	2007-08-08 12:20:44.000000000 -0500
@@ -64,8 +64,6 @@ volatile int error_log_cnt = 0;
  */
 static unsigned char logdata[RTAS_ERROR_LOG_MAX];
 
-static int get_eventscan_parms(void);
-
 static char *rtas_type[] = {
 	"Unknown", "Retry", "TCE Error", "Internal Device Failure",
 	"Timeout", "Data Parity", "Address Parity", "Cache Parity",
@@ -169,9 +167,9 @@ static int log_rtas_len(char * buf)
 		len += err->extended_log_length;
 	}
 
-	if (rtas_error_log_max == 0) {
-		get_eventscan_parms();
-	}
+	if (rtas_error_log_max == 0)
+		rtas_error_log_max = rtas_get_error_log_max();
+
 	if (len > rtas_error_log_max)
 		len = rtas_error_log_max;
 
@@ -359,22 +357,6 @@ static int enable_surveillance(int timeo
 	return -1;
 }
 
-static int get_eventscan_parms(void)
-{
-	rtas_event_scan_rate = rtas_token("rtas-event-scan-rate");
-	if (rtas_event_scan_rate == RTAS_UNKNOWN_SERVICE) {
-		printk(KERN_ERR "rtasd: no rtas-event-scan-rate\n");
-		return -1;
-	}
-	DEBUG("rtas-event-scan-rate %d\n", rtas_event_scan_rate);
-
-	/* Make room for the sequence number */
-	rtas_error_log_max = rtas_get_error_log_max();
-	rtas_error_log_buffer_max = rtas_error_log_max + sizeof(int);
-
-	return 0;
-}
-
 static void do_event_scan(void)
 {
 	int error;
@@ -424,22 +406,11 @@ static int rtasd(void *unused)
 
 	daemonize("rtasd");
 
-	if (get_eventscan_parms() == -1)
-		goto error;
-
-	rtas_log_buf = vmalloc(rtas_error_log_buffer_max*LOG_NUMBER);
-	if (!rtas_log_buf) {
-		printk(KERN_ERR "rtasd: no memory\n");
-		goto error;
-	}
-
 	printk(KERN_DEBUG "RTAS daemon started\n");
-
 	DEBUG("will sleep for %d milliseconds\n", (30000/rtas_event_scan_rate));
 
 	/* See if we have any error stored in NVRAM */
 	memset(logdata, 0, rtas_error_log_max);
-
 	rc = nvram_read_error_log(logdata, rtas_error_log_max, &err_type);
 
 	/* We can use rtas_log_buf now */
@@ -466,8 +437,6 @@ static int rtasd(void *unused)
 	for (;;)
 		do_event_scan_all_cpus(30000/rtas_event_scan_rate);
 
-error:
-	/* Should delete proc entries */
 	return -EINVAL;
 }
 
@@ -485,6 +454,22 @@ static int __init rtas_init(void)
 		return -ENODEV;
 	}
 
+	rtas_event_scan_rate = rtas_token("rtas-event-scan-rate");
+	if (rtas_event_scan_rate == RTAS_UNKNOWN_SERVICE) {
+		printk(KERN_ERR "rtasd: no rtas-event-scan-rate on system\n");
+		return -ENODEV;
+	}
+
+	/* Make room for the sequence number */
+	rtas_error_log_max = rtas_get_error_log_max();
+	rtas_error_log_buffer_max = rtas_error_log_max + sizeof(int);
+
+	rtas_log_buf = vmalloc(rtas_error_log_buffer_max*LOG_NUMBER);
+	if (!rtas_log_buf) {
+		printk(KERN_ERR "rtasd: no memory\n");
+		return -ENOMEM;
+	}
+
 	entry = create_proc_entry("ppc64/rtas/error_log", S_IRUSR, NULL);
 	if (entry)
 		entry->proc_fops = &proc_rtas_log_operations;

[PATCH 4/6] powerpc: remove nvram forward declarations

[Linas Vepstas]

Forward declarations serve no purpose in this file.

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>

----

 arch/powerpc/kernel/nvram_64.c |    5 -----
 1 file changed, 5 deletions(-)

Index: linux-2.6.22-git2/arch/powerpc/kernel/nvram_64.c
===================================================================
--- linux-2.6.22-git2.orig/arch/powerpc/kernel/nvram_64.c	2007-08-08 12:19:17.000000000 -0500
+++ linux-2.6.22-git2/arch/powerpc/kernel/nvram_64.c	2007-08-08 12:21:05.000000000 -0500
@@ -34,11 +34,6 @@
 
 #undef DEBUG_NVRAM
 
-static int nvram_scan_partitions(void);
-static int nvram_setup_partition(void);
-static int nvram_create_os_partition(void);
-static int nvram_remove_os_partition(void);
-
 static struct nvram_partition * nvram_part;
 static long nvram_error_log_index = -1;
 static long nvram_error_log_size = 0;

[PATCH 5/6] pseries: fix jumbled no_logging flag.

[Linas Vepstas]

Get rid of the jumbled usage of the no_logging flag. Its use 
spans several directories, and is incorrectly/misleadingly
documented. Instead, two changes:
1) nvram will accept error log as soon as its ready.
2) logging to nvram stops on the first fatal error reported.

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>

----
 arch/powerpc/kernel/nvram_64.c         |    8 --------
 arch/powerpc/platforms/pseries/rtasd.c |   14 ++++++--------
 2 files changed, 6 insertions(+), 16 deletions(-)

Index: linux-2.6.22-git2/arch/powerpc/kernel/nvram_64.c
===================================================================
--- linux-2.6.22-git2.orig/arch/powerpc/kernel/nvram_64.c	2007-08-08 12:21:05.000000000 -0500
+++ linux-2.6.22-git2/arch/powerpc/kernel/nvram_64.c	2007-08-08 12:21:16.000000000 -0500
@@ -38,10 +38,6 @@ static struct nvram_partition * nvram_pa
 static long nvram_error_log_index = -1;
 static long nvram_error_log_size = 0;
 
-int no_logging = 1; 	/* Until we initialize everything,
-			 * make sure we don't try logging
-			 * anything */
-
 extern volatile int error_log_cnt;
 
 struct err_log_info {
@@ -637,10 +633,6 @@ int nvram_write_error_log(char * buff, i
 	loff_t tmp_index;
 	struct err_log_info info;
 	
-	if (no_logging) {
-		return -EPERM;
-	}
-
 	if (nvram_error_log_index == -1) {
 		return -ESPIPE;
 	}
Index: linux-2.6.22-git2/arch/powerpc/platforms/pseries/rtasd.c
===================================================================
--- linux-2.6.22-git2.orig/arch/powerpc/platforms/pseries/rtasd.c	2007-08-08 12:20:44.000000000 -0500
+++ linux-2.6.22-git2/arch/powerpc/platforms/pseries/rtasd.c	2007-08-08 12:21:16.000000000 -0500
@@ -53,7 +53,8 @@ static unsigned int rtas_event_scan_rate
 
 static int full_rtas_msgs = 0;
 
-extern int no_logging;
+/* Stop logging to nvram after first fatal error */
+static int no_more_logging;
 
 volatile int error_log_cnt = 0;
 
@@ -216,7 +217,7 @@ void pSeries_log_error(char *buf, unsign
 	}
 
 	/* Write error to NVRAM */
-	if (!no_logging && !(err_type & ERR_FLAG_BOOT))
+	if (!no_more_logging && !(err_type & ERR_FLAG_BOOT))
 		nvram_write_error_log(buf, len, err_type);
 
 	/*
@@ -228,8 +229,8 @@ void pSeries_log_error(char *buf, unsign
 		printk_log_rtas(buf, len);
 
 	/* Check to see if we need to or have stopped logging */
-	if (fatal || no_logging) {
-		no_logging = 1;
+	if (fatal || no_more_logging) {
+		no_more_logging = 1;
 		spin_unlock_irqrestore(&rtasd_log_lock, s);
 		return;
 	}
@@ -301,7 +302,7 @@ static ssize_t rtas_log_read(struct file
 
 	spin_lock_irqsave(&rtasd_log_lock, s);
 	/* if it's 0, then we know we got the last one (the one in NVRAM) */
-	if (rtas_log_size == 0 && !no_logging)
+	if (rtas_log_size == 0 && !no_more_logging)
 		nvram_clear_error_log();
 	spin_unlock_irqrestore(&rtasd_log_lock, s);
 
@@ -413,9 +414,6 @@ static int rtasd(void *unused)
 	memset(logdata, 0, rtas_error_log_max);
 	rc = nvram_read_error_log(logdata, rtas_error_log_max, &err_type);
 
-	/* We can use rtas_log_buf now */
-	no_logging = 0;
-
 	if (!rc) {
 		if (err_type != ERR_FLAG_ALREADY_LOGGED) {
 			pSeries_log_error(logdata, err_type | ERR_FLAG_BOOT, 0);

[PATCH 6/6] pseries: eliminate global var

[Linas Vepstas]

Eliminate the use of error_log_cnt as a global var shared across
differnt directories. Pass it as a subroutine arg instead.

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>

----
 arch/powerpc/kernel/nvram_64.c         |   10 +++++-----
 arch/powerpc/platforms/pseries/rtasd.c |    7 ++++---
 include/asm-powerpc/nvram.h            |   15 ++++++++++-----
 3 files changed, 19 insertions(+), 13 deletions(-)

Index: linux-2.6.22-git2/arch/powerpc/kernel/nvram_64.c
===================================================================
--- linux-2.6.22-git2.orig/arch/powerpc/kernel/nvram_64.c	2007-08-08 12:21:16.000000000 -0500
+++ linux-2.6.22-git2/arch/powerpc/kernel/nvram_64.c	2007-08-08 12:24:01.000000000 -0500
@@ -38,8 +38,6 @@ static struct nvram_partition * nvram_pa
 static long nvram_error_log_index = -1;
 static long nvram_error_log_size = 0;
 
-extern volatile int error_log_cnt;
-
 struct err_log_info {
 	int error_type;
 	unsigned int seq_num;
@@ -627,7 +625,8 @@ void __exit nvram_cleanup(void)
  * sequence #: The unique sequence # for each event. (until it wraps)
  * error log: The error log from event_scan
  */
-int nvram_write_error_log(char * buff, int length, unsigned int err_type)
+int nvram_write_error_log(char * buff, int length,
+                          unsigned int err_type, unsigned int error_log_cnt)
 {
 	int rc;
 	loff_t tmp_index;
@@ -665,7 +664,8 @@ int nvram_write_error_log(char * buff, i
  *
  * Reads nvram for error log for at most 'length'
  */
-int nvram_read_error_log(char * buff, int length, unsigned int * err_type)
+int nvram_read_error_log(char * buff, int length,
+                         unsigned int * err_type, unsigned int * error_log_cnt)
 {
 	int rc;
 	loff_t tmp_index;
@@ -691,7 +691,7 @@ int nvram_read_error_log(char * buff, in
 		return rc;
 	}
 
-	error_log_cnt = info.seq_num;
+	*error_log_cnt = info.seq_num;
 	*err_type = info.error_type;
 
 	return 0;
Index: linux-2.6.22-git2/include/asm-powerpc/nvram.h
===================================================================
--- linux-2.6.22-git2.orig/include/asm-powerpc/nvram.h	2007-07-08 18:32:17.000000000 -0500
+++ linux-2.6.22-git2/include/asm-powerpc/nvram.h	2007-08-08 13:34:27.000000000 -0500
@@ -62,14 +62,19 @@ struct nvram_partition {
 	unsigned int index;
 };
 
-
-extern int nvram_write_error_log(char * buff, int length, unsigned int err_type);
-extern int nvram_read_error_log(char * buff, int length, unsigned int * err_type);
-extern int nvram_clear_error_log(void);
 extern struct nvram_partition *nvram_find_partition(int sig, const char *name);
 
-extern int pSeries_nvram_init(void);
 extern int mmio_nvram_init(void);
+
+#ifdef CONFIG_PPC_PSERIES
+extern int pSeries_nvram_init(void);
+extern int nvram_write_error_log(char * buff, int length,
+					 unsigned int err_type, unsigned int err_seq);
+extern int nvram_read_error_log(char * buff, int length,
+					 unsigned int * err_type, unsigned int *err_seq);
+extern int nvram_clear_error_log(void);
+#endif /* CONFIG_PPC_PSERIES */
+
 #endif /* __KERNEL__ */
 
 /* PowerMac specific nvram stuffs */
Index: linux-2.6.22-git2/arch/powerpc/platforms/pseries/rtasd.c
===================================================================
--- linux-2.6.22-git2.orig/arch/powerpc/platforms/pseries/rtasd.c	2007-08-08 12:21:16.000000000 -0500
+++ linux-2.6.22-git2/arch/powerpc/platforms/pseries/rtasd.c	2007-08-08 12:50:40.000000000 -0500
@@ -56,7 +56,7 @@ static int full_rtas_msgs = 0;
 /* Stop logging to nvram after first fatal error */
 static int no_more_logging;
 
-volatile int error_log_cnt = 0;
+static int error_log_cnt;
 
 /*
  * Since we use 32 bit RTAS, the physical address of this must be below
@@ -218,7 +218,7 @@ void pSeries_log_error(char *buf, unsign
 
 	/* Write error to NVRAM */
 	if (!no_more_logging && !(err_type & ERR_FLAG_BOOT))
-		nvram_write_error_log(buf, len, err_type);
+		nvram_write_error_log(buf, len, err_type, error_log_cnt);
 
 	/*
 	 * rtas errors can occur during boot, and we do want to capture
@@ -412,7 +412,8 @@ static int rtasd(void *unused)
 
 	/* See if we have any error stored in NVRAM */
 	memset(logdata, 0, rtas_error_log_max);
-	rc = nvram_read_error_log(logdata, rtas_error_log_max, &err_type);
+	rc = nvram_read_error_log(logdata, rtas_error_log_max,
+	                          &err_type, &error_log_cnt);
 
 	if (!rc) {
 		if (err_type != ERR_FLAG_ALREADY_LOGGED) {

Re: [PATCH 6/6] pseries: eliminate global var

[Nathan Lynch]

Linas Vepstas wrote:
> --- linux-2.6.22-git2.orig/include/asm-powerpc/nvram.h	2007-07-08 18:32:17.000000000 -0500
> +++ linux-2.6.22-git2/include/asm-powerpc/nvram.h	2007-08-08 13:34:27.000000000 -0500
> @@ -62,14 +62,19 @@ struct nvram_partition {
>  	unsigned int index;
>  };
>  
> -
> -extern int nvram_write_error_log(char * buff, int length, unsigned int err_type);
> -extern int nvram_read_error_log(char * buff, int length, unsigned int * err_type);
> -extern int nvram_clear_error_log(void);
>  extern struct nvram_partition *nvram_find_partition(int sig, const char *name);
>  
> -extern int pSeries_nvram_init(void);
>  extern int mmio_nvram_init(void);
> +
> +#ifdef CONFIG_PPC_PSERIES
> +extern int pSeries_nvram_init(void);
> +extern int nvram_write_error_log(char * buff, int length,
> +					 unsigned int err_type, unsigned int err_seq);
> +extern int nvram_read_error_log(char * buff, int length,
> +					 unsigned int * err_type, unsigned int *err_seq);
> +extern int nvram_clear_error_log(void);
> +#endif /* CONFIG_PPC_PSERIES */

Declarations need not be #ifdef'd.

Re: [PATCH 6/6] pseries: eliminate global var

[Linas Vepstas]

On Wed, Aug 08, 2007 at 04:57:14PM -0500, Nathan Lynch wrote:
> Linas Vepstas wrote:
> > +
> > +#ifdef CONFIG_PPC_PSERIES
> > +extern int pSeries_nvram_init(void);
> > +extern int nvram_write_error_log(char * buff, int length,
> > +					 unsigned int err_type, unsigned int err_seq);
> > +extern int nvram_read_error_log(char * buff, int length,
> > +					 unsigned int * err_type, unsigned int *err_seq);
> > +extern int nvram_clear_error_log(void);
> > +#endif /* CONFIG_PPC_PSERIES */
> 
> Declarations need not be #ifdef'd.

Ah, I thought that would be cleaner ... should I resubmit,
the patch, or does it matter that much?

--linas

Re: [PATCH 6/6] pseries: eliminate global var

[Nathan Lynch]

Linas Vepstas wrote:
> On Wed, Aug 08, 2007 at 04:57:14PM -0500, Nathan Lynch wrote:
> > Linas Vepstas wrote:
> > > +
> > > +#ifdef CONFIG_PPC_PSERIES
> > > +extern int pSeries_nvram_init(void);
> > > +extern int nvram_write_error_log(char * buff, int length,
> > > +					 unsigned int err_type, unsigned int err_seq);
> > > +extern int nvram_read_error_log(char * buff, int length,
> > > +					 unsigned int * err_type, unsigned int *err_seq);
> > > +extern int nvram_clear_error_log(void);
> > > +#endif /* CONFIG_PPC_PSERIES */
> > 
> > Declarations need not be #ifdef'd.
> 
> Ah, I thought that would be cleaner ... should I resubmit,
> the patch, or does it matter that much?

FWIW I'd prefer you drop that hunk and resubmit; it's needless churn
that can only cause build problems.

[PATCH 6/6 v2] pseries: eliminate global var

[Linas Vepstas]

Eliminate the use of error_log_cnt as a global var shared across
differnt directories. Pass it as a subroutine arg instead.

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>

----

Respin of earlier patch, with the CONFIG_PSERIES junk removed from the
header file.

 arch/powerpc/kernel/nvram_64.c         |   10 +++++-----
 arch/powerpc/platforms/pseries/rtasd.c |    7 ++++---
 include/asm-powerpc/nvram.h            |    6 ++++--
 3 files changed, 13 insertions(+), 10 deletions(-)

Index: linux-2.6.22-git2/arch/powerpc/kernel/nvram_64.c
===================================================================
--- linux-2.6.22-git2.orig/arch/powerpc/kernel/nvram_64.c	2007-08-08 12:21:16.000000000 -0500
+++ linux-2.6.22-git2/arch/powerpc/kernel/nvram_64.c	2007-08-08 12:24:01.000000000 -0500
@@ -38,8 +38,6 @@ static struct nvram_partition * nvram_pa
 static long nvram_error_log_index = -1;
 static long nvram_error_log_size = 0;
 
-extern volatile int error_log_cnt;
-
 struct err_log_info {
 	int error_type;
 	unsigned int seq_num;
@@ -627,7 +625,8 @@ void __exit nvram_cleanup(void)
  * sequence #: The unique sequence # for each event. (until it wraps)
  * error log: The error log from event_scan
  */
-int nvram_write_error_log(char * buff, int length, unsigned int err_type)
+int nvram_write_error_log(char * buff, int length,
+                          unsigned int err_type, unsigned int error_log_cnt)
 {
 	int rc;
 	loff_t tmp_index;
@@ -665,7 +664,8 @@ int nvram_write_error_log(char * buff, i
  *
  * Reads nvram for error log for at most 'length'
  */
-int nvram_read_error_log(char * buff, int length, unsigned int * err_type)
+int nvram_read_error_log(char * buff, int length,
+                         unsigned int * err_type, unsigned int * error_log_cnt)
 {
 	int rc;
 	loff_t tmp_index;
@@ -691,7 +691,7 @@ int nvram_read_error_log(char * buff, in
 		return rc;
 	}
 
-	error_log_cnt = info.seq_num;
+	*error_log_cnt = info.seq_num;
 	*err_type = info.error_type;
 
 	return 0;
Index: linux-2.6.22-git2/include/asm-powerpc/nvram.h
===================================================================
--- linux-2.6.22-git2.orig/include/asm-powerpc/nvram.h	2007-07-08 18:32:17.000000000 -0500
+++ linux-2.6.22-git2/include/asm-powerpc/nvram.h	2007-08-09 15:41:43.000000000 -0500
@@ -63,8 +63,10 @@ struct nvram_partition {
 };
 
 
-extern int nvram_write_error_log(char * buff, int length, unsigned int err_type);
-extern int nvram_read_error_log(char * buff, int length, unsigned int * err_type);
+extern int nvram_write_error_log(char * buff, int length,
+					 unsigned int err_type, unsigned int err_seq);
+extern int nvram_read_error_log(char * buff, int length,
+					 unsigned int * err_type, unsigned int *err_seq);
 extern int nvram_clear_error_log(void);
 extern struct nvram_partition *nvram_find_partition(int sig, const char *name);
 
Index: linux-2.6.22-git2/arch/powerpc/platforms/pseries/rtasd.c
===================================================================
--- linux-2.6.22-git2.orig/arch/powerpc/platforms/pseries/rtasd.c	2007-08-08 12:21:16.000000000 -0500
+++ linux-2.6.22-git2/arch/powerpc/platforms/pseries/rtasd.c	2007-08-08 12:50:40.000000000 -0500
@@ -56,7 +56,7 @@ static int full_rtas_msgs = 0;
 /* Stop logging to nvram after first fatal error */
 static int no_more_logging;
 
-volatile int error_log_cnt = 0;
+static int error_log_cnt;
 
 /*
  * Since we use 32 bit RTAS, the physical address of this must be below
@@ -218,7 +218,7 @@ void pSeries_log_error(char *buf, unsign
 
 	/* Write error to NVRAM */
 	if (!no_more_logging && !(err_type & ERR_FLAG_BOOT))
-		nvram_write_error_log(buf, len, err_type);
+		nvram_write_error_log(buf, len, err_type, error_log_cnt);
 
 	/*
 	 * rtas errors can occur during boot, and we do want to capture
@@ -412,7 +412,8 @@ static int rtasd(void *unused)
 
 	/* See if we have any error stored in NVRAM */
 	memset(logdata, 0, rtas_error_log_max);
-	rc = nvram_read_error_log(logdata, rtas_error_log_max, &err_type);
+	rc = nvram_read_error_log(logdata, rtas_error_log_max,
+	                          &err_type, &error_log_cnt);
 
 	if (!rc) {
 		if (err_type != ERR_FLAG_ALREADY_LOGGED) {

Re: [PATCH 1/6] pseries: avoid excess rtas calls

[Nathan Lynch]

Linas Vepstas wrote:
> 
> We don't need to look up the rtas event token once per 
> cpu per second. This avoids some misc string ops and 
> rtas calls and provides some minor performance improvement.

It does not avoid any calls to RTAS.  (rtas_token merely looks up
properties under the /rtas node.)

Otherwise, looks okay.

Re: [PATCH 1/6] pseries: avoid excess rtas calls

[Linas Vepstas]

On Wed, Aug 08, 2007 at 04:57:39PM -0500, Nathan Lynch wrote:
> Linas Vepstas wrote:
> > 
> > We don't need to look up the rtas event token once per 
> > cpu per second. This avoids some misc string ops and 
> > rtas calls and provides some minor performance improvement.
> 
> It does not avoid any calls to RTAS.  (rtas_token merely looks up
> properties under the /rtas node.)

Right. I'd vaguely known but forgotten that of_get_property didn't 
actually turn into an rtas call.

> Otherwise, looks okay.

Should I resubmit with modified commit message, or does anyone care?

--linas

[PATCH 1/6 v2] pseries: avoid excess rtas calls

[Linas Vepstas]

We don't need to look up the rtas event token once per 
cpu per second. This avoids some misc device-tree lookups
and string ops and so provides some minor performance 
improvement.

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>

----
Revised commit-log message.

 arch/powerpc/platforms/pseries/rtasd.c |   15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

Index: linux-2.6.22-git2/arch/powerpc/platforms/pseries/rtasd.c
===================================================================
--- linux-2.6.22-git2.orig/arch/powerpc/platforms/pseries/rtasd.c	2007-07-08 18:32:17.000000000 -0500
+++ linux-2.6.22-git2/arch/powerpc/platforms/pseries/rtasd.c	2007-08-07 17:57:13.000000000 -0500
@@ -44,10 +44,13 @@ static unsigned long rtas_log_start;
 static unsigned long rtas_log_size;
 
 static int surveillance_timeout = -1;
-static unsigned int rtas_event_scan_rate;
 static unsigned int rtas_error_log_max;
 static unsigned int rtas_error_log_buffer_max;
 
+/* RTAS service tokens */
+static unsigned int event_scan;
+static unsigned int rtas_event_scan_rate;
+
 static int full_rtas_msgs = 0;
 
 extern int no_logging;
@@ -381,7 +384,7 @@ static int get_eventscan_parms(void)
 	return 0;
 }
 
-static void do_event_scan(int event_scan)
+static void do_event_scan(void)
 {
 	int error;
 	do {
@@ -408,7 +411,7 @@ static void do_event_scan_all_cpus(long 
 	cpu = first_cpu(cpu_online_map);
 	for (;;) {
 		set_cpus_allowed(current, cpumask_of_cpu(cpu));
-		do_event_scan(rtas_token("event-scan"));
+		do_event_scan();
 		set_cpus_allowed(current, CPU_MASK_ALL);
 
 		/* Drop hotplug lock, and sleep for the specified delay */
@@ -426,12 +429,11 @@ static void do_event_scan_all_cpus(long 
 static int rtasd(void *unused)
 {
 	unsigned int err_type;
-	int event_scan = rtas_token("event-scan");
 	int rc;
 
 	daemonize("rtasd");
 
-	if (event_scan == RTAS_UNKNOWN_SERVICE || get_eventscan_parms() == -1)
+	if (get_eventscan_parms() == -1)
 		goto error;
 
 	rtas_log_buf = vmalloc(rtas_error_log_buffer_max*LOG_NUMBER);
@@ -486,7 +488,8 @@ static int __init rtas_init(void)
 		return 0;
 
 	/* No RTAS */
-	if (rtas_token("event-scan") == RTAS_UNKNOWN_SERVICE) {
+	event_scan = rtas_token("event-scan");
+	if (event_scan == RTAS_UNKNOWN_SERVICE) {
 		printk(KERN_DEBUG "rtasd: no event-scan on system\n");
 		return -ENODEV;
 	}