[prev in list] [next in list] [prev in thread] [next in thread] 

List:       fwts-devel
Subject:    Re: ACK: [PATCH 2/3] acpi: s3: do kernel log comparisons without destroying klog (LP: #1262208)
From:       Keng-Yu Lin <kengyu () canonical ! com>
Date:       2013-12-24 9:42:49
Message-ID: CADXHx7YDdTcWTqGbBQzY23CNASwjwgdD4HWCQ9MWtBvTB4fHYw () mail ! gmail ! com
[Download RAW message or body]

On Mon, Dec 23, 2013 at 4:09 PM, IvanHu <ivan.hu@canonical.com> wrote:
> On 12/18/2013 10:19 PM, Colin King wrote:
>>
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> Use the new kernel log comapring helper to compare old vs new
>> kernel logs and remove the need to clear the kernel log before
>> each S3 iteration.
>>
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>> ---
>>   src/acpi/s3/s3.c | 36 +++++++++++++++++-------------------
>>   1 file changed, 17 insertions(+), 19 deletions(-)
>>
>> diff --git a/src/acpi/s3/s3.c b/src/acpi/s3/s3.c
>> index 27a5af0..7a86641 100644
>> --- a/src/acpi/s3/s3.c
>> +++ b/src/acpi/s3/s3.c
>> @@ -29,7 +29,7 @@
>>   #include <unistd.h>
>>   #include <time.h>
>>
>> -#define PM_SUSPEND "pm-suspend"
>> +#define PM_SUSPEND     "pm-suspend"
>>   #define FWTS_SUSPEND  "FWTS_SUSPEND"
>>   #define FWTS_RESUME   "FWTS_RESUME"
>>
>> @@ -50,10 +50,6 @@ static int s3_init(fwts_framework *fw)
>>         int ret;
>>
>>         /* Pre-init - make sure wakealarm works so that we can wake up
>> after suspend */
>> -       if (fwts_klog_clear()) {
>> -               fwts_log_error(fw, "Cannot clear kernel log.");
>> -               return FWTS_ERROR;
>> -       }
>>         if ((ret = fwts_wakealarm_test_firing(fw, 1))) {
>>                 fwts_log_error(fw, "Cannot automatically wake machine up -
>> aborting S3 test.");
>>                 fwts_failed(fw, LOG_LEVEL_MEDIUM, "BadWakeAlarmS3",
>> @@ -81,8 +77,6 @@ static int s3_do_suspend_resume(fwts_framework *fw,
>>         char *quirks;
>>         char buffer[80];
>>
>> -       fwts_klog_clear();
>> -
>>         if (s3_device_check)
>>                 fwts_hwinfo_get(fw, &hwinfo1);
>>
>> @@ -271,24 +265,17 @@ static int s3_scan_times(
>>
>>   static int s3_check_log(
>>         fwts_framework *fw,
>> +       fwts_list *klog,
>>         int *errors,
>>         int *oopses,
>>         int *warn_ons,
>>         int *suspend_too_long,
>>         int *resume_too_long)
>>   {
>> -       fwts_list *klog;
>>         int error;
>>         int oops;
>>         int warn_on;
>>
>> -       if ((klog = fwts_klog_read()) == NULL) {
>> -               fwts_log_error(fw, "Cannot read kernel log.");
>> -               fwts_failed(fw, LOG_LEVEL_MEDIUM, "KlogCheckS3",
>> -                       "Unable to check kernel log for S3 suspend/resume
>> test.");
>> -               return FWTS_ERROR;
>> -       }
>> -
>>         if (fwts_klog_pm_check(fw, NULL, klog, &error))
>>                 fwts_log_error(fw, "Error parsing kernel log.");
>>         *errors += error;
>> @@ -305,8 +292,6 @@ static int s3_check_log(
>>
>>         s3_scan_times(fw, klog, suspend_too_long, resume_too_long);
>>
>> -       fwts_klog_free(klog);
>> -
>>         return FWTS_OK;
>>   }
>>
>> @@ -329,17 +314,30 @@ static int s3_test_multiple(fwts_framework *fw)
>>         for (i=0; i<s3_multiple; i++) {
>>                 struct timeval tv;
>>                 int percent = (i * 100) / s3_multiple;
>> -
>> +               fwts_list *klog_pre, *klog_post, *klog_diff;
>>                 fwts_log_info(fw, "S3 cycle %d of %d\n",i+1,s3_multiple);
>>
>> +               if ((klog_pre = fwts_klog_read()) == NULL)
>> +                       fwts_log_error(fw, "Cannot read kernel log.");
>> +
>>                 if (s3_do_suspend_resume(fw, &hw_errors, &pm_errors,
>> s3_sleep_delay, percent) == FWTS_OUT_OF_MEMORY) {
>>                         fwts_log_error(fw, "S3 cycle %d failed - out of
>> memory error.", i+1);
>> +                       fwts_klog_free(klog_pre);
>>                         break;
>>                 }
>> +
>> +               if ((klog_post = fwts_klog_read()) == NULL)
>> +                       fwts_log_error(fw, "Cannot re-read kernel log.");
>> +
>>                 fwts_progress_message(fw, percent, "(Checking logs for
>> errors)");
>> -               s3_check_log(fw, &klog_errors, &klog_oopses,
>> &klog_warn_ons,
>> +               klog_diff = fwts_klog_find_changes(klog_pre, klog_post);
>> +               s3_check_log(fw, klog_diff, &klog_errors, &klog_oopses,
>> &klog_warn_ons,
>>                         &suspend_too_long, &resume_too_long);
>>
>> +               fwts_klog_free(klog_pre);
>> +               fwts_klog_free(klog_post);
>> +               fwts_list_free(klog_diff, NULL);
>> +
>>                 if (!s3_device_check) {
>>                         char buffer[80];
>>                         int i;
>>
>
> Acked-by: Ivan Hu <ivan.hu@canonical.com>
>

Acked-by: Keng-Yu Lin <kengyu@canonical.com>


[prev in list] [next in list] [prev in thread] [next in thread]