powerpc/powernv: Framework to log critical errors on powernv.
PATCH v2 1/1 powerpc/powernv: Framework to log critical errors on powernv.
This patch provides error logging interfaces to report critical powernv error to FSP.
All the required information to dump the error is collected at POWERNV level through error log interfaces
and then pushed on to FSP.
This also supports synchronous error logging in cases of PANIC, by passing OPAL_ERROR_PANIC as severity in elog_create() call.
--- arch/powerpc/include/asm/opal.h | 125 ++++++++++++++++++++++++ arch/powerpc/platforms/powernv/opal-elog.c | 60 +++++++++++- arch/powerpc/platforms/powernv/opal-wrappers.S | 1 3 files changed, 185 insertions(+), 1 deletion(-)
arch/powerpc/include/asm/opal.h +125 -0
@@ -168,6 +168,7 @@ extern int opal_enter_rtas(struct rtas_args *args,#define OPAL_GET_MSG 85#define OPAL_CHECK_ASYNC_COMPLETION 86#define OPAL_SYNC_HOST_REBOOT 87 +#define OPAL_ELOG_SEND 88#ifndef __ASSEMBLY__
@@ -260,6 +261,122 @@ enum OpalMessageType { OPAL_MSG_TYPE_MAX, }; +/* Classification of Error/Events reporting type classification +* Platform Events/Errors: Report Machine Check Interrupt +* INPUT_OUTPUT: Report all I/O related events/errors +* RESOURCE_DEALLOC: Hotplug events and errors +* MISC: Miscellanous error +* Field: error_events_type +*/ +enum error_events { +OPAL_PLATFORM , +OPAL_INPUT_OUTPUT , +OPAL_RESOURCE_DEALLOC , +OPAL_MISC , +}; + +/* OPAL Subsystem IDs listed for reporting events/errors +* Field: subsystem_id +*/ + +#define OPAL_PROCESSOR_SUBSYSTEM 0x10 +#define OPAL_MEMORY_SUBSYSTEM 0x20 +#define OPAL_IO_SUBSYSTEM 0x30 +#define OPAL_IO_DEVICES 0x40 +#define OPAL_CEC_HARDWARE 0x50 +#define OPAL_POWER_COOLING 0x60 +#define OPAL_MISC_SUBSYSTEM 0x70 +#define OPAL_SURVEILLANCE_ERR 0x7A +#define OPAL_PLATFORM_FIRMWARE 0x80 +#define OPAL_SOFTWARE 0x90 +#define OPAL_EXTERNAL_ENV 0xA0 + +/* During reporting an event/error the following represents +* how serious the logged event/error is. (Severity) +* Field: event_sev +*/ +#define OPAL_INFO 0x00 +#define OPAL_RECOVERED_ERR_GENERAL 0x10 + +/* 0x2X series is to denote set of Predictive Error +* 0x20 Generic predictive error +* 0x21 Predictive error, degraded performance +* 0x22 Predictive error, fault may be corrected after reboot +* 0x23 Predictive error, fault may be corrected after reboot, +* degraded performance +* 0x24 Predictive error, loss of redundancy +*/ +#define OPAL_PREDICTIVE_ERR_GENERAL 0x20 +#define OPAL_PREDICTIVE_ERR_DEGRADED_PERF 0x21 +#define OPAL_PREDICTIVE_ERR_FAULT_RECTIFY_REBOOT 0x22 +#define OPAL_PREDICTIVE_ERR_FAULT_RECTIFY_BOOT_DEGRADE_PERF 0x23 +#define OPAL_PREDICTIVE_ERR_LOSS_OF_REDUNDANCY 0x24 + +/* 0x4X series for Unrecoverable Error +* 0x40 Generic Unrecoverable error +* 0x41 Unrecoverable error bypassed with degraded performance +* 0x44 Unrecoverable error bypassed with loss of redundancy +* 0x45 Unrecoverable error bypassed with loss of redundancy and performance +* 0x48 Unrecoverable error bypassed with loss of function +*/ +#define OPAL_UNRECOVERABLE_ERR_GENERAL 0x40 +#define OPAL_UNRECOVERABLE_ERR_DEGRADE_PERF 0x41 +#define OPAL_UNRECOVERABLE_ERR_LOSS_REDUNDANCY 0x44 +#define OPAL_UNRECOVERABLE_ERR_LOSS_REDUNDANCY_PERF 0x45 +#define OPAL_UNRECOVERABLE_ERR_LOSS_OF_FUNCTION 0x48 +#define OPAL_ERROR_PANIC 0x50 + +/* Event Sub-type +* This field provides additional information on the non-error +* event type +* Field: event_subtype +*/ +#define OPAL_NA 0x00 +#define OPAL_MISCELLANEOUS_INFO_ONLY 0x01 +#define OPAL_PREV_REPORTED_ERR_RECTIFIED 0x10 +#define OPAL_SYS_RESOURCES_DECONFIG_BY_USER 0x20 +#define OPAL_SYS_RESOURCE_DECONFIG_PRIOR_ERR 0x21 +#define OPAL_RESOURCE_DEALLOC_EVENT_NOTIFY 0x22 +#define OPAL_CONCURRENT_MAINTENANCE_EVENT 0x40 +#define OPAL_CAPACITY_UPGRADE_EVENT 0x60 +#define OPAL_RESOURCE_SPARING_EVENT 0x70 +#define OPAL_DYNAMIC_RECONFIG_EVENT 0x80 +#define OPAL_NORMAL_SYS_PLATFORM_SHUTDOWN 0xD0 +#define OPAL_ABNORMAL_POWER_OFF 0xE0 + +/* Max user dump size is 14K */ +#define OPAL_LOG_MAX_DUMP 14336 + +/* Multiple user data sections */ +struct opal_usr_data_scn { +uint32_t tag; +uint16_t size; +uint16_t component_id; +char data_dump[4 ]; +}; + +/* All the information regarding an error/event to be reported +* needs to populate this structure using pre-defined interfaces +* only +*/ +struct opal_errorlog { + +uint16_t component_id; +uint8_t error_events_type:3 ; +uint8_t subsystem_id; + +uint8_t event_sev; +uint8_t event_subtype; +uint8_t usr_scn_count;/* User section count */ +uint8_t elog_origin; + +uint32_t usr_scn_size;/* User section size */ +uint32_t reason_code; +uint32_t additional_info[4 ]; + +char usr_data_dump[OPAL_LOG_MAX_DUMP ]; +}; +/* Machine check related definitions */ enum OpalMCE_Version { OpalMCE_V1 =1 ,
@@ -853,6 +970,14 @@ int64_t opal_dump_ack(uint32_t dump_id);int64_t opal_get_msg (uint64_t buffer,size_t size);int64_t opal_check_completion (uint64_t buffer,size_t size,uint64_t token);int64_t opal_sync_host_reboot (void ); +struct opal_errorlog *elog_create (uint8_t err_evt_type,uint16_t component_id, +uint8_t subsystem_id,uint8_t event_sev,uint8_t event_subtype, +uint32_t reason_code,uint32_t info0,uint32_t info1, +uint32_t info2,uint32_t info3); +int update_user_dump (struct opal_errorlog *buf,unsigned char *data, +uint32_t tag,uint16_t size); +void commit_errorlog_to_fsp (struct opal_errorlog *buf); +int opal_commit_log_to_fsp (void *buf);/* Internal functions */ extern int early_init_dt_scan_opal (unsigned long node,const char *uname,int depth,void *data);
arch/powerpc/platforms/powernv/opal-elog.c +59 -1
78 lines not on this page · raw message
arch/powerpc/platforms/powernv/opal-wrappers.S +1 -0
@@ -133,3 +133,4 @@ OPAL_CALL(opal_dump_ack, OPAL_DUMP_ACK); OPAL_CALL(opal_get_msg, OPAL_GET_MSG); OPAL_CALL(opal_check_completion, OPAL_CHECK_ASYNC_COMPLETION); OPAL_CALL(opal_sync_host_reboot, OPAL_SYNC_HOST_REBOOT); +OPAL_CALL(opal_commit_log_to_fsp, OPAL_ELOG_SEND);
Regards,
Deepthi
Michael Ellerman
reviewerQuoted 9 lines · Deepthi Dharwar
This patch provides error logging interfaces to report critical powernv error to FSP.
All the required information to dump the error is collected at POWERNV level through error log interfaces
and then pushed on to FSP.This also supports synchronous error logging in cases of PANIC, by passing
OPAL_ERROR_PANICas severity inelog_create()call.
Please make note of the fact that none of this code is currently used but will be in a subsequent patch. When can we expect those patches?
Quoted 13 lines
arch/powerpc/include/asm/opal.h +1
-0@@ -168,6 +168,7 @@ extern int opal_enter_rtas(struct rtas_args *args,#define OPAL_GET_MSG 85#define OPAL_CHECK_ASYNC_COMPLETION 86#define OPAL_SYNC_HOST_REBOOT 87 +#define OPAL_ELOG_SEND 88#ifndef __ASSEMBLY__
arch/powerpc/include/asm/opal.h enum OpalMessageType { · line 264
Quoted 4 lines
arch/powerpc/include/asm/opal.h +1
-0@@ -260,6 +261,122 @@ enum OpalMessageType { OPAL_MSG_TYPE_MAX, }; +/* Classification of Error/Events reporting type classification
Standard comment style for block comments is:
/*
* Classification ...
*/
That applies to almost all of your comments in here.
arch/powerpc/include/asm/opal.h enum OpalMessageType { · line 265
Quoted 1 line
arch/powerpc/include/asm/opal.h +1
-0+* Platform Events/Errors: Report Machine Check Interrupt
I think these comments would be better inline with the values, eg:
/* Report Machine Check Interrupt */ OPAL_PLATFORM, /* Report all I/O related events/errors */ OPAL_INPUT_OUTPUT, etc.
Again that applies to most of your comments.
arch/powerpc/include/asm/opal.h enum OpalMessageType { · line 269
Quoted 4 lines
arch/powerpc/include/asm/opal.h +4
-0+* INPUT_OUTPUT: Report all I/O related events/errors +* RESOURCE_DEALLOC: Hotplug events and errors +* MISC: Miscellanous error +* Field: error_events_type
What is this "Field:" thing about?
arch/powerpc/include/asm/opal.h enum OpalMessageType { · line 271
Quoted 2 lines
arch/powerpc/include/asm/opal.h +2
-0+*/ +enum error_events {
If you're going to define an enum you should actually use it in the API, I can't see anywhere you do?
If you do want to use an enum it should be "opal_error_events".
Quoted 70 lines
arch/powerpc/include/asm/opal.h +70
-0+OPAL_PLATFORM , +OPAL_INPUT_OUTPUT , +OPAL_RESOURCE_DEALLOC , + OPAL_MISC, +}; + +/* OPAL Subsystem IDs listed for reporting events/errors +* Field: subsystem_id +*/ + +#define OPAL_PROCESSOR_SUBSYSTEM 0x10 +#define OPAL_MEMORY_SUBSYSTEM 0x20 +#define OPAL_IO_SUBSYSTEM 0x30 +#define OPAL_IO_DEVICES 0x40 +#define OPAL_CEC_HARDWARE 0x50 +#define OPAL_POWER_COOLING 0x60 +#define OPAL_MISC_SUBSYSTEM 0x70 +#define OPAL_SURVEILLANCE_ERR 0x7A +#define OPAL_PLATFORM_FIRMWARE 0x80 +#define OPAL_SOFTWARE 0x90 +#define OPAL_EXTERNAL_ENV 0xA0 + +/* During reporting an event/error the following represents +* how serious the logged event/error is. (Severity) +* Field: event_sev +*/ +#define OPAL_INFO 0x00 +#define OPAL_RECOVERED_ERR_GENERAL 0x10 + +/* 0x2X series is to denote set of Predictive Error +* 0x20 Generic predictive error +* 0x21 Predictive error, degraded performance +* 0x22 Predictive error, fault may be corrected after reboot +* 0x23 Predictive error, fault may be corrected after reboot, +* degraded performance +* 0x24 Predictive error, loss of redundancy +*/ +#define OPAL_PREDICTIVE_ERR_GENERAL 0x20 +#define OPAL_PREDICTIVE_ERR_DEGRADED_PERF 0x21 +#define OPAL_PREDICTIVE_ERR_FAULT_RECTIFY_REBOOT 0x22 +#define OPAL_PREDICTIVE_ERR_FAULT_RECTIFY_BOOT_DEGRADE_PERF 0x23 +#define OPAL_PREDICTIVE_ERR_LOSS_OF_REDUNDANCY 0x24 + +/* 0x4X series for Unrecoverable Error +* 0x40 Generic Unrecoverable error +* 0x41 Unrecoverable error bypassed with degraded performance +* 0x44 Unrecoverable error bypassed with loss of redundancy +* 0x45 Unrecoverable error bypassed with loss of redundancy and performance +* 0x48 Unrecoverable error bypassed with loss of function +*/ +#define OPAL_UNRECOVERABLE_ERR_GENERAL 0x40 +#define OPAL_UNRECOVERABLE_ERR_DEGRADE_PERF 0x41 +#define OPAL_UNRECOVERABLE_ERR_LOSS_REDUNDANCY 0x44 +#define OPAL_UNRECOVERABLE_ERR_LOSS_REDUNDANCY_PERF 0x45 +#define OPAL_UNRECOVERABLE_ERR_LOSS_OF_FUNCTION 0x48 +#define OPAL_ERROR_PANIC 0x50 + +/* Event Sub-type +* This field provides additional information on the non-error +* event type +* Field: event_subtype +*/ +#define OPAL_NA 0x00 +#define OPAL_MISCELLANEOUS_INFO_ONLY 0x01 +#define OPAL_PREV_REPORTED_ERR_RECTIFIED 0x10 +#define OPAL_SYS_RESOURCES_DECONFIG_BY_USER 0x20 +#define OPAL_SYS_RESOURCE_DECONFIG_PRIOR_ERR 0x21 +#define OPAL_RESOURCE_DEALLOC_EVENT_NOTIFY 0x22 +#define OPAL_CONCURRENT_MAINTENANCE_EVENT 0x40 +#define OPAL_CAPACITY_UPGRADE_EVENT 0x60
arch/powerpc/include/asm/opal.h enum OpalMessageType { · line 351
Quoted 10 lines
arch/powerpc/include/asm/opal.h +10
-0+#define OPAL_RESOURCE_SPARING_EVENT 0x70 +#define OPAL_DYNAMIC_RECONFIG_EVENT 0x80 +#define OPAL_NORMAL_SYS_PLATFORM_SHUTDOWN 0xD0 +#define OPAL_ABNORMAL_POWER_OFF 0xE0 + +/* Max user dump size is 14K */ +#define OPAL_LOG_MAX_DUMP 14336 + +/* Multiple user data sections */ +struct opal_usr_data_scn {
Just spell it out? opal_user_data_section
Quoted 4 lines
arch/powerpc/include/asm/opal.h +4
-0+uint32_t tag; +uint16_t size; +uint16_t component_id; +char data_dump[4 ];
arch/powerpc/include/asm/opal.h enum OpalMessageType { · line 365
Quoted 10 lines
arch/powerpc/include/asm/opal.h +10
-0+}; + +/* All the information regarding an error/event to be reported +* needs to populate this structure using pre-defined interfaces +* only +*/ +struct opal_errorlog { + +uint16_t component_id; +uint8_t error_events_type:3 ;
Bit field?
arch/powerpc/include/asm/opal.h enum OpalMessageType { · line 370
Quoted 5 lines
arch/powerpc/include/asm/opal.h +5
-0+uint8_t subsystem_id; + +uint8_t event_sev; +uint8_t event_subtype; +uint8_t usr_scn_count;/* User section count */
user_section_count;
arch/powerpc/include/asm/opal.h enum OpalMessageType { · line 373
Quoted 3 lines
arch/powerpc/include/asm/opal.h +3
-0+uint8_t elog_origin; + +uint32_t usr_scn_size;/* User section size */
user_section_size;
arch/powerpc/include/asm/opal.h enum OpalMessageType { · line 378
Quoted 5 lines
arch/powerpc/include/asm/opal.h +5
-0+uint32_t reason_code; +uint32_t additional_info[4 ]; + +char usr_data_dump[OPAL_LOG_MAX_DUMP ]; +};
It looks like this goes straight to Opal, should we be using __packed ?
arch/powerpc/include/asm/opal.h int64_t opal_dump_ack(uint32_t dump_id); · line 980
Quoted 12 lines
arch/powerpc/include/asm/opal.h +8
-0@@ -853,6 +970,14 @@ int64_t opal_dump_ack(uint32_t dump_id);int64_t opal_get_msg (uint64_t buffer,size_t size);int64_t opal_check_completion (uint64_t buffer,size_t size,uint64_t token);int64_t opal_sync_host_reboot (void ); +struct opal_errorlog *elog_create (uint8_t err_evt_type,uint16_t component_id, +uint8_t subsystem_id,uint8_t event_sev,uint8_t event_subtype, +uint32_t reason_code,uint32_t info0,uint32_t info1, +uint32_t info2,uint32_t info3); +int update_user_dump (struct opal_errorlog *buf,unsigned char *data, +uint32_t tag,uint16_t size); +void commit_errorlog_to_fsp (struct opal_errorlog *buf); +int opal_commit_log_to_fsp (void *buf);
Are we using "opal_" as a prefix or not?
Quoted 13 lines
arch/powerpc/platforms/powernv/opal-elog.c +1
-0@@ -15,6 +15,7 @@#include <linux/sysfs.h> #include <linux/fs.h> #include <linux/vmalloc.h> +#include <linux/mm.h> #include <linux/fcntl.h> #include <asm/uaccess.h> #include <asm/opal.h>
arch/powerpc/platforms/powernv/opal-elog.c line 26
Quoted 5 lines
arch/powerpc/platforms/powernv/opal-elog.c +1
-1@@ -22,7 +23,9 @@/* Maximum size of a single log on FSP is 16KB */ #define OPAL_MAX_ERRLOG_SIZE 16384 -/* maximu number of records powernv can hold */ +#define USR_CHAR_ARRAY_FIXED_SIZE 4
What is this?
arch/powerpc/platforms/powernv/opal-elog.c line 28
Quoted 1 line
arch/powerpc/platforms/powernv/opal-elog.c +1
-0+/* Maximum number of records powernv can hold */
That's an unrelated typo fix AFAICS, please send it separately.
arch/powerpc/platforms/powernv/opal-elog.c static int init_err_log_buffer(void) · line 282
Quoted 12 lines
arch/powerpc/platforms/powernv/opal-elog.c +5
-0#define MAX_NUM_RECORD 128struct opal_err_log {@@ -272,6 +275,61 @@ static int init_err_log_buffer(void)return 0 ; } +/* Interface to be used by POWERNV to push the logs to FSP via Sapphire */ +struct opal_errorlog *elog_create (uint8_t err_evt_type,uint16_t component_id, +uint8_t subsystem_id,uint8_t event_sev,uint8_t event_subtype, +uint32_t reason_code,uint32_t info0,uint32_t info1, +uint32_t info2,uint32_t info3)
A call to this function is going to be just a giant list of integer values, it will not be easy to see at a glance which value goes in which field.
I think you'd be better off with an elog_alloc() routine, and then you just do the initialisation explicitly so that it's obvious which value goes where:
elog->error_events_type = FOO; elog->component_id = BAR; elog->subsystem_id = ETC;
elog_create(uint8_t err_evt_type, uint16_t component_id,
Quoted 23 lines
arch/powerpc/platforms/powernv/opal-elog.c +23
-0+ uint8_t subsystem_id, uint8_t event_sev, uint8_t event_subtype, + uint32_t reason_code, uint32_t info0, uint32_t info1, + uint32_t info2, uint32_t info3) +{ + struct opal_errorlog *buf; + + buf =kzalloc (sizeof (struct opal_errorlog ),GFP_KERNEL ); +if (!buf) { +printk (KERN_ERR "ELOG: failed to allocate memory.\n" ); +return NULL ; + } + + buf->error_events_type = err_evt_type; + buf->component_id = component_id; + buf->subsystem_id = subsystem_id; + buf->event_sev = event_sev; + buf->event_subtype = event_subtype; + buf->reason_code = reason_code; + buf->additional_info[0 ] = info0; + buf->additional_info[1 ] = info1; + buf->additional_info[2 ] = info2; + buf->additional_info[3 ] = info3; +return buf;
arch/powerpc/platforms/powernv/opal-elog.c static int init_err_log_buffer(void) · line 312
Quoted 10 lines
arch/powerpc/platforms/powernv/opal-elog.c +10
-0+} + +int update_user_dump (struct opal_errorlog *buf,unsigned char *data, +uint32_t tag,uint16_t size) +{ +char *buffer = (char *)buf->usr_data_dump + buf->usr_scn_size; +struct opal_usr_data_scn *tmp; + +if ((buf->usr_scn_size + size) >OPAL_LOG_MAX_DUMP ) { +printk (KERN_ERR "ELOG: Size of dump data overruns buffer" );
Use pr_err() and set pr_fmt() to "opal-error-log" at the top of the file.
Quoted 7 lines
arch/powerpc/platforms/powernv/opal-elog.c +7
-0+return -1 ; + } + + tmp = (struct opal_usr_data_scn *)buffer; + tmp->tag = tag; + tmp->size = size +sizeof (struct opal_usr_data_scn ) + -USR_CHAR_ARRAY_FIXED_SIZE ;
arch/powerpc/platforms/powernv/opal-elog.c static int init_err_log_buffer(void) · line 329
Quoted 10 lines
arch/powerpc/platforms/powernv/opal-elog.c +10
-0+memcpy (tmp->data_dump, data, size); + + buf->usr_scn_size += tmp->size; + buf->usr_scn_count++; +return 0 ; +} + +void commit_errorlog_to_fsp (struct opal_errorlog *buf) +{ +opal_commit_log_to_fsp ((void *)(vmalloc_to_pfn (buf) <<PAGE_SHIFT ));
Can't fail?
arch/powerpc/platforms/powernv/opal-elog.c static int init_err_log_buffer(void) · line 330
Quoted 1 line
arch/powerpc/platforms/powernv/opal-elog.c +1
-0+kfree (buf);
It's a bit rude to free buf when the caller still has a pointer to it.
arch/powerpc/platforms/powernv/opal-elog.c static int init_err_log_buffer(void) · line 334
Quoted 1 line
arch/powerpc/platforms/powernv/opal-elog.c +1
-0+}
cheers
Hi Micheal,
Thanks for the review.
Quoted 7 lines · Michael Ellerman
Quoted 6 lines · Deepthi Dharwar
This patch provides error logging interfaces to report critical powernv error to FSP.
All the required information to dump the error is collected at POWERNV level through error log interfaces
and then pushed on to FSP.
Quoted 6 lines
Quoted 3 lines
This also supports synchronous error logging in cases of PANIC, by passing
OPAL_ERROR_PANICas severity inelog_create()call.Please make note of the fact that none of this code is currently used but will be in a subsequent patch. When can we expect those patches?
This patch only adds the framework to log errors. Coming days this framework will be used to report all POWERNV errors in a phased manner. We would ideally be targeting one sub-system at a time and use these interfaces.
Quoted 41 lines
Quoted 14 lines
arch/powerpc/include/asm/opal.h +1
-0@@ -168,6 +168,7 @@ extern int opal_enter_rtas(struct rtas_args *args,#define OPAL_GET_MSG 85#define OPAL_CHECK_ASYNC_COMPLETION 86#define OPAL_SYNC_HOST_REBOOT 87 +#define OPAL_ELOG_SEND 88#ifndef __ASSEMBLY__arch/powerpc/include/asm/opal.h enum OpalMessageType { · line 264
Quoted 4 lines
arch/powerpc/include/asm/opal.h +1
-0@@ -260,6 +261,122 @@ enum OpalMessageType { OPAL_MSG_TYPE_MAX, }; +/* Classification of Error/Events reporting type classification Standard comment style for block comments is:
/*
* Classification ...
*/That applies to almost all of your comments in here.
arch/powerpc/include/asm/opal.h enum OpalMessageType { · line 265
Quoted 1 line
arch/powerpc/include/asm/opal.h +1
-0+* Platform Events/Errors: Report Machine Check Interrupt I think these comments would be better inline with the values, eg:
/* Report Machine Check Interrupt */ OPAL_PLATFORM, /* Report all I/O related events/errors */ OPAL_INPUT_OUTPUT, etc.
Quoted 1 line
Again that applies to most of your comments.
Sure, I'll make it inline.
Quoted 6 lines
arch/powerpc/include/asm/opal.h enum OpalMessageType { · line 269
Quoted 4 lines
arch/powerpc/include/asm/opal.h +4
-0+* INPUT_OUTPUT: Report all I/O related events/errors +* RESOURCE_DEALLOC: Hotplug events and errors +* MISC: Miscellanous error +* Field: error_events_type What is this "Field:" thing about?
Field is just to add some readability that these options relate to corresponding elog_create argument field.
Looks like the purpose is not getting solved.
Quoted 7 lines
arch/powerpc/include/asm/opal.h enum OpalMessageType { · line 271
Quoted 2 lines
arch/powerpc/include/asm/opal.h +2
-0+*/ +enum error_events {If you're going to define an enum you should actually use it in the API, I can't see anywhere you do?
If you do want to use an enum it should be "
opal_error_events".
Agree.
Quoted 73 lines
Quoted 73 lines
arch/powerpc/include/asm/opal.h +73
-0+OPAL_PLATFORM , +OPAL_INPUT_OUTPUT , +OPAL_RESOURCE_DEALLOC , + OPAL_MISC, +}; +/* OPAL Subsystem IDs listed for reporting events/errors +* Field: subsystem_id +*/ + +#define OPAL_PROCESSOR_SUBSYSTEM 0x10 +#define OPAL_MEMORY_SUBSYSTEM 0x20 +#define OPAL_IO_SUBSYSTEM 0x30 +#define OPAL_IO_DEVICES 0x40 +#define OPAL_CEC_HARDWARE 0x50 +#define OPAL_POWER_COOLING 0x60 +#define OPAL_MISC_SUBSYSTEM 0x70 +#define OPAL_SURVEILLANCE_ERR 0x7A +#define OPAL_PLATFORM_FIRMWARE 0x80 +#define OPAL_SOFTWARE 0x90 +#define OPAL_EXTERNAL_ENV 0xA0 + +/* During reporting an event/error the following represents +* how serious the logged event/error is. (Severity) +* Field: event_sev +*/ +#define OPAL_INFO 0x00 +#define OPAL_RECOVERED_ERR_GENERAL 0x10 + +/* 0x2X series is to denote set of Predictive Error +* 0x20 Generic predictive error +* 0x21 Predictive error, degraded performance +* 0x22 Predictive error, fault may be corrected after reboot +* 0x23 Predictive error, fault may be corrected after reboot, +* degraded performance +* 0x24 Predictive error, loss of redundancy +*/ +#define OPAL_PREDICTIVE_ERR_GENERAL 0x20 +#define OPAL_PREDICTIVE_ERR_DEGRADED_PERF 0x21 +#define OPAL_PREDICTIVE_ERR_FAULT_RECTIFY_REBOOT 0x22 +#define OPAL_PREDICTIVE_ERR_FAULT_RECTIFY_BOOT_DEGRADE_PERF 0x23 +#define OPAL_PREDICTIVE_ERR_LOSS_OF_REDUNDANCY 0x24 + +/* 0x4X series for Unrecoverable Error +* 0x40 Generic Unrecoverable error +* 0x41 Unrecoverable error bypassed with degraded performance +* 0x44 Unrecoverable error bypassed with loss of redundancy +* 0x45 Unrecoverable error bypassed with loss of redundancy and performance +* 0x48 Unrecoverable error bypassed with loss of function +*/ +#define OPAL_UNRECOVERABLE_ERR_GENERAL 0x40 +#define OPAL_UNRECOVERABLE_ERR_DEGRADE_PERF 0x41 +#define OPAL_UNRECOVERABLE_ERR_LOSS_REDUNDANCY 0x44 +#define OPAL_UNRECOVERABLE_ERR_LOSS_REDUNDANCY_PERF 0x45 +#define OPAL_UNRECOVERABLE_ERR_LOSS_OF_FUNCTION 0x48 +#define OPAL_ERROR_PANIC 0x50 + +/* Event Sub-type +* This field provides additional information on the non-error +* event type +* Field: event_subtype +*/ +#define OPAL_NA 0x00 +#define OPAL_MISCELLANEOUS_INFO_ONLY 0x01 +#define OPAL_PREV_REPORTED_ERR_RECTIFIED 0x10 +#define OPAL_SYS_RESOURCES_DECONFIG_BY_USER 0x20 +#define OPAL_SYS_RESOURCE_DECONFIG_PRIOR_ERR 0x21 +#define OPAL_RESOURCE_DEALLOC_EVENT_NOTIFY 0x22 +#define OPAL_CONCURRENT_MAINTENANCE_EVENT 0x40 +#define OPAL_CAPACITY_UPGRADE_EVENT 0x60 +#define OPAL_RESOURCE_SPARING_EVENT 0x70 +#define OPAL_DYNAMIC_RECONFIG_EVENT 0x80 +#define OPAL_NORMAL_SYS_PLATFORM_SHUTDOWN 0xD0 +#define OPAL_ABNORMAL_POWER_OFF 0xE0
Quoted 8 lines
arch/powerpc/include/asm/opal.h enum OpalMessageType { · line 351
Quoted 6 lines
arch/powerpc/include/asm/opal.h +6
-0+ +/* Max user dump size is 14K */ +#define OPAL_LOG_MAX_DUMP 14336 + +/* Multiple user data sections */ +struct opal_usr_data_scn {Just spell it out?
opal_user_data_section
Sure.
Quoted 31 lines
Quoted 4 lines
arch/powerpc/include/asm/opal.h +4
-0+uint32_t tag; +uint16_t size; +uint16_t component_id; +char data_dump[4 ];arch/powerpc/include/asm/opal.h enum OpalMessageType { · line 365
Quoted 10 lines
arch/powerpc/include/asm/opal.h +10
-0+}; + +/* All the information regarding an error/event to be reported +* needs to populate this structure using pre-defined interfaces +* only +*/ +struct opal_errorlog { + +uint16_t component_id; +uint8_t error_events_type:3 ;Bit field?
arch/powerpc/include/asm/opal.h enum OpalMessageType { · line 370
Quoted 5 lines
arch/powerpc/include/asm/opal.h +5
-0+uint8_t subsystem_id; + +uint8_t event_sev; +uint8_t event_subtype; +uint8_t usr_scn_count;/* User section count */
user_section_count;arch/powerpc/include/asm/opal.h enum OpalMessageType { · line 373
Quoted 3 lines
arch/powerpc/include/asm/opal.h +3
-0+uint8_t elog_origin; + +uint32_t usr_scn_size;/* User section size */
user_section_size;
Quoted 7 lines
arch/powerpc/include/asm/opal.h enum OpalMessageType { · line 378
Quoted 5 lines
arch/powerpc/include/asm/opal.h +5
-0+uint32_t reason_code; +uint32_t additional_info[4 ]; + +char usr_data_dump[OPAL_LOG_MAX_DUMP ]; +};It looks like this goes straight to Opal, should we be using
__packed?
Yes, this goes straight into Opal. The structure is defined such that it is packed by default, this will not require compiler to pack bytes.
Quoted 15 lines
arch/powerpc/include/asm/opal.h int64_t opal_dump_ack(uint32_t dump_id); · line 980
Quoted 13 lines
arch/powerpc/include/asm/opal.h +8
-0@@ -853,6 +970,14 @@ int64_t opal_dump_ack(uint32_t dump_id);int64_t opal_get_msg (uint64_t buffer,size_t size);int64_t opal_check_completion (uint64_t buffer,size_t size,uint64_t token);int64_t opal_sync_host_reboot (void ); +struct opal_errorlog *elog_create (uint8_t err_evt_type,uint16_t component_id, +uint8_t subsystem_id,uint8_t event_sev,uint8_t event_subtype, +uint32_t reason_code,uint32_t info0,uint32_t info1, +uint32_t info2,uint32_t info3); +int update_user_dump (struct opal_errorlog *buf,unsigned char *data, +uint32_t tag,uint16_t size); +void commit_errorlog_to_fsp (struct opal_errorlog *buf); +int opal_commit_log_to_fsp (void *buf);Are we using "
opal_" as a prefix or not?
Uniformity is better. Shall follow the signature here too.
Quoted 16 lines
Quoted 16 lines
arch/powerpc/platforms/powernv/opal-elog.c +1
-0@@ -15,6 +15,7 @@#include <linux/sysfs.h> #include <linux/fs.h> #include <linux/vmalloc.h> +#include <linux/mm.h> #include <linux/fcntl.h> #include <asm/uaccess.h> #include <asm/opal.h> @@ -22,7 +23,9 @@/* Maximum size of a single log on FSP is 16KB */ #define OPAL_MAX_ERRLOG_SIZE 16384
Quoted 4 lines
arch/powerpc/platforms/powernv/opal-elog.c line 26
Quoted 2 lines
arch/powerpc/platforms/powernv/opal-elog.c +1
-1-/* maximu number of records powernv can hold */ +#define USR_CHAR_ARRAY_FIXED_SIZE 4What is this?
struct User data section is mapped to a buffer. As all the structures are padded, we need to subtract the same to do data manipulation. Make me need to re-word it or use __packed here.
Quoted 4 lines
arch/powerpc/platforms/powernv/opal-elog.c line 28
Quoted 2 lines
arch/powerpc/platforms/powernv/opal-elog.c +1
-0+/* Maximum number of records powernv can hold */ That's an unrelated typo fix AFAICS, please send it separately.
Sure.
Quoted 18 lines
arch/powerpc/platforms/powernv/opal-elog.c static int init_err_log_buffer(void) · line 282
Quoted 13 lines
arch/powerpc/platforms/powernv/opal-elog.c +5
-0#define MAX_NUM_RECORD 128struct opal_err_log {@@ -272,6 +275,61 @@ static int init_err_log_buffer(void)return 0 ; } +/* Interface to be used by POWERNV to push the logs to FSP via Sapphire */ +struct opal_errorlog *elog_create (uint8_t err_evt_type,uint16_t component_id, +uint8_t subsystem_id,uint8_t event_sev,uint8_t event_subtype, +uint32_t reason_code,uint32_t info0,uint32_t info1, +uint32_t info2,uint32_t info3)A call to this function is going to be just a giant list of integer values, it will not be easy to see at a glance which value goes in which field.
Quoted 7 lines
I think you'd be better off with an
elog_alloc()routine, and then you just do the initialisation explicitly so that it's obvious which value goes where:elog->error_events_type = FOO; elog->component_id = BAR; elog->subsystem_id = ETC;
elog_create() will be called by all sub-systems on POWERNV platform to log events and errors. I feel we are better off passing all the required arguments to the interface than initialize explicitly.
This would have a cleaner interface to error logging by
1) Removing huge amount of code duplication ( Each and every error/event to be reported needs to initialise fields of the opal_errorlog struct done many many times on POWERNV, results in redundant code )
2) There are chances of missing out on initialising key fields if done by the user. Having an interface mandates the fields that needs to populated while logging error/events.
Quoted 28 lines
elog_create(uint8_t
err_evt_type,uint16_tcomponent_id,Quoted 27 lines
arch/powerpc/platforms/powernv/opal-elog.c +27
-0+ uint8_t subsystem_id, uint8_t event_sev, uint8_t event_subtype, + uint32_t reason_code, uint32_t info0, uint32_t info1, + uint32_t info2, uint32_t info3) +{ + struct opal_errorlog *buf; + + buf =kzalloc (sizeof (struct opal_errorlog ),GFP_KERNEL ); +if (!buf) { +printk (KERN_ERR "ELOG: failed to allocate memory.\n" ); +return NULL ; + } + + buf->error_events_type = err_evt_type; + buf->component_id = component_id; + buf->subsystem_id = subsystem_id; + buf->event_sev = event_sev; + buf->event_subtype = event_subtype; + buf->reason_code = reason_code; + buf->additional_info[0 ] = info0; + buf->additional_info[1 ] = info1; + buf->additional_info[2 ] = info2; + buf->additional_info[3 ] = info3; +return buf; +} + +int update_user_dump (struct opal_errorlog *buf,unsigned char *data, +uint32_t tag,uint16_t size)
Quoted 8 lines
arch/powerpc/platforms/powernv/opal-elog.c static int init_err_log_buffer(void) · line 312
Quoted 6 lines
arch/powerpc/platforms/powernv/opal-elog.c +6
-0+{ +char *buffer = (char *)buf->usr_data_dump + buf->usr_scn_size; +struct opal_usr_data_scn *tmp; + +if ((buf->usr_scn_size + size) >OPAL_LOG_MAX_DUMP ) { +printk (KERN_ERR "ELOG: Size of dump data overruns buffer" );Use
pr_err()and setpr_fmt()to "opal-error-log" at the top of the file.
Sure. I'll do that.
Quoted 11 lines
Quoted 11 lines
arch/powerpc/platforms/powernv/opal-elog.c +11
-0+return -1 ; + } + + tmp = (struct opal_usr_data_scn *)buffer; + tmp->tag = tag; + tmp->size = size +sizeof (struct opal_usr_data_scn ) + -USR_CHAR_ARRAY_FIXED_SIZE ; +memcpy (tmp->data_dump, data, size); + + buf->usr_scn_size += tmp->size; + buf->usr_scn_count++;
Quoted 8 lines
arch/powerpc/platforms/powernv/opal-elog.c static int init_err_log_buffer(void) · line 329
Quoted 6 lines
arch/powerpc/platforms/powernv/opal-elog.c +6
-0+return 0 ; +} + +void commit_errorlog_to_fsp (struct opal_errorlog *buf) +{ +opal_commit_log_to_fsp ((void *)(vmalloc_to_pfn (buf) <<PAGE_SHIFT ));Can't fail?
It is better to have a return here, atleast for the caller to know if opal handling of the same is successful or not. I will make the required change.
Quoted 3 lines
arch/powerpc/platforms/powernv/opal-elog.c static int init_err_log_buffer(void) · line 330
Quoted 1 line
arch/powerpc/platforms/powernv/opal-elog.c +1
-0+kfree (buf);It's a bit rude to free buf when the caller still has a pointer to it.
Technically, after the error log has been committed, the user is not supposed to re-use or do anything with that buffer. I need to add checks in all my routines if(buf != NULL), to handle the case where the user by mistake is trying to use the same buffer pointer.
Regards,
Deepthi
Quoted 7 lines
arch/powerpc/platforms/powernv/opal-elog.c static int init_err_log_buffer(void) · line 334
Quoted 1 line
arch/powerpc/platforms/powernv/opal-elog.c +1
-0+}cheers
Michael Ellerman
reviewerQuoted 3 lines · Deepthi Dharwar
Hi Micheal,
Thanks for the review.
No worries.
Quoted 35 lines · Michael Ellerman
Quoted 26 lines · Deepthi Dharwar
arch/powerpc/include/asm/opal.h enum OpalMessageType { · line 365
Quoted 8 lines
arch/powerpc/include/asm/opal.h +8
-0+/* All the information regarding an error/event to be reported +* needs to populate this structure using pre-defined interfaces +* only +*/ +struct opal_errorlog { + +uint16_t component_id; +uint8_t error_events_type:3 ;Bit field?
arch/powerpc/include/asm/opal.h enum OpalMessageType { · line 370
Quoted 5 lines
arch/powerpc/include/asm/opal.h +5
-0+uint8_t subsystem_id; + +uint8_t event_sev; +uint8_t event_subtype; +uint8_t usr_scn_count;/* User section count */
user_section_count;arch/powerpc/include/asm/opal.h enum OpalMessageType { · line 373
Quoted 3 lines
arch/powerpc/include/asm/opal.h +3
-0+uint8_t elog_origin; + +uint32_t usr_scn_size;/* User section size */
user_section_size;Quoted 7 lines
arch/powerpc/include/asm/opal.h enum OpalMessageType { · line 378
Quoted 5 lines
arch/powerpc/include/asm/opal.h +5
-0+uint32_t reason_code; +uint32_t additional_info[4 ]; + +char usr_data_dump[OPAL_LOG_MAX_DUMP ]; +};It looks like this goes straight to Opal, should we be using
__packed?
Quoted 2 lines
Yes, this goes straight into Opal. The structure is defined such that it is packed by default, this will not require compiler to pack bytes.
Sure, but the compiler might decide to lay it out differently for some reason. You should use __packed.
Also bitfields are essentially a big "implementation defined behaviour" sign, so I would avoid using the bitfield.
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
arch/powerpc/platforms/powernv/opal-elog.c +0
-0@@ -22,7 +23,9 @@/* Maximum size of a single log on FSP is 16KB */ #define OPAL_MAX_ERRLOG_SIZE 16384
Quoted 8 lines
Quoted 4 lines
arch/powerpc/platforms/powernv/opal-elog.c line 26
Quoted 2 lines
arch/powerpc/platforms/powernv/opal-elog.c +1
-1-/* maximu number of records powernv can hold */ +#define USR_CHAR_ARRAY_FIXED_SIZE 4What is this?
struct User data section is mapped to a buffer. As all the structures are padded, we need to subtract the same to do data manipulation. Make me need to re-word it or use
__packedhere.
Yeah that's still not really clear to me, so if you can do something that is more obvious that would be good.
Quoted 22 lines
Quoted 14 lines
arch/powerpc/platforms/powernv/opal-elog.c static int init_err_log_buffer(void) · line 282
Quoted 9 lines
arch/powerpc/platforms/powernv/opal-elog.c +5
-0@@ -272,6 +275,61 @@ static int init_err_log_buffer(void)return 0 ; } +/* Interface to be used by POWERNV to push the logs to FSP via Sapphire */ +struct opal_errorlog *elog_create (uint8_t err_evt_type,uint16_t component_id, +uint8_t subsystem_id,uint8_t event_sev,uint8_t event_subtype, +uint32_t reason_code,uint32_t info0,uint32_t info1, +uint32_t info2,uint32_t info3)A call to this function is going to be just a giant list of integer values, it will not be easy to see at a glance which value goes in which field.
Quoted 7 lines
I think you'd be better off with an
elog_alloc()routine, and then you just do the initialisation explicitly so that it's obvious which value goes where:elog->error_events_type = FOO; elog->component_id = BAR; elog->subsystem_id = ETC;
Quoted 7 lines
elog_create()will be called by all sub-systems on POWERNV platform to log events and errors. I feel we are better off passing all the required arguments to the interface than initialize explicitly.
This would have a cleaner interface to error logging by
1) Removing huge amount of code duplication ( Each and every error/event to be reported needs to initialise fields of theopal_errorlogstruct done many many times on POWERNV, results in redundant code )
It will be more lines of code, but it might be more readable code.
Quoted 3 lines
2) There are chances of missing out on initialising key fields if done by the user. Having an interface mandates the fields that needs to populated while logging error/events.
I can always pass 0 :)
We will see how it looks once there are some callers.
Quoted 10 lines
Quoted 5 lines
arch/powerpc/platforms/powernv/opal-elog.c static int init_err_log_buffer(void) · line 329
Quoted 3 lines
arch/powerpc/platforms/powernv/opal-elog.c +3
-0+void commit_errorlog_to_fsp (struct opal_errorlog *buf) +{ +opal_commit_log_to_fsp ((void *)(vmalloc_to_pfn (buf) <<PAGE_SHIFT ));Can't fail?
It is better to have a return here, atleast for the caller to know if opal handling of the same is successful or not. I will make the required change.
Quoted 8 lines
Quoted 3 lines
arch/powerpc/platforms/powernv/opal-elog.c static int init_err_log_buffer(void) · line 330
Quoted 1 line
arch/powerpc/platforms/powernv/opal-elog.c +1
-0+kfree (buf);It's a bit rude to free buf when the caller still has a pointer to it.
Technically, after the error log has been committed, the user is not supposed to re-use or do anything with that buffer. I need to add checks in all my routines if(buf != NULL), to handle the case where the user by mistake is trying to use the same buffer pointer.
Why is the user not supposed to re-use it?
kfree()'ing the buffer doesn't prevent the caller from re-using it.
cheers
Quoted 40 lines · Michael Ellerman
Quoted 3 lines · Deepthi Dharwar
Hi Micheal,
Thanks for the review.
No worries.
Quoted 33 lines · Michael Ellerman
Quoted 31 lines · Deepthi Dharwar
arch/powerpc/include/asm/opal.h enum OpalMessageType { · line 365
Quoted 8 lines
arch/powerpc/include/asm/opal.h +8
-0+/* All the information regarding an error/event to be reported +* needs to populate this structure using pre-defined interfaces +* only +*/ +struct opal_errorlog { + +uint16_t component_id; +uint8_t error_events_type:3 ;Bit field?
arch/powerpc/include/asm/opal.h enum OpalMessageType { · line 370
Quoted 5 lines
arch/powerpc/include/asm/opal.h +5
-0+uint8_t subsystem_id; + +uint8_t event_sev; +uint8_t event_subtype; +uint8_t usr_scn_count;/* User section count */
user_section_count;arch/powerpc/include/asm/opal.h enum OpalMessageType { · line 373
Quoted 3 lines
arch/powerpc/include/asm/opal.h +3
-0+uint8_t elog_origin; + +uint32_t usr_scn_size;/* User section size */
user_section_size;Quoted 5 lines
arch/powerpc/include/asm/opal.h +5
-0+uint32_t reason_code; +uint32_t additional_info[4 ]; + +char usr_data_dump[OPAL_LOG_MAX_DUMP ]; +};
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
It looks like this goes straight to Opal, should we be using
__packed?Yes, this goes straight into Opal. The structure is defined such that it is packed by default, this will not require compiler to pack bytes.
Sure, but the compiler might decide to lay it out differently for some reason. You should use
__packed.
Ok.
Quoted 2 lines
Also bitfields are essentially a big "implementation defined behaviour" sign, so I would avoid using the bitfield.
Yes, bitfields will be gone.
Quoted 13 lines
Quoted 13 lines
Quoted 12 lines
arch/powerpc/platforms/powernv/opal-elog.c line 26
Quoted 10 lines
arch/powerpc/platforms/powernv/opal-elog.c +1
-1@@ -22,7 +23,9 @@/* Maximum size of a single log on FSP is 16KB */ #define OPAL_MAX_ERRLOG_SIZE 16384 -/* maximu number of records powernv can hold */ +#define USR_CHAR_ARRAY_FIXED_SIZE 4What is this?
Quoted 6 lines
Quoted 3 lines
struct User data section is mapped to a buffer. As all the structures are padded, we need to subtract the same to do data manipulation. Make me need to re-word it or use
__packedhere.Yeah that's still not really clear to me, so if you can do something that is more obvious that would be good.
Sure.
Quoted 30 lines
Quoted 22 lines
Quoted 14 lines
arch/powerpc/platforms/powernv/opal-elog.c static int init_err_log_buffer(void) · line 282
Quoted 9 lines
arch/powerpc/platforms/powernv/opal-elog.c +5
-0@@ -272,6 +275,61 @@ static int init_err_log_buffer(void)return 0 ; } +/* Interface to be used by POWERNV to push the logs to FSP via Sapphire */ +struct opal_errorlog *elog_create (uint8_t err_evt_type,uint16_t component_id, +uint8_t subsystem_id,uint8_t event_sev,uint8_t event_subtype, +uint32_t reason_code,uint32_t info0,uint32_t info1, +uint32_t info2,uint32_t info3)A call to this function is going to be just a giant list of integer values, it will not be easy to see at a glance which value goes in which field.
Quoted 7 lines
I think you'd be better off with an
elog_alloc()routine, and then you just do the initialisation explicitly so that it's obvious which value goes where:elog->error_events_type = FOO; elog->component_id = BAR; elog->subsystem_id = ETC;Quoted 7 lines
elog_create()will be called by all sub-systems on POWERNV platform to log events and errors. I feel we are better off passing all the required arguments to the interface than initialize explicitly.
This would have a cleaner interface to error logging by
1) Removing huge amount of code duplication ( Each and every error/event to be reported needs to initialise fields of theopal_errorlogstruct done many many times on POWERNV, results in redundant code )
Quoted 7 lines
It will be more lines of code, but it might be more readable code.
Quoted 3 lines
2) There are chances of missing out on initialising key fields if done by the user. Having an interface mandates the fields that needs to populated while logging error/events.
I can always pass 0 :)
I was referring to more on the lines of missing unintentionally :)
Quoted 1 line
We will see how it looks once there are some callers.
Sure. I will retain it for now. Going forward once we start adding users exploiting this interface, we can then take a call.
Quoted 15 lines
Quoted 15 lines
Quoted 6 lines
arch/powerpc/platforms/powernv/opal-elog.c static int init_err_log_buffer(void) · line 329
Quoted 4 lines
arch/powerpc/platforms/powernv/opal-elog.c +3
-0+void commit_errorlog_to_fsp (struct opal_errorlog *buf) +{ +opal_commit_log_to_fsp ((void *)(vmalloc_to_pfn (buf) <<PAGE_SHIFT ));Can't fail?
It is better to have a return here, atleast for the caller to know if opal handling of the same is successful or not. I will make the required change.
Quoted 3 lines
arch/powerpc/platforms/powernv/opal-elog.c static int init_err_log_buffer(void) · line 330
Quoted 1 line
arch/powerpc/platforms/powernv/opal-elog.c +1
-0+kfree (buf);It's a bit rude to free buf when the caller still has a pointer to it.
Quoted 8 lines
Quoted 4 lines
Technically, after the error log has been committed, the user is not supposed to re-use or do anything with that buffer. I need to add checks in all my routines if(buf != NULL), to handle the case where the user by mistake is trying to use the same buffer pointer.
Why is the user not supposed to re-use it?
kfree()'ing the buffer doesn't prevent the caller from re-using it.
Release the memory. Assign pointer to NULL before returning. All the error logging interfaces should have NULL check (to return). User can't do much in that case.
Regards,
Deepthi
Quoted 5 lines
cheers