public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] igt_core: Inject subtest message into dmesg
@ 2014-07-24 11:48 Chris Wilson
  2014-07-24 12:36 ` Daniel Vetter
  0 siblings, 1 reply; 5+ messages in thread
From: Chris Wilson @ 2014-07-24 11:48 UTC (permalink / raw)
  To: intel-gfx

One of the side-effects we test for are kernel oops and knowing the
guilty subtest can help speed up debugging. We can write to /dev/kmsg to
inject messages into dmesg, so let's do so before the start of every
test.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 lib/igt_core.c | 29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/lib/igt_core.c b/lib/igt_core.c
index b0800e8..deaf145 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -269,8 +269,9 @@ static void print_version(void)
 		uts.sysname, uts.release, uts.machine);
 }
 
-static void print_usage(const char *command_str, const char *help_str,
-			bool output_on_stderr)
+static const char *command_str;
+
+static void print_usage(const char *help_str, bool output_on_stderr)
 {
 	FILE *f = output_on_stderr ? stderr : stdout;
 
@@ -306,7 +307,6 @@ static int common_init(int argc, char **argv,
 		{"debug", 0, 0, 'd'},
 		{"help", 0, 0, 'h'},
 	};
-	const char *command_str;
 	char *short_opts;
 	struct option *combined_opts;
 	int extra_opt_count;
@@ -364,11 +364,11 @@ static int common_init(int argc, char **argv,
 				run_single_subtest = strdup(optarg);
 			break;
 		case 'h':
-			print_usage(command_str, help_str, false);
+			print_usage(help_str, false);
 			ret = -1;
 			goto out;
 		case '?':
-			print_usage(command_str, help_str, true);
+			print_usage(help_str, true);
 			ret = -2;
 			goto out;
 		default:
@@ -498,6 +498,23 @@ void igt_simple_init_parse_opts(int argc, char **argv,
 		    extra_opt_handler);
 }
 
+__attribute__((format(printf, 2, 3)))
+static void echo(const char *path, const char *format, ...)
+{
+	va_list ap;
+	FILE *file;
+
+	file = fopen(path, "w");
+	if (file == NULL)
+		return;
+
+	va_start(ap, format);
+	vfprintf(file, format, ap);
+	va_end(ap);
+
+	fclose(file);
+}
+
 /*
  * Note: Testcases which use these helpers MUST NOT output anything to stdout
  * outside of places protected by igt_run_subtest checks - the piglit
@@ -529,6 +546,8 @@ bool __igt_run_subtest(const char *subtest_name)
 		return false;
 	}
 
+	echo("/dev/kmsg", "%s: starting subtest %s\n", command_str, subtest_name);
+
 	return (in_subtest = subtest_name);
 }
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] igt_core: Inject subtest message into dmesg
  2014-07-24 11:48 [PATCH] igt_core: Inject subtest message into dmesg Chris Wilson
@ 2014-07-24 12:36 ` Daniel Vetter
  2014-07-24 13:03   ` Thomas Wood
  0 siblings, 1 reply; 5+ messages in thread
From: Daniel Vetter @ 2014-07-24 12:36 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Thu, Jul 24, 2014 at 12:48:33PM +0100, Chris Wilson wrote:
> One of the side-effects we test for are kernel oops and knowing the
> guilty subtest can help speed up debugging. We can write to /dev/kmsg to
> inject messages into dmesg, so let's do so before the start of every
> test.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Should we change proc->comm too? Would help with the oops printing ... Ack
on the patch itself.
-Daniel
> ---
>  lib/igt_core.c | 29 ++++++++++++++++++++++++-----
>  1 file changed, 24 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index b0800e8..deaf145 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -269,8 +269,9 @@ static void print_version(void)
>  		uts.sysname, uts.release, uts.machine);
>  }
>  
> -static void print_usage(const char *command_str, const char *help_str,
> -			bool output_on_stderr)
> +static const char *command_str;
> +
> +static void print_usage(const char *help_str, bool output_on_stderr)
>  {
>  	FILE *f = output_on_stderr ? stderr : stdout;
>  
> @@ -306,7 +307,6 @@ static int common_init(int argc, char **argv,
>  		{"debug", 0, 0, 'd'},
>  		{"help", 0, 0, 'h'},
>  	};
> -	const char *command_str;
>  	char *short_opts;
>  	struct option *combined_opts;
>  	int extra_opt_count;
> @@ -364,11 +364,11 @@ static int common_init(int argc, char **argv,
>  				run_single_subtest = strdup(optarg);
>  			break;
>  		case 'h':
> -			print_usage(command_str, help_str, false);
> +			print_usage(help_str, false);
>  			ret = -1;
>  			goto out;
>  		case '?':
> -			print_usage(command_str, help_str, true);
> +			print_usage(help_str, true);
>  			ret = -2;
>  			goto out;
>  		default:
> @@ -498,6 +498,23 @@ void igt_simple_init_parse_opts(int argc, char **argv,
>  		    extra_opt_handler);
>  }
>  
> +__attribute__((format(printf, 2, 3)))
> +static void echo(const char *path, const char *format, ...)
> +{
> +	va_list ap;
> +	FILE *file;
> +
> +	file = fopen(path, "w");
> +	if (file == NULL)
> +		return;
> +
> +	va_start(ap, format);
> +	vfprintf(file, format, ap);
> +	va_end(ap);
> +
> +	fclose(file);
> +}
> +
>  /*
>   * Note: Testcases which use these helpers MUST NOT output anything to stdout
>   * outside of places protected by igt_run_subtest checks - the piglit
> @@ -529,6 +546,8 @@ bool __igt_run_subtest(const char *subtest_name)
>  		return false;
>  	}
>  
> +	echo("/dev/kmsg", "%s: starting subtest %s\n", command_str, subtest_name);
> +
>  	return (in_subtest = subtest_name);
>  }
>  
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] igt_core: Inject subtest message into dmesg
  2014-07-24 12:36 ` Daniel Vetter
@ 2014-07-24 13:03   ` Thomas Wood
  2014-07-24 13:58     ` Chris Wilson
  0 siblings, 1 reply; 5+ messages in thread
From: Thomas Wood @ 2014-07-24 13:03 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Intel Graphics Development

On 24 July 2014 13:36, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Thu, Jul 24, 2014 at 12:48:33PM +0100, Chris Wilson wrote:
>> One of the side-effects we test for are kernel oops and knowing the
>> guilty subtest can help speed up debugging. We can write to /dev/kmsg to
>> inject messages into dmesg, so let's do so before the start of every
>> test.
>>
>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>
> Should we change proc->comm too? Would help with the oops printing ... Ack
> on the patch itself.

It should also make sure that the log level is appropriate so that the
Piglit dmesg capture isn't triggered accidentally.

Would it be useful to include this in simple tests (tests without
subtests) as well?


> -Daniel
>> ---
>>  lib/igt_core.c | 29 ++++++++++++++++++++++++-----
>>  1 file changed, 24 insertions(+), 5 deletions(-)
>>
>> diff --git a/lib/igt_core.c b/lib/igt_core.c
>> index b0800e8..deaf145 100644
>> --- a/lib/igt_core.c
>> +++ b/lib/igt_core.c
>> @@ -269,8 +269,9 @@ static void print_version(void)
>>               uts.sysname, uts.release, uts.machine);
>>  }
>>
>> -static void print_usage(const char *command_str, const char *help_str,
>> -                     bool output_on_stderr)
>> +static const char *command_str;
>> +
>> +static void print_usage(const char *help_str, bool output_on_stderr)
>>  {
>>       FILE *f = output_on_stderr ? stderr : stdout;
>>
>> @@ -306,7 +307,6 @@ static int common_init(int argc, char **argv,
>>               {"debug", 0, 0, 'd'},
>>               {"help", 0, 0, 'h'},
>>       };
>> -     const char *command_str;
>>       char *short_opts;
>>       struct option *combined_opts;
>>       int extra_opt_count;
>> @@ -364,11 +364,11 @@ static int common_init(int argc, char **argv,
>>                               run_single_subtest = strdup(optarg);
>>                       break;
>>               case 'h':
>> -                     print_usage(command_str, help_str, false);
>> +                     print_usage(help_str, false);
>>                       ret = -1;
>>                       goto out;
>>               case '?':
>> -                     print_usage(command_str, help_str, true);
>> +                     print_usage(help_str, true);
>>                       ret = -2;
>>                       goto out;
>>               default:
>> @@ -498,6 +498,23 @@ void igt_simple_init_parse_opts(int argc, char **argv,
>>                   extra_opt_handler);
>>  }
>>
>> +__attribute__((format(printf, 2, 3)))
>> +static void echo(const char *path, const char *format, ...)
>> +{
>> +     va_list ap;
>> +     FILE *file;
>> +
>> +     file = fopen(path, "w");
>> +     if (file == NULL)
>> +             return;
>> +
>> +     va_start(ap, format);
>> +     vfprintf(file, format, ap);
>> +     va_end(ap);
>> +
>> +     fclose(file);
>> +}
>> +
>>  /*
>>   * Note: Testcases which use these helpers MUST NOT output anything to stdout
>>   * outside of places protected by igt_run_subtest checks - the piglit
>> @@ -529,6 +546,8 @@ bool __igt_run_subtest(const char *subtest_name)
>>               return false;
>>       }
>>
>> +     echo("/dev/kmsg", "%s: starting subtest %s\n", command_str, subtest_name);
>> +
>>       return (in_subtest = subtest_name);
>>  }
>>
>> --
>> 1.9.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] igt_core: Inject subtest message into dmesg
  2014-07-24 13:03   ` Thomas Wood
@ 2014-07-24 13:58     ` Chris Wilson
  2014-07-24 14:02       ` Daniel Vetter
  0 siblings, 1 reply; 5+ messages in thread
From: Chris Wilson @ 2014-07-24 13:58 UTC (permalink / raw)
  To: Thomas Wood; +Cc: Intel Graphics Development

On Thu, Jul 24, 2014 at 02:03:01PM +0100, Thomas Wood wrote:
> On 24 July 2014 13:36, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Thu, Jul 24, 2014 at 12:48:33PM +0100, Chris Wilson wrote:
> >> One of the side-effects we test for are kernel oops and knowing the
> >> guilty subtest can help speed up debugging. We can write to /dev/kmsg to
> >> inject messages into dmesg, so let's do so before the start of every
> >> test.
> >>
> >> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >
> > Should we change proc->comm too? Would help with the oops printing ... Ack
> > on the patch itself.
> 
> It should also make sure that the log level is appropriate so that the
> Piglit dmesg capture isn't triggered accidentally.

I found out how to set the log-level, so choose KERN_INFO which should be
sufficient.
 
> Would it be useful to include this in simple tests (tests without
> subtests) as well?

Definitely. Is there a way to do that automatically or do we need to
adjust the tests themselves? I was thinkg we could add a kmsg() to
common_init() to capture those.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] igt_core: Inject subtest message into dmesg
  2014-07-24 13:58     ` Chris Wilson
@ 2014-07-24 14:02       ` Daniel Vetter
  0 siblings, 0 replies; 5+ messages in thread
From: Daniel Vetter @ 2014-07-24 14:02 UTC (permalink / raw)
  To: Chris Wilson, Thomas Wood, Daniel Vetter,
	Intel Graphics Development

On Thu, Jul 24, 2014 at 02:58:08PM +0100, Chris Wilson wrote:
> On Thu, Jul 24, 2014 at 02:03:01PM +0100, Thomas Wood wrote:
> > On 24 July 2014 13:36, Daniel Vetter <daniel@ffwll.ch> wrote:
> > > On Thu, Jul 24, 2014 at 12:48:33PM +0100, Chris Wilson wrote:
> > >> One of the side-effects we test for are kernel oops and knowing the
> > >> guilty subtest can help speed up debugging. We can write to /dev/kmsg to
> > >> inject messages into dmesg, so let's do so before the start of every
> > >> test.
> > >>
> > >> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > >
> > > Should we change proc->comm too? Would help with the oops printing ... Ack
> > > on the patch itself.
> > 
> > It should also make sure that the log level is appropriate so that the
> > Piglit dmesg capture isn't triggered accidentally.
> 
> I found out how to set the log-level, so choose KERN_INFO which should be
> sufficient.
>  
> > Would it be useful to include this in simple tests (tests without
> > subtests) as well?
> 
> Definitely. Is there a way to do that automatically or do we need to
> adjust the tests themselves? I was thinkg we could add a kmsg() to
> common_init() to capture those.

common_init kmsg'ing the test binary should be good. That should help in
lining up any test setup work that's done before the first test starts,
too. So useful even with subtests.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2014-07-24 14:02 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-07-24 11:48 [PATCH] igt_core: Inject subtest message into dmesg Chris Wilson
2014-07-24 12:36 ` Daniel Vetter
2014-07-24 13:03   ` Thomas Wood
2014-07-24 13:58     ` Chris Wilson
2014-07-24 14:02       ` Daniel Vetter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox