All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] intel-gpu-tools: remove the --cmd option from gem_seqno_wrap
@ 2014-06-16  8:38 tim.gore
  2014-06-16 12:32 ` Mika Kuoppala
  0 siblings, 1 reply; 3+ messages in thread
From: tim.gore @ 2014-06-16  8:38 UTC (permalink / raw)
  To: intel-gfx

From: Tim Gore <tim.gore@intel.com>

gem_seqno_wrap was not being built on Android because it uses
wordexp which is not in Bionic.
After discussion with Mika Kuoppala (the test author) it seems
that wordexp was used to implement the --cmd option that was
really only intended for use during development of the test and
is no longer needed. So I have removed support for this option
and enabled this test for Android.

Signed-off-by: Tim Gore <tim.gore@intel.com>
---
 tests/Android.mk       |  1 -
 tests/gem_seqno_wrap.c | 77 ++------------------------------------------------
 2 files changed, 3 insertions(+), 75 deletions(-)

diff --git a/tests/Android.mk b/tests/Android.mk
index 9db6625..f085d35 100644
--- a/tests/Android.mk
+++ b/tests/Android.mk
@@ -28,7 +28,6 @@ endef
 
 # some tests still do not build under android
 skip_tests_list :=
-skip_tests_list += gem_seqno_wrap
 skip_tests_list += testdisplay        # needs glib.h
 skip_tests_list += pm_rpm
 
diff --git a/tests/gem_seqno_wrap.c b/tests/gem_seqno_wrap.c
index fa38f1f..ad4f839 100644
--- a/tests/gem_seqno_wrap.c
+++ b/tests/gem_seqno_wrap.c
@@ -38,7 +38,6 @@
 #include <sys/types.h>
 #include <sys/wait.h>
 #include <limits.h>
-#include <wordexp.h>
 #include <getopt.h>
 #include <signal.h>
 #include <errno.h>
@@ -62,7 +61,6 @@ static struct intel_batchbuffer *batch_3d;
 struct option_struct {
 	int rounds;
 	int background;
-	char cmd[1024];
 	int timeout;
 	int dontwrap;
 	int prewrap_space;
@@ -281,64 +279,6 @@ static void run_sync_test(int num_buffers, bool verify)
 	close(fd);
 }
 
-static int run_cmd(char *s)
-{
-	int pid;
-	int r = -1;
-	int status = 0;
-	wordexp_t wexp;
-	int i;
-	r = wordexp(s, &wexp, 0);
-	if (r != 0) {
-		printf("can't parse %s\n", s);
-		return r;
-	}
-
-	for(i = 0; i < wexp.we_wordc; i++)
-		printf("argv[%d] = %s\n", i, wexp.we_wordv[i]);
-
-	pid = fork();
-
-	if (pid == 0) {
-		char path[PATH_MAX];
-		char full_path[PATH_MAX];
-
-		if (getcwd(path, PATH_MAX) == NULL)
-			perror("getcwd");
-
-		igt_assert(snprintf(full_path, PATH_MAX, "%s/%s", path, wexp.we_wordv[0]) > 0);
-
-		r = execv(full_path, wexp.we_wordv);
-		if (r == -1)
-			perror("execv failed");
-	} else {
-		int waitcount = options.timeout;
-
-		while(waitcount-- > 0) {
-			r = waitpid(pid, &status, WNOHANG);
-			if (r == pid) {
-				if(WIFEXITED(status)) {
-					if (WEXITSTATUS(status))
-						fprintf(stderr,
-						    "child returned with %d\n",
-							WEXITSTATUS(status));
-					return WEXITSTATUS(status);
-				}
-			} else if (r != 0) {
-				perror("waitpid");
-				return -errno;
-			}
-
-			sleep(3);
-		}
-
-		kill(pid, SIGKILL);
-		return -ETIMEDOUT;
-	}
-
-	return r;
-}
-
 static const char *dfs_base = "/sys/kernel/debug/dri";
 static const char *dfs_entry = "i915_next_seqno";
 
@@ -415,7 +355,7 @@ static int write_seqno(uint32_t seqno)
 	int fh;
 	char buf[32];
 	int r;
-	uint32_t rb;
+	uint32_t rb = -1;
 
 	if (options.dontwrap)
 		return 0;
@@ -461,10 +401,7 @@ static uint32_t calc_prewrap_val(void)
 
 static void run_test(void)
 {
-	if (strnlen(options.cmd, sizeof(options.cmd)) > 0)
-		igt_assert(run_cmd(options.cmd) == 0);
-	else
-		run_sync_test(options.buffers, true);
+	run_sync_test(options.buffers, true);
 }
 
 static void preset_run_once(void)
@@ -521,7 +458,6 @@ static void print_usage(const char *s)
 	printf("%s: [OPTION]...\n", s);
 	printf("    where options are:\n");
 	printf("    -b --background       run in background inducing wraps\n");
-	printf("    -c --cmd=cmdstring    use cmdstring to cross wrap\n");
 	printf("    -n --rounds=num       run num times across wrap boundary, 0 == forever\n");
 	printf("    -t --timeout=sec      set timeout to wait for testrun to sec seconds\n");
 	printf("    -d --dontwrap         don't wrap just run the test\n");
@@ -536,7 +472,6 @@ static void parse_options(int argc, char **argv)
 	int c;
 	int option_index = 0;
 	static struct option long_options[] = {
-		{"cmd", required_argument, 0, 'c'},
 		{"rounds", required_argument, 0, 'n'},
 		{"background", no_argument, 0, 'b'},
 		{"timeout", required_argument, 0, 't'},
@@ -546,7 +481,6 @@ static void parse_options(int argc, char **argv)
 		{"buffers", required_argument, 0, 'i'},
 	};
 
-	strcpy(options.cmd, "");
 	options.rounds = SLOW_QUICK(50, 2);
 	options.background = 0;
 	options.dontwrap = 0;
@@ -555,7 +489,7 @@ static void parse_options(int argc, char **argv)
 	options.prewrap_space = 21;
 	options.buffers = 10;
 
-	while((c = getopt_long(argc, argv, "c:n:bvt:dp:ri:",
+	while((c = getopt_long(argc, argv, "n:bvt:dp:ri:",
 			       long_options, &option_index)) != -1) {
 		switch(c) {
 		case 'b':
@@ -570,11 +504,6 @@ static void parse_options(int argc, char **argv)
 			options.rounds = atoi(optarg);
 			printf("running %d rounds\n", options.rounds);
 			break;
-		case 'c':
-			strncpy(options.cmd, optarg, sizeof(options.cmd) - 1);
-			options.cmd[sizeof(options.cmd) - 1] = 0;
-			printf("cmd set to %s\n", options.cmd);
-			break;
 		case 'i':
 			options.buffers = atoi(optarg);
 			printf("buffers %d\n", options.buffers);
-- 
1.9.2

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

* Re: [PATCH] intel-gpu-tools: remove the --cmd option from gem_seqno_wrap
  2014-06-16  8:38 [PATCH] intel-gpu-tools: remove the --cmd option from gem_seqno_wrap tim.gore
@ 2014-06-16 12:32 ` Mika Kuoppala
  2014-06-16 18:05   ` Daniel Vetter
  0 siblings, 1 reply; 3+ messages in thread
From: Mika Kuoppala @ 2014-06-16 12:32 UTC (permalink / raw)
  To: tim.gore, intel-gfx

tim.gore@intel.com writes:

> From: Tim Gore <tim.gore@intel.com>
>
> gem_seqno_wrap was not being built on Android because it uses
> wordexp which is not in Bionic.
> After discussion with Mika Kuoppala (the test author) it seems
> that wordexp was used to implement the --cmd option that was
> really only intended for use during development of the test and
> is no longer needed. So I have removed support for this option
> and enabled this test for Android.
>
> Signed-off-by: Tim Gore <tim.gore@intel.com>

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

> ---
>  tests/Android.mk       |  1 -
>  tests/gem_seqno_wrap.c | 77 ++------------------------------------------------
>  2 files changed, 3 insertions(+), 75 deletions(-)
>
> diff --git a/tests/Android.mk b/tests/Android.mk
> index 9db6625..f085d35 100644
> --- a/tests/Android.mk
> +++ b/tests/Android.mk
> @@ -28,7 +28,6 @@ endef
>  
>  # some tests still do not build under android
>  skip_tests_list :=
> -skip_tests_list += gem_seqno_wrap
>  skip_tests_list += testdisplay        # needs glib.h
>  skip_tests_list += pm_rpm
>  
> diff --git a/tests/gem_seqno_wrap.c b/tests/gem_seqno_wrap.c
> index fa38f1f..ad4f839 100644
> --- a/tests/gem_seqno_wrap.c
> +++ b/tests/gem_seqno_wrap.c
> @@ -38,7 +38,6 @@
>  #include <sys/types.h>
>  #include <sys/wait.h>
>  #include <limits.h>
> -#include <wordexp.h>
>  #include <getopt.h>
>  #include <signal.h>
>  #include <errno.h>
> @@ -62,7 +61,6 @@ static struct intel_batchbuffer *batch_3d;
>  struct option_struct {
>  	int rounds;
>  	int background;
> -	char cmd[1024];
>  	int timeout;
>  	int dontwrap;
>  	int prewrap_space;
> @@ -281,64 +279,6 @@ static void run_sync_test(int num_buffers, bool verify)
>  	close(fd);
>  }
>  
> -static int run_cmd(char *s)
> -{
> -	int pid;
> -	int r = -1;
> -	int status = 0;
> -	wordexp_t wexp;
> -	int i;
> -	r = wordexp(s, &wexp, 0);
> -	if (r != 0) {
> -		printf("can't parse %s\n", s);
> -		return r;
> -	}
> -
> -	for(i = 0; i < wexp.we_wordc; i++)
> -		printf("argv[%d] = %s\n", i, wexp.we_wordv[i]);
> -
> -	pid = fork();
> -
> -	if (pid == 0) {
> -		char path[PATH_MAX];
> -		char full_path[PATH_MAX];
> -
> -		if (getcwd(path, PATH_MAX) == NULL)
> -			perror("getcwd");
> -
> -		igt_assert(snprintf(full_path, PATH_MAX, "%s/%s", path, wexp.we_wordv[0]) > 0);
> -
> -		r = execv(full_path, wexp.we_wordv);
> -		if (r == -1)
> -			perror("execv failed");
> -	} else {
> -		int waitcount = options.timeout;
> -
> -		while(waitcount-- > 0) {
> -			r = waitpid(pid, &status, WNOHANG);
> -			if (r == pid) {
> -				if(WIFEXITED(status)) {
> -					if (WEXITSTATUS(status))
> -						fprintf(stderr,
> -						    "child returned with %d\n",
> -							WEXITSTATUS(status));
> -					return WEXITSTATUS(status);
> -				}
> -			} else if (r != 0) {
> -				perror("waitpid");
> -				return -errno;
> -			}
> -
> -			sleep(3);
> -		}
> -
> -		kill(pid, SIGKILL);
> -		return -ETIMEDOUT;
> -	}
> -
> -	return r;
> -}
> -
>  static const char *dfs_base = "/sys/kernel/debug/dri";
>  static const char *dfs_entry = "i915_next_seqno";
>  
> @@ -415,7 +355,7 @@ static int write_seqno(uint32_t seqno)
>  	int fh;
>  	char buf[32];
>  	int r;
> -	uint32_t rb;
> +	uint32_t rb = -1;
>  
>  	if (options.dontwrap)
>  		return 0;
> @@ -461,10 +401,7 @@ static uint32_t calc_prewrap_val(void)
>  
>  static void run_test(void)
>  {
> -	if (strnlen(options.cmd, sizeof(options.cmd)) > 0)
> -		igt_assert(run_cmd(options.cmd) == 0);
> -	else
> -		run_sync_test(options.buffers, true);
> +	run_sync_test(options.buffers, true);
>  }
>  
>  static void preset_run_once(void)
> @@ -521,7 +458,6 @@ static void print_usage(const char *s)
>  	printf("%s: [OPTION]...\n", s);
>  	printf("    where options are:\n");
>  	printf("    -b --background       run in background inducing wraps\n");
> -	printf("    -c --cmd=cmdstring    use cmdstring to cross wrap\n");
>  	printf("    -n --rounds=num       run num times across wrap boundary, 0 == forever\n");
>  	printf("    -t --timeout=sec      set timeout to wait for testrun to sec seconds\n");
>  	printf("    -d --dontwrap         don't wrap just run the test\n");
> @@ -536,7 +472,6 @@ static void parse_options(int argc, char **argv)
>  	int c;
>  	int option_index = 0;
>  	static struct option long_options[] = {
> -		{"cmd", required_argument, 0, 'c'},
>  		{"rounds", required_argument, 0, 'n'},
>  		{"background", no_argument, 0, 'b'},
>  		{"timeout", required_argument, 0, 't'},
> @@ -546,7 +481,6 @@ static void parse_options(int argc, char **argv)
>  		{"buffers", required_argument, 0, 'i'},
>  	};
>  
> -	strcpy(options.cmd, "");
>  	options.rounds = SLOW_QUICK(50, 2);
>  	options.background = 0;
>  	options.dontwrap = 0;
> @@ -555,7 +489,7 @@ static void parse_options(int argc, char **argv)
>  	options.prewrap_space = 21;
>  	options.buffers = 10;
>  
> -	while((c = getopt_long(argc, argv, "c:n:bvt:dp:ri:",
> +	while((c = getopt_long(argc, argv, "n:bvt:dp:ri:",
>  			       long_options, &option_index)) != -1) {
>  		switch(c) {
>  		case 'b':
> @@ -570,11 +504,6 @@ static void parse_options(int argc, char **argv)
>  			options.rounds = atoi(optarg);
>  			printf("running %d rounds\n", options.rounds);
>  			break;
> -		case 'c':
> -			strncpy(options.cmd, optarg, sizeof(options.cmd) - 1);
> -			options.cmd[sizeof(options.cmd) - 1] = 0;
> -			printf("cmd set to %s\n", options.cmd);
> -			break;
>  		case 'i':
>  			options.buffers = atoi(optarg);
>  			printf("buffers %d\n", options.buffers);
> -- 
> 1.9.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] intel-gpu-tools: remove the --cmd option from gem_seqno_wrap
  2014-06-16 12:32 ` Mika Kuoppala
@ 2014-06-16 18:05   ` Daniel Vetter
  0 siblings, 0 replies; 3+ messages in thread
From: Daniel Vetter @ 2014-06-16 18:05 UTC (permalink / raw)
  To: Mika Kuoppala; +Cc: intel-gfx

On Mon, Jun 16, 2014 at 03:32:35PM +0300, Mika Kuoppala wrote:
> tim.gore@intel.com writes:
> 
> > From: Tim Gore <tim.gore@intel.com>
> >
> > gem_seqno_wrap was not being built on Android because it uses
> > wordexp which is not in Bionic.
> > After discussion with Mika Kuoppala (the test author) it seems
> > that wordexp was used to implement the --cmd option that was
> > really only intended for use during development of the test and
> > is no longer needed. So I have removed support for this option
> > and enabled this test for Android.
> >
> > Signed-off-by: Tim Gore <tim.gore@intel.com>
> 
> Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

Merged, thanks.
-Daniel
> 
> > ---
> >  tests/Android.mk       |  1 -
> >  tests/gem_seqno_wrap.c | 77 ++------------------------------------------------
> >  2 files changed, 3 insertions(+), 75 deletions(-)
> >
> > diff --git a/tests/Android.mk b/tests/Android.mk
> > index 9db6625..f085d35 100644
> > --- a/tests/Android.mk
> > +++ b/tests/Android.mk
> > @@ -28,7 +28,6 @@ endef
> >  
> >  # some tests still do not build under android
> >  skip_tests_list :=
> > -skip_tests_list += gem_seqno_wrap
> >  skip_tests_list += testdisplay        # needs glib.h
> >  skip_tests_list += pm_rpm
> >  
> > diff --git a/tests/gem_seqno_wrap.c b/tests/gem_seqno_wrap.c
> > index fa38f1f..ad4f839 100644
> > --- a/tests/gem_seqno_wrap.c
> > +++ b/tests/gem_seqno_wrap.c
> > @@ -38,7 +38,6 @@
> >  #include <sys/types.h>
> >  #include <sys/wait.h>
> >  #include <limits.h>
> > -#include <wordexp.h>
> >  #include <getopt.h>
> >  #include <signal.h>
> >  #include <errno.h>
> > @@ -62,7 +61,6 @@ static struct intel_batchbuffer *batch_3d;
> >  struct option_struct {
> >  	int rounds;
> >  	int background;
> > -	char cmd[1024];
> >  	int timeout;
> >  	int dontwrap;
> >  	int prewrap_space;
> > @@ -281,64 +279,6 @@ static void run_sync_test(int num_buffers, bool verify)
> >  	close(fd);
> >  }
> >  
> > -static int run_cmd(char *s)
> > -{
> > -	int pid;
> > -	int r = -1;
> > -	int status = 0;
> > -	wordexp_t wexp;
> > -	int i;
> > -	r = wordexp(s, &wexp, 0);
> > -	if (r != 0) {
> > -		printf("can't parse %s\n", s);
> > -		return r;
> > -	}
> > -
> > -	for(i = 0; i < wexp.we_wordc; i++)
> > -		printf("argv[%d] = %s\n", i, wexp.we_wordv[i]);
> > -
> > -	pid = fork();
> > -
> > -	if (pid == 0) {
> > -		char path[PATH_MAX];
> > -		char full_path[PATH_MAX];
> > -
> > -		if (getcwd(path, PATH_MAX) == NULL)
> > -			perror("getcwd");
> > -
> > -		igt_assert(snprintf(full_path, PATH_MAX, "%s/%s", path, wexp.we_wordv[0]) > 0);
> > -
> > -		r = execv(full_path, wexp.we_wordv);
> > -		if (r == -1)
> > -			perror("execv failed");
> > -	} else {
> > -		int waitcount = options.timeout;
> > -
> > -		while(waitcount-- > 0) {
> > -			r = waitpid(pid, &status, WNOHANG);
> > -			if (r == pid) {
> > -				if(WIFEXITED(status)) {
> > -					if (WEXITSTATUS(status))
> > -						fprintf(stderr,
> > -						    "child returned with %d\n",
> > -							WEXITSTATUS(status));
> > -					return WEXITSTATUS(status);
> > -				}
> > -			} else if (r != 0) {
> > -				perror("waitpid");
> > -				return -errno;
> > -			}
> > -
> > -			sleep(3);
> > -		}
> > -
> > -		kill(pid, SIGKILL);
> > -		return -ETIMEDOUT;
> > -	}
> > -
> > -	return r;
> > -}
> > -
> >  static const char *dfs_base = "/sys/kernel/debug/dri";
> >  static const char *dfs_entry = "i915_next_seqno";
> >  
> > @@ -415,7 +355,7 @@ static int write_seqno(uint32_t seqno)
> >  	int fh;
> >  	char buf[32];
> >  	int r;
> > -	uint32_t rb;
> > +	uint32_t rb = -1;
> >  
> >  	if (options.dontwrap)
> >  		return 0;
> > @@ -461,10 +401,7 @@ static uint32_t calc_prewrap_val(void)
> >  
> >  static void run_test(void)
> >  {
> > -	if (strnlen(options.cmd, sizeof(options.cmd)) > 0)
> > -		igt_assert(run_cmd(options.cmd) == 0);
> > -	else
> > -		run_sync_test(options.buffers, true);
> > +	run_sync_test(options.buffers, true);
> >  }
> >  
> >  static void preset_run_once(void)
> > @@ -521,7 +458,6 @@ static void print_usage(const char *s)
> >  	printf("%s: [OPTION]...\n", s);
> >  	printf("    where options are:\n");
> >  	printf("    -b --background       run in background inducing wraps\n");
> > -	printf("    -c --cmd=cmdstring    use cmdstring to cross wrap\n");
> >  	printf("    -n --rounds=num       run num times across wrap boundary, 0 == forever\n");
> >  	printf("    -t --timeout=sec      set timeout to wait for testrun to sec seconds\n");
> >  	printf("    -d --dontwrap         don't wrap just run the test\n");
> > @@ -536,7 +472,6 @@ static void parse_options(int argc, char **argv)
> >  	int c;
> >  	int option_index = 0;
> >  	static struct option long_options[] = {
> > -		{"cmd", required_argument, 0, 'c'},
> >  		{"rounds", required_argument, 0, 'n'},
> >  		{"background", no_argument, 0, 'b'},
> >  		{"timeout", required_argument, 0, 't'},
> > @@ -546,7 +481,6 @@ static void parse_options(int argc, char **argv)
> >  		{"buffers", required_argument, 0, 'i'},
> >  	};
> >  
> > -	strcpy(options.cmd, "");
> >  	options.rounds = SLOW_QUICK(50, 2);
> >  	options.background = 0;
> >  	options.dontwrap = 0;
> > @@ -555,7 +489,7 @@ static void parse_options(int argc, char **argv)
> >  	options.prewrap_space = 21;
> >  	options.buffers = 10;
> >  
> > -	while((c = getopt_long(argc, argv, "c:n:bvt:dp:ri:",
> > +	while((c = getopt_long(argc, argv, "n:bvt:dp:ri:",
> >  			       long_options, &option_index)) != -1) {
> >  		switch(c) {
> >  		case 'b':
> > @@ -570,11 +504,6 @@ static void parse_options(int argc, char **argv)
> >  			options.rounds = atoi(optarg);
> >  			printf("running %d rounds\n", options.rounds);
> >  			break;
> > -		case 'c':
> > -			strncpy(options.cmd, optarg, sizeof(options.cmd) - 1);
> > -			options.cmd[sizeof(options.cmd) - 1] = 0;
> > -			printf("cmd set to %s\n", options.cmd);
> > -			break;
> >  		case 'i':
> >  			options.buffers = atoi(optarg);
> >  			printf("buffers %d\n", options.buffers);
> > -- 
> > 1.9.2
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> 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] 3+ messages in thread

end of thread, other threads:[~2014-06-16 18:05 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-16  8:38 [PATCH] intel-gpu-tools: remove the --cmd option from gem_seqno_wrap tim.gore
2014-06-16 12:32 ` Mika Kuoppala
2014-06-16 18:05   ` Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.