intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH igt] gem_concurrent_blit: Don't call igt_require() outside of a subtest/fixture
@ 2016-01-08  9:10 Chris Wilson
  2016-01-11  8:00 ` Daniel Vetter
  0 siblings, 1 reply; 4+ messages in thread
From: Chris Wilson @ 2016-01-08  9:10 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter, Thomas Wood

gem_concurrent_blit tries to ensure that it doesn't try and run a test
that would grind the system to a halt, i.e. unexpectedly cause swap
thrashing. It currently calls intel_require_memory(), but outside of
the subtest (as the tests use fork, it cannot do requirement testing
within the test children) - but intel_require_memory() calls
igt_require() and triggers and abort. Wrapping that initial require
within an igt_fixture() stops the abort(), but also prevents any further
testing.

This patch restructures the requirement checking to ordinary conditions,
which though allowing the test to run, also prevents listing of subtests
on machines which cannot handle them.
---
 lib/igt_aux.h              |  2 ++
 lib/intel_os.c             | 53 +++++++++++++++++++++++-------------
 tests/gem_concurrent_all.c | 67 +++++++++++++++++++++++++++++++++-------------
 3 files changed, 85 insertions(+), 37 deletions(-)

diff --git a/lib/igt_aux.h b/lib/igt_aux.h
index 6e11ee6..5a88c2a 100644
--- a/lib/igt_aux.h
+++ b/lib/igt_aux.h
@@ -88,6 +88,8 @@ uint64_t intel_get_total_swap_mb(void);
 
 #define CHECK_RAM 0x1
 #define CHECK_SWAP 0x2
+int __intel_check_memory(uint32_t count, uint64_t size, unsigned mode,
+			 uint64_t *out_required, uint64_t *out_total);
 void intel_require_memory(uint32_t count, uint64_t size, unsigned mode);
 int intel_num_objects_for_memory(uint32_t size, unsigned mode);
 
diff --git a/lib/intel_os.c b/lib/intel_os.c
index dba9e17..90f9bb3 100644
--- a/lib/intel_os.c
+++ b/lib/intel_os.c
@@ -192,6 +192,38 @@ intel_get_total_swap_mb(void)
 	return retval / (1024*1024);
 }
 
+int __intel_check_memory(uint32_t count, uint64_t size, unsigned mode,
+			 uint64_t *out_required, uint64_t *out_total)
+{
+/* rough estimate of how many bytes the kernel requires to track each object */
+#define KERNEL_BO_OVERHEAD 512
+	uint64_t required, total;
+
+	required = count;
+	required *= size + KERNEL_BO_OVERHEAD;
+	required = ALIGN(required, 4096);
+
+	igt_debug("Checking %'u surfaces of size %'llu bytes (total %'llu) against %s%s\n",
+		  count, (long long)size, (long long)required,
+		  mode & (CHECK_RAM | CHECK_SWAP) ? "RAM" : "",
+		  mode & CHECK_SWAP ? " + swap": "");
+
+	total = 0;
+	if (mode & (CHECK_RAM | CHECK_SWAP))
+		total += intel_get_avail_ram_mb();
+	if (mode & CHECK_SWAP)
+		total += intel_get_total_swap_mb();
+	total *= 1024 * 1024;
+
+	if (out_required)
+		*out_required = required;
+
+	if (out_total)
+		*out_total = total;
+
+	return required < total;
+}
+
 /**
  * intel_require_memory:
  * @count: number of surfaces that will be created
@@ -217,27 +249,10 @@ intel_get_total_swap_mb(void)
  */
 void intel_require_memory(uint32_t count, uint64_t size, unsigned mode)
 {
-/* rough estimate of how many bytes the kernel requires to track each object */
-#define KERNEL_BO_OVERHEAD 512
 	uint64_t required, total;
 
-	required = count;
-	required *= size + KERNEL_BO_OVERHEAD;
-	required = ALIGN(required, 4096);
-
-	igt_debug("Checking %'u surfaces of size %'llu bytes (total %'llu) against %s%s\n",
-		  count, (long long)size, (long long)required,
-		  mode & (CHECK_RAM | CHECK_SWAP) ? "RAM" : "",
-		  mode & CHECK_SWAP ? " + swap": "");
-
-	total = 0;
-	if (mode & (CHECK_RAM | CHECK_SWAP))
-		total += intel_get_avail_ram_mb();
-	if (mode & CHECK_SWAP)
-		total += intel_get_total_swap_mb();
-	total *= 1024 * 1024;
-
-	igt_skip_on_f(total <= required,
+	igt_skip_on_f(!__intel_check_memory(count, size, mode,
+					    &required, &total),
 		      "Estimated that we need %'llu bytes for the test, but only have %'llu bytes available (%s%s)\n",
 		      (long long)required, (long long)total,
 		      mode & (CHECK_RAM | CHECK_SWAP) ? "RAM" : "",
diff --git a/tests/gem_concurrent_all.c b/tests/gem_concurrent_all.c
index 0e873c4..9a2fb6d 100644
--- a/tests/gem_concurrent_all.c
+++ b/tests/gem_concurrent_all.c
@@ -155,9 +155,9 @@ static bool can_create_stolen(void)
 static drm_intel_bo *
 (*create_func)(drm_intel_bufmgr *bufmgr, uint64_t size);
 
-static void create_cpu_require(void)
+static bool create_cpu_require(void)
 {
-	igt_require(create_func != create_stolen_bo);
+	return create_func != create_stolen_bo;
 }
 
 static drm_intel_bo *
@@ -375,7 +375,7 @@ gpu_cmp_bo(drm_intel_bo *bo, uint32_t val, int width, int height, drm_intel_bo *
 
 const struct access_mode {
 	const char *name;
-	void (*require)(void);
+	bool (*require)(void);
 	void (*set_bo)(drm_intel_bo *bo, uint32_t val, int w, int h);
 	void (*cmp_bo)(drm_intel_bo *bo, uint32_t val, int w, int h, drm_intel_bo *tmp);
 	drm_intel_bo *(*create_bo)(drm_intel_bufmgr *bufmgr, int width, int height);
@@ -1294,23 +1294,22 @@ run_basic_modes(const char *prefix,
 }
 
 static void
-run_modes(const char *style, const struct access_mode *mode)
+run_modes(const char *style, const struct access_mode *mode, unsigned allow_mem)
 {
-	if (mode->require)
-		mode->require();
+	if (mode->require && !mode->require())
+		return;
 
-	igt_debug("%s: using 2x%d buffers, each 1MiB\n", style, num_buffers);
-	intel_require_memory(2*num_buffers, 1024*1024, CHECK_RAM);
+	igt_debug("%s: using 2x%d buffers, each 1MiB\n",
+			style, num_buffers);
+	if (!__intel_check_memory(2*num_buffers, 1024*1024, allow_mem,
+				  NULL, NULL))
+		return;
 
-	if (all) {
-		run_basic_modes(style, mode, "", run_single);
-
-		igt_fork_signal_helper();
-		run_basic_modes(style, mode, "-interruptible", run_interruptible);
-		igt_stop_signal_helper();
-	}
+	run_basic_modes(style, mode, "", run_single);
 
 	igt_fork_signal_helper();
+	run_basic_modes(style, mode, "-interruptible",
+			run_interruptible);
 	run_basic_modes(style, mode, "-forked", run_forked);
 	run_basic_modes(style, mode, "-bomb", run_bomb);
 	igt_stop_signal_helper();
@@ -1328,6 +1327,8 @@ igt_main
 		{ "stolen-", create_stolen_bo, can_create_stolen },
 		{ NULL, NULL }
 	}, *c;
+	void *pinned;
+	uint64_t pin_sz;
 	int i;
 
 	igt_skip_on_simulation();
@@ -1354,7 +1355,7 @@ igt_main
 		if (c->require()) {
 			snprintf(name, sizeof(name), "%s%s", c->name, "small");
 			for (i = 0; i < ARRAY_SIZE(access_modes); i++)
-				run_modes(name, &access_modes[i]);
+				run_modes(name, &access_modes[i], CHECK_RAM);
 		}
 
 		igt_fixture {
@@ -1364,7 +1365,7 @@ igt_main
 		if (c->require()) {
 			snprintf(name, sizeof(name), "%s%s", c->name, "thrash");
 			for (i = 0; i < ARRAY_SIZE(access_modes); i++)
-				run_modes(name, &access_modes[i]);
+				run_modes(name, &access_modes[i], CHECK_RAM);
 		}
 
 		igt_fixture {
@@ -1374,7 +1375,37 @@ igt_main
 		if (c->require()) {
 			snprintf(name, sizeof(name), "%s%s", c->name, "full");
 			for (i = 0; i < ARRAY_SIZE(access_modes); i++)
-				run_modes(name, &access_modes[i]);
+				run_modes(name, &access_modes[i], CHECK_RAM);
+		}
+
+		igt_fixture {
+			num_buffers = gem_mappable_aperture_size() / (1024 * 1024);
+			pin_sz = intel_get_avail_ram_mb() - num_buffers;
+
+			igt_debug("Pinning %ld MiB\n", pin_sz);
+			pin_sz *= 1024 * 1024;
+
+			if (posix_memalign(&pinned, 4096, pin_sz) ||
+			    mlock(pinned, pin_sz) ||
+			    madvise(pinned, pin_sz, MADV_DONTFORK)) {
+				free(pinned);
+				pinned = NULL;
+			}
+			igt_require(pinned);
+		}
+
+		if (c->require()) {
+			snprintf(name, sizeof(name), "%s%s", c->name, "swap");
+			for (i = 0; i < ARRAY_SIZE(access_modes); i++)
+				run_modes(name, &access_modes[i], CHECK_RAM | CHECK_SWAP);
+		}
+
+		igt_fixture {
+			if (pinned) {
+				munlock(pinned, pin_sz);
+				free(pinned);
+				pinned = NULL;
+			}
 		}
 	}
 }
-- 
2.7.0.rc3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH igt] gem_concurrent_blit: Don't call igt_require() outside of a subtest/fixture
  2016-01-08  9:10 [PATCH igt] gem_concurrent_blit: Don't call igt_require() outside of a subtest/fixture Chris Wilson
@ 2016-01-11  8:00 ` Daniel Vetter
  2016-01-11  8:52   ` Chris Wilson
  0 siblings, 1 reply; 4+ messages in thread
From: Daniel Vetter @ 2016-01-11  8:00 UTC (permalink / raw)
  To: Chris Wilson; +Cc: Daniel Vetter, intel-gfx, Thomas Wood

On Fri, Jan 08, 2016 at 09:10:38AM +0000, Chris Wilson wrote:
> gem_concurrent_blit tries to ensure that it doesn't try and run a test
> that would grind the system to a halt, i.e. unexpectedly cause swap
> thrashing. It currently calls intel_require_memory(), but outside of
> the subtest (as the tests use fork, it cannot do requirement testing
> within the test children) - but intel_require_memory() calls
> igt_require() and triggers and abort. Wrapping that initial require
> within an igt_fixture() stops the abort(), but also prevents any further
> testing.
> 
> This patch restructures the requirement checking to ordinary conditions,
> which though allowing the test to run, also prevents listing of subtests
> on machines which cannot handle them.


> ---
>  lib/igt_aux.h              |  2 ++
>  lib/intel_os.c             | 53 +++++++++++++++++++++++-------------
>  tests/gem_concurrent_all.c | 67 +++++++++++++++++++++++++++++++++-------------
>  3 files changed, 85 insertions(+), 37 deletions(-)
> 
> diff --git a/lib/igt_aux.h b/lib/igt_aux.h
> index 6e11ee6..5a88c2a 100644
> --- a/lib/igt_aux.h
> +++ b/lib/igt_aux.h
> @@ -88,6 +88,8 @@ uint64_t intel_get_total_swap_mb(void);
>  
>  #define CHECK_RAM 0x1
>  #define CHECK_SWAP 0x2
> +int __intel_check_memory(uint32_t count, uint64_t size, unsigned mode,
> +			 uint64_t *out_required, uint64_t *out_total);
>  void intel_require_memory(uint32_t count, uint64_t size, unsigned mode);
>  int intel_num_objects_for_memory(uint32_t size, unsigned mode);
>  
> diff --git a/lib/intel_os.c b/lib/intel_os.c
> index dba9e17..90f9bb3 100644
> --- a/lib/intel_os.c
> +++ b/lib/intel_os.c
> @@ -192,6 +192,38 @@ intel_get_total_swap_mb(void)
>  	return retval / (1024*1024);
>  }
>  

Please add the usual gtkdoc boilerplate here with a mention of
intel_check_memory. Ack with that.
-Daniel

> +int __intel_check_memory(uint32_t count, uint64_t size, unsigned mode,
> +			 uint64_t *out_required, uint64_t *out_total)
> +{
> +/* rough estimate of how many bytes the kernel requires to track each object */
> +#define KERNEL_BO_OVERHEAD 512
> +	uint64_t required, total;
> +
> +	required = count;
> +	required *= size + KERNEL_BO_OVERHEAD;
> +	required = ALIGN(required, 4096);
> +
> +	igt_debug("Checking %'u surfaces of size %'llu bytes (total %'llu) against %s%s\n",
> +		  count, (long long)size, (long long)required,
> +		  mode & (CHECK_RAM | CHECK_SWAP) ? "RAM" : "",
> +		  mode & CHECK_SWAP ? " + swap": "");
> +
> +	total = 0;
> +	if (mode & (CHECK_RAM | CHECK_SWAP))
> +		total += intel_get_avail_ram_mb();
> +	if (mode & CHECK_SWAP)
> +		total += intel_get_total_swap_mb();
> +	total *= 1024 * 1024;
> +
> +	if (out_required)
> +		*out_required = required;
> +
> +	if (out_total)
> +		*out_total = total;
> +
> +	return required < total;
> +}
> +
>  /**
>   * intel_require_memory:
>   * @count: number of surfaces that will be created
> @@ -217,27 +249,10 @@ intel_get_total_swap_mb(void)
>   */
>  void intel_require_memory(uint32_t count, uint64_t size, unsigned mode)
>  {
> -/* rough estimate of how many bytes the kernel requires to track each object */
> -#define KERNEL_BO_OVERHEAD 512
>  	uint64_t required, total;
>  
> -	required = count;
> -	required *= size + KERNEL_BO_OVERHEAD;
> -	required = ALIGN(required, 4096);
> -
> -	igt_debug("Checking %'u surfaces of size %'llu bytes (total %'llu) against %s%s\n",
> -		  count, (long long)size, (long long)required,
> -		  mode & (CHECK_RAM | CHECK_SWAP) ? "RAM" : "",
> -		  mode & CHECK_SWAP ? " + swap": "");
> -
> -	total = 0;
> -	if (mode & (CHECK_RAM | CHECK_SWAP))
> -		total += intel_get_avail_ram_mb();
> -	if (mode & CHECK_SWAP)
> -		total += intel_get_total_swap_mb();
> -	total *= 1024 * 1024;
> -
> -	igt_skip_on_f(total <= required,
> +	igt_skip_on_f(!__intel_check_memory(count, size, mode,
> +					    &required, &total),
>  		      "Estimated that we need %'llu bytes for the test, but only have %'llu bytes available (%s%s)\n",
>  		      (long long)required, (long long)total,
>  		      mode & (CHECK_RAM | CHECK_SWAP) ? "RAM" : "",
> diff --git a/tests/gem_concurrent_all.c b/tests/gem_concurrent_all.c
> index 0e873c4..9a2fb6d 100644
> --- a/tests/gem_concurrent_all.c
> +++ b/tests/gem_concurrent_all.c
> @@ -155,9 +155,9 @@ static bool can_create_stolen(void)
>  static drm_intel_bo *
>  (*create_func)(drm_intel_bufmgr *bufmgr, uint64_t size);
>  
> -static void create_cpu_require(void)
> +static bool create_cpu_require(void)
>  {
> -	igt_require(create_func != create_stolen_bo);
> +	return create_func != create_stolen_bo;
>  }
>  
>  static drm_intel_bo *
> @@ -375,7 +375,7 @@ gpu_cmp_bo(drm_intel_bo *bo, uint32_t val, int width, int height, drm_intel_bo *
>  
>  const struct access_mode {
>  	const char *name;
> -	void (*require)(void);
> +	bool (*require)(void);
>  	void (*set_bo)(drm_intel_bo *bo, uint32_t val, int w, int h);
>  	void (*cmp_bo)(drm_intel_bo *bo, uint32_t val, int w, int h, drm_intel_bo *tmp);
>  	drm_intel_bo *(*create_bo)(drm_intel_bufmgr *bufmgr, int width, int height);
> @@ -1294,23 +1294,22 @@ run_basic_modes(const char *prefix,
>  }
>  
>  static void
> -run_modes(const char *style, const struct access_mode *mode)
> +run_modes(const char *style, const struct access_mode *mode, unsigned allow_mem)
>  {
> -	if (mode->require)
> -		mode->require();
> +	if (mode->require && !mode->require())
> +		return;
>  
> -	igt_debug("%s: using 2x%d buffers, each 1MiB\n", style, num_buffers);
> -	intel_require_memory(2*num_buffers, 1024*1024, CHECK_RAM);
> +	igt_debug("%s: using 2x%d buffers, each 1MiB\n",
> +			style, num_buffers);
> +	if (!__intel_check_memory(2*num_buffers, 1024*1024, allow_mem,
> +				  NULL, NULL))
> +		return;
>  
> -	if (all) {
> -		run_basic_modes(style, mode, "", run_single);
> -
> -		igt_fork_signal_helper();
> -		run_basic_modes(style, mode, "-interruptible", run_interruptible);
> -		igt_stop_signal_helper();
> -	}
> +	run_basic_modes(style, mode, "", run_single);
>  
>  	igt_fork_signal_helper();
> +	run_basic_modes(style, mode, "-interruptible",
> +			run_interruptible);
>  	run_basic_modes(style, mode, "-forked", run_forked);
>  	run_basic_modes(style, mode, "-bomb", run_bomb);
>  	igt_stop_signal_helper();
> @@ -1328,6 +1327,8 @@ igt_main
>  		{ "stolen-", create_stolen_bo, can_create_stolen },
>  		{ NULL, NULL }
>  	}, *c;
> +	void *pinned;
> +	uint64_t pin_sz;
>  	int i;
>  
>  	igt_skip_on_simulation();
> @@ -1354,7 +1355,7 @@ igt_main
>  		if (c->require()) {
>  			snprintf(name, sizeof(name), "%s%s", c->name, "small");
>  			for (i = 0; i < ARRAY_SIZE(access_modes); i++)
> -				run_modes(name, &access_modes[i]);
> +				run_modes(name, &access_modes[i], CHECK_RAM);
>  		}
>  
>  		igt_fixture {
> @@ -1364,7 +1365,7 @@ igt_main
>  		if (c->require()) {
>  			snprintf(name, sizeof(name), "%s%s", c->name, "thrash");
>  			for (i = 0; i < ARRAY_SIZE(access_modes); i++)
> -				run_modes(name, &access_modes[i]);
> +				run_modes(name, &access_modes[i], CHECK_RAM);
>  		}
>  
>  		igt_fixture {
> @@ -1374,7 +1375,37 @@ igt_main
>  		if (c->require()) {
>  			snprintf(name, sizeof(name), "%s%s", c->name, "full");
>  			for (i = 0; i < ARRAY_SIZE(access_modes); i++)
> -				run_modes(name, &access_modes[i]);
> +				run_modes(name, &access_modes[i], CHECK_RAM);
> +		}
> +
> +		igt_fixture {
> +			num_buffers = gem_mappable_aperture_size() / (1024 * 1024);
> +			pin_sz = intel_get_avail_ram_mb() - num_buffers;
> +
> +			igt_debug("Pinning %ld MiB\n", pin_sz);
> +			pin_sz *= 1024 * 1024;
> +
> +			if (posix_memalign(&pinned, 4096, pin_sz) ||
> +			    mlock(pinned, pin_sz) ||
> +			    madvise(pinned, pin_sz, MADV_DONTFORK)) {
> +				free(pinned);
> +				pinned = NULL;
> +			}
> +			igt_require(pinned);
> +		}
> +
> +		if (c->require()) {
> +			snprintf(name, sizeof(name), "%s%s", c->name, "swap");
> +			for (i = 0; i < ARRAY_SIZE(access_modes); i++)
> +				run_modes(name, &access_modes[i], CHECK_RAM | CHECK_SWAP);
> +		}
> +
> +		igt_fixture {
> +			if (pinned) {
> +				munlock(pinned, pin_sz);
> +				free(pinned);
> +				pinned = NULL;
> +			}
>  		}
>  	}
>  }
> -- 
> 2.7.0.rc3
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
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] 4+ messages in thread

* Re: [PATCH igt] gem_concurrent_blit: Don't call igt_require() outside of a subtest/fixture
  2016-01-11  8:00 ` Daniel Vetter
@ 2016-01-11  8:52   ` Chris Wilson
  2016-01-11  9:08     ` Daniel Vetter
  0 siblings, 1 reply; 4+ messages in thread
From: Chris Wilson @ 2016-01-11  8:52 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Daniel Vetter, intel-gfx, Thomas Wood

On Mon, Jan 11, 2016 at 09:00:13AM +0100, Daniel Vetter wrote:
> On Fri, Jan 08, 2016 at 09:10:38AM +0000, Chris Wilson wrote:
> > gem_concurrent_blit tries to ensure that it doesn't try and run a test
> > that would grind the system to a halt, i.e. unexpectedly cause swap
> > thrashing. It currently calls intel_require_memory(), but outside of
> > the subtest (as the tests use fork, it cannot do requirement testing
> > within the test children) - but intel_require_memory() calls
> > igt_require() and triggers and abort. Wrapping that initial require
> > within an igt_fixture() stops the abort(), but also prevents any further
> > testing.
> > 
> > This patch restructures the requirement checking to ordinary conditions,
> > which though allowing the test to run, also prevents listing of subtests
> > on machines which cannot handle them.
> 
> 
> > ---
> >  lib/igt_aux.h              |  2 ++
> >  lib/intel_os.c             | 53 +++++++++++++++++++++++-------------
> >  tests/gem_concurrent_all.c | 67 +++++++++++++++++++++++++++++++++-------------
> >  3 files changed, 85 insertions(+), 37 deletions(-)
> > 
> > diff --git a/lib/igt_aux.h b/lib/igt_aux.h
> > index 6e11ee6..5a88c2a 100644
> > --- a/lib/igt_aux.h
> > +++ b/lib/igt_aux.h
> > @@ -88,6 +88,8 @@ uint64_t intel_get_total_swap_mb(void);
> >  
> >  #define CHECK_RAM 0x1
> >  #define CHECK_SWAP 0x2
> > +int __intel_check_memory(uint32_t count, uint64_t size, unsigned mode,
> > +			 uint64_t *out_required, uint64_t *out_total);
> >  void intel_require_memory(uint32_t count, uint64_t size, unsigned mode);
> >  int intel_num_objects_for_memory(uint32_t size, unsigned mode);
> >  
> > diff --git a/lib/intel_os.c b/lib/intel_os.c
> > index dba9e17..90f9bb3 100644
> > --- a/lib/intel_os.c
> > +++ b/lib/intel_os.c
> > @@ -192,6 +192,38 @@ intel_get_total_swap_mb(void)
> >  	return retval / (1024*1024);
> >  }
> >  
> 
> Please add the usual gtkdoc boilerplate here with a mention of
> intel_check_memory. Ack with that.

You were meant to object about how this breaks test runners and suggest
how we can do this without that breakage.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH igt] gem_concurrent_blit: Don't call igt_require() outside of a subtest/fixture
  2016-01-11  8:52   ` Chris Wilson
@ 2016-01-11  9:08     ` Daniel Vetter
  0 siblings, 0 replies; 4+ messages in thread
From: Daniel Vetter @ 2016-01-11  9:08 UTC (permalink / raw)
  To: Chris Wilson, Daniel Vetter, intel-gfx, Thomas Wood,
	Daniel Vetter

On Mon, Jan 11, 2016 at 08:52:24AM +0000, Chris Wilson wrote:
> On Mon, Jan 11, 2016 at 09:00:13AM +0100, Daniel Vetter wrote:
> > On Fri, Jan 08, 2016 at 09:10:38AM +0000, Chris Wilson wrote:
> > > gem_concurrent_blit tries to ensure that it doesn't try and run a test
> > > that would grind the system to a halt, i.e. unexpectedly cause swap
> > > thrashing. It currently calls intel_require_memory(), but outside of
> > > the subtest (as the tests use fork, it cannot do requirement testing
> > > within the test children) - but intel_require_memory() calls
> > > igt_require() and triggers and abort. Wrapping that initial require
> > > within an igt_fixture() stops the abort(), but also prevents any further
> > > testing.
> > > 
> > > This patch restructures the requirement checking to ordinary conditions,
> > > which though allowing the test to run, also prevents listing of subtests
> > > on machines which cannot handle them.
> > 
> > 
> > > ---
> > >  lib/igt_aux.h              |  2 ++
> > >  lib/intel_os.c             | 53 +++++++++++++++++++++++-------------
> > >  tests/gem_concurrent_all.c | 67 +++++++++++++++++++++++++++++++++-------------
> > >  3 files changed, 85 insertions(+), 37 deletions(-)
> > > 
> > > diff --git a/lib/igt_aux.h b/lib/igt_aux.h
> > > index 6e11ee6..5a88c2a 100644
> > > --- a/lib/igt_aux.h
> > > +++ b/lib/igt_aux.h
> > > @@ -88,6 +88,8 @@ uint64_t intel_get_total_swap_mb(void);
> > >  
> > >  #define CHECK_RAM 0x1
> > >  #define CHECK_SWAP 0x2
> > > +int __intel_check_memory(uint32_t count, uint64_t size, unsigned mode,
> > > +			 uint64_t *out_required, uint64_t *out_total);
> > >  void intel_require_memory(uint32_t count, uint64_t size, unsigned mode);
> > >  int intel_num_objects_for_memory(uint32_t size, unsigned mode);
> > >  
> > > diff --git a/lib/intel_os.c b/lib/intel_os.c
> > > index dba9e17..90f9bb3 100644
> > > --- a/lib/intel_os.c
> > > +++ b/lib/intel_os.c
> > > @@ -192,6 +192,38 @@ intel_get_total_swap_mb(void)
> > >  	return retval / (1024*1024);
> > >  }
> > >  
> > 
> > Please add the usual gtkdoc boilerplate here with a mention of
> > intel_check_memory. Ack with that.
> 
> You were meant to object about how this breaks test runners and suggest
> how we can do this without that breakage.

Oh I didn't realize that you're moving things out of
igt_fixture/igt_subtest blocks again ... Ack retracted, I'll check what
happened to my coffee meanwhile ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
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] 4+ messages in thread

end of thread, other threads:[~2016-01-11  9:08 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-01-08  9:10 [PATCH igt] gem_concurrent_blit: Don't call igt_require() outside of a subtest/fixture Chris Wilson
2016-01-11  8:00 ` Daniel Vetter
2016-01-11  8:52   ` Chris Wilson
2016-01-11  9:08     ` Daniel Vetter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).