[PATCH v5] powerpc/pseries: Limit EPOW reset event warnings
Vipin K Parashar
vipin at linux.vnet.ibm.com
Mon Nov 30 23:01:07 AEDT 2015
On Thursday 26 November 2015 02:50 PM, Vasant Hegde wrote:
> On 11/18/2015 02:12 PM, Vipin K Parashar wrote:
>> Kernel prints respective warnings about various EPOW events for
>> user information/action after parsing EPOW interrupts. At times
>> below EPOW reset event warning is seen to be flooding kernel log
>> over a period of time.
>>
>> May 25 03:46:34 alp kernel: Non critical power or cooling issue cleared
>> May 25 03:46:52 alp kernel: Non critical power or cooling issue cleared
>> May 25 03:53:48 alp kernel: Non critical power or cooling issue cleared
>> May 25 03:55:46 alp kernel: Non critical power or cooling issue cleared
>> May 25 03:56:34 alp kernel: Non critical power or cooling issue cleared
>> May 25 03:59:04 alp kernel: Non critical power or cooling issue cleared
>> May 25 04:02:01 alp kernel: Non critical power or cooling issue cleared
>>
> @Michael,
> I think above log is raising some concern. We have been asked by multiple
> people on this. Hence I think we should avoid these duplicate messages.
Hi Michael,
Please do let know if you have some suggestions with this patch.
>
>> These EPOW reset events are spurious in nature and are triggered by
>> firmware witout an actual EPOW event being reset. This patch avoids these
> s/witout/without/
sure, will edit.
>
>> multiple EPOW reset warnings by using a counter variable. This variable
>> is incremented every time an EPOW event is reported. Upon receiving a EPOW
>> reset event the same variable is checked to filer out spurious events and
>> decremented accordingly.
>>
>> This patch also improves log messages to better describe EPOW event being
>> reported. Merged adjacent log messages into single one to reduce number of
>> lines printed per event.
>>
>> Signed-off-by: Kamalesh Babulal<kamalesh at linux.vnet.ibm.com>
>> Signed-off-by: Vipin K Parashar<vipin at linux.vnet.ibm.com>
>> ---
>> v5 changes:
>> - Used num_epow_events counter variable to count number of epow_events
>> - Improved log messages to better describe epow event.
>> - Merged adjacent warnings into single lines.
>>
>> v4 changes:
>> - Changed the approach to depth counter to match the EPOW events and
>> EPOW reset.
>> - Converted pr_err() ot pr_info() for non-critical errors.
>> - Merged adjacent warnings into single line across the file.
>> - Fixed grammar in the warnings to make is short.
>>
>> v3 changes:
>> - Limit warning printed by EPOW RESET event, by guarding it with bool flag.
>> Instead of rate limiting all the EPOW events.
>>
>> v2 changes:
>> - Merged multiple adjacent pr_err/pr_emerg into single line to reduce multi-line
>> warnings, based on Michael's comments.
>>
>> arch/powerpc/platforms/pseries/ras.c | 54 ++++++++++++++++++++----------------
>> 1 file changed, 30 insertions(+), 24 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
>> index 3b6647e..bbe2856 100644
>> --- a/arch/powerpc/platforms/pseries/ras.c
>> +++ b/arch/powerpc/platforms/pseries/ras.c
>> @@ -40,6 +40,8 @@ static int ras_check_exception_token;
>> #define EPOW_SENSOR_TOKEN 9
>> #define EPOW_SENSOR_INDEX 0
>>
>> +static int num_epow_events;
>> +
>> static irqreturn_t ras_epow_interrupt(int irq, void *dev_id);
>> static irqreturn_t ras_error_interrupt(int irq, void *dev_id);
>>
>> @@ -82,32 +84,30 @@ static void handle_system_shutdown(char event_modifier)
>> {
>> switch (event_modifier) {
>> case EPOW_SHUTDOWN_NORMAL:
>> - pr_emerg("Firmware initiated power off");
>> + pr_emerg("Power off requested\n");
> Why are you changing this message? These are FW initiated Power off and helps
> us to identify who initiated shutdown request.
EPOW_SHUTDOWN_NORMAL event maps to DPO event in harwdare, which is received
upon system admin requesting LPAR poweroff. I felt that using FW
initiated poweroff
phrase doesn't convey that poweroff was requested by a user so changed it.
Please do suggest if you have something better to convey message.
>> orderly_poweroff(true);
>> break;
>>
>> case EPOW_SHUTDOWN_ON_UPS:
>> - pr_emerg("Loss of power reported by firmware, system is "
>> - "running on UPS/battery");
>> - pr_emerg("Check RTAS error log for details");
>> + pr_emerg("Loss of system power detected. System is running on"
>> + " UPS/battery. Check RTAS error log for details\n");
>> orderly_poweroff(true);
>> break;
>>
>> case EPOW_SHUTDOWN_LOSS_OF_CRITICAL_FUNCTIONS:
>> - pr_emerg("Loss of system critical functions reported by "
>> - "firmware");
>> - pr_emerg("Check RTAS error log for details");
>> + pr_emerg("Loss of system critical functions detected. Check"
>> + " RTAS error log for details\n");
>> orderly_poweroff(true);
>> break;
>>
>> case EPOW_SHUTDOWN_AMBIENT_TEMPERATURE_TOO_HIGH:
>> - pr_emerg("Ambient temperature too high reported by firmware");
>> - pr_emerg("Check RTAS error log for details");
>> + pr_emerg("High ambient temperature detected. Check RTAS"
>> + " error log for details\n");
>> orderly_poweroff(true);
>> break;
>>
>> default:
>> - pr_err("Unknown power/cooling shutdown event (modifier %d)",
>> + pr_err("Unknown power/cooling shutdown event (modifier = %d)\n",
>> event_modifier);
>> }
>> }
>> @@ -145,40 +145,47 @@ static void rtas_parse_epow_errlog(struct rtas_error_log *log)
>>
>> switch (action_code) {
>> case EPOW_RESET:
>> - pr_err("Non critical power or cooling issue cleared");
>> + if (num_epow_events) {
>> + pr_info("Non critical power/cooling issue cleared\n");
>> + num_epow_events--;
>> + }
>> break;
>>
>> case EPOW_WARN_COOLING:
>> - pr_err("Non critical cooling issue reported by firmware");
>> - pr_err("Check RTAS error log for details");
>> + pr_info("Non-critical cooling issue detected. Check RTAS error"
>> + " log for details\n");
>> + num_epow_events++;
> So every switch-case you are modifying this variable. Not sure this is the best way.
> How about adding if condition after switch to modify this variable ?
yeah i thought about it but dropped the idea as the variable isn't being
incrementing
for all switch cases. Its only getting incremented for cases when we
aren't doing system
poweroff. Though i can send out a patch with a single increment of the
variable down
below the switch case as incrementing it for poweroff cases too would
not no harm.
> -Vasant
>
More information about the Linuxppc-dev
mailing list