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
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 <redacted>
----
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
===================================================================
@@ -44,10 +44,13 @@ static unsigned long rtas_log_start;staticunsignedlongrtas_log_size;staticintsurveillance_timeout=-1;-staticunsignedintrtas_event_scan_rate;staticunsignedintrtas_error_log_max;staticunsignedintrtas_error_log_buffer_max;+/* RTAS service tokens */+staticunsignedintevent_scan;+staticunsignedintrtas_event_scan_rate;+staticintfull_rtas_msgs=0;externintno_logging;
@@ -381,7 +384,7 @@ static int get_eventscan_parms(void)return0;}-staticvoiddo_event_scan(intevent_scan)+staticvoiddo_event_scan(void){interror;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 */
@@ -486,7 +488,8 @@ static int __init rtas_init(void)return0;/* 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;}
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 <redacted>
----
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
===================================================================
@@ -361,26 +361,17 @@ static int enable_surveillance(int timeostaticintget_eventscan_parms(void){-structdevice_node*node;-constint*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);-return0;}
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 <redacted>
----
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
===================================================================
@@ -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 timeoreturn-1;}-staticintget_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);--return0;-}-staticvoiddo_event_scan(void){interror;
@@ -424,22 +406,11 @@ static int rtasd(void *unused)daemonize("rtasd");-if(get_eventscan_parms()==-1)-gotoerror;--rtas_log_buf=vmalloc(rtas_error_log_buffer_max*LOG_NUMBER);-if(!rtas_log_buf){-printk(KERN_ERR"rtasd: no memory\n");-gotoerror;-}-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;
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 <redacted>
----
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
===================================================================
@@ -53,7 +53,8 @@ static unsigned int rtas_event_scan_ratestaticintfull_rtas_msgs=0;-externintno_logging;+/* Stop logging to nvram after first fatal error */+staticintno_more_logging;volatileinterror_log_cnt=0;
@@ -228,8 +229,8 @@ void pSeries_log_error(char *buf, unsignprintk_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 filespin_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);
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 <redacted>
----
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
===================================================================
@@ -56,7 +56,7 @@ static int full_rtas_msgs = 0;/* Stop logging to nvram after first fatal error */staticintno_more_logging;-volatileinterror_log_cnt=0;+staticinterror_log_cnt;/**Sinceweuse32bitRTAS,thephysicaladdressofthismustbebelow
@@ -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){
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.
On Wed, Aug 08, 2007 at 04:57:39PM -0500, Nathan Lynch wrote:
Linas Vepstas wrote:
quoted
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
On Wed, Aug 08, 2007 at 04:57:14PM -0500, Nathan Lynch wrote:
Linas Vepstas wrote:
quoted
+
+#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
On Wed, Aug 08, 2007 at 04:57:14PM -0500, Nathan Lynch wrote:
quoted
Linas Vepstas wrote:
quoted
+
+#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.
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 <redacted>
----
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
===================================================================
@@ -56,7 +56,7 @@ static int full_rtas_msgs = 0;/* Stop logging to nvram after first fatal error */staticintno_more_logging;-volatileinterror_log_cnt=0;+staticinterror_log_cnt;/**Sinceweuse32bitRTAS,thephysicaladdressofthismustbebelow
@@ -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){
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 <redacted>
----
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
===================================================================
@@ -44,10 +44,13 @@ static unsigned long rtas_log_start;staticunsignedlongrtas_log_size;staticintsurveillance_timeout=-1;-staticunsignedintrtas_event_scan_rate;staticunsignedintrtas_error_log_max;staticunsignedintrtas_error_log_buffer_max;+/* RTAS service tokens */+staticunsignedintevent_scan;+staticunsignedintrtas_event_scan_rate;+staticintfull_rtas_msgs=0;externintno_logging;
@@ -381,7 +384,7 @@ static int get_eventscan_parms(void)return0;}-staticvoiddo_event_scan(intevent_scan)+staticvoiddo_event_scan(void){interror;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 */
@@ -486,7 +488,8 @@ static int __init rtas_init(void)return0;/* 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;}