diff mbox

[2/3] acpi: s3: do kernel log comparisons without destroying klog (LP: #1262208)

Message ID 1387376358-24508-3-git-send-email-colin.king@canonical.com
State Accepted
Headers show

Commit Message

Colin Ian King Dec. 18, 2013, 2:19 p.m. UTC 
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(-)

Comments

Ivan Hu Dec. 23, 2013, 8:09 a.m. UTC | #1 
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>
Keng-Yu Lin Dec. 24, 2013, 9:42 a.m. UTC | #2 
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>
diff mbox

Patch

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;