public inbox for igt-dev@lists.freedesktop.org
 help / color / mirror / Atom feed
* [igt-dev] [PATCH i-g-t] benchmarks/gem_wsim: Heap allocate VLA structs
@ 2019-05-24  7:25 Chris Wilson
  2019-05-24  7:45 ` Ser, Simon
                   ` (6 more replies)
  0 siblings, 7 replies; 13+ messages in thread
From: Chris Wilson @ 2019-05-24  7:25 UTC (permalink / raw)
  To: intel-gfx; +Cc: igt-dev, Tvrtko Ursulin

Apparently VLA structs (e.g. struct { int array[count] }) is a gcc
extension that clang refuses to support as handling memory layout is too
difficult for it.

Move the on-stack VLA to the heap.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 benchmarks/gem_wsim.c | 146 +++++++++++++++++++++++++++---------------
 1 file changed, 95 insertions(+), 51 deletions(-)

diff --git a/benchmarks/gem_wsim.c b/benchmarks/gem_wsim.c
index e2ffb93a9..0a0032bff 100644
--- a/benchmarks/gem_wsim.c
+++ b/benchmarks/gem_wsim.c
@@ -1441,6 +1441,48 @@ set_ctx_sseu(struct ctx *ctx, uint64_t slice_mask)
 	return slice_mask;
 }
 
+static size_t sizeof_load_balance(int count)
+{
+	struct i915_context_engines_load_balance *ptr;
+
+	assert(sizeof(ptr->engines[count]) == count * sizeof(ptr->engines[0]));
+	return sizeof(*ptr) + sizeof(ptr->engines[count]);
+}
+
+static struct i915_context_engines_load_balance *
+alloc_load_balance(int count)
+{
+	return calloc(1, sizeof_load_balance(count));
+}
+
+static size_t sizeof_param_engines(int count)
+{
+	struct i915_context_param_engines *ptr;
+
+	assert(sizeof(ptr->engines[count]) == count * sizeof(ptr->engines[0]));
+	return sizeof(*ptr) + sizeof(ptr->engines[count]);
+}
+
+static struct i915_context_param_engines *
+alloc_param_engines(int count)
+{
+	return calloc(1, sizeof_param_engines(count));
+}
+
+static size_t sizeof_engines_bond(int count)
+{
+	struct i915_context_engines_bond *ptr;
+
+	assert(sizeof(ptr->engines[count]) == count * sizeof(ptr->engines[0]));
+	return sizeof(*ptr) + sizeof(ptr->engines[count]);
+}
+
+static struct i915_context_engines_bond *
+alloc_engines_bond(int count)
+{
+	return calloc(1, sizeof_engines_bond(count));
+}
+
 static int
 prepare_workload(unsigned int id, struct workload *wrk, unsigned int flags)
 {
@@ -1676,66 +1718,54 @@ prepare_workload(unsigned int id, struct workload *wrk, unsigned int flags)
 		}
 
 		if (ctx->engine_map) {
-			I915_DEFINE_CONTEXT_PARAM_ENGINES(set_engines,
-							  ctx->engine_map_count + 1);
-			I915_DEFINE_CONTEXT_ENGINES_LOAD_BALANCE(load_balance,
-								 ctx->engine_map_count);
+			struct i915_context_param_engines *set_engines =
+				alloc_param_engines(ctx->engine_map_count + 1);
+			struct i915_context_engines_load_balance *load_balance =
+				alloc_load_balance(ctx->engine_map_count);
 			struct drm_i915_gem_context_param param = {
 				.ctx_id = ctx_id,
 				.param = I915_CONTEXT_PARAM_ENGINES,
-				.size = sizeof(set_engines),
-				.value = to_user_pointer(&set_engines),
+				.size = sizeof_param_engines(ctx->engine_map_count + 1),
+				.value = to_user_pointer(set_engines),
 			};
+			struct i915_context_engines_bond *last = NULL;
 
 			if (ctx->wants_balance) {
-				set_engines.extensions =
-					to_user_pointer(&load_balance);
+				set_engines->extensions =
+					to_user_pointer(load_balance);
 
-				memset(&load_balance, 0, sizeof(load_balance));
-				load_balance.base.name =
+				load_balance->base.name =
 					I915_CONTEXT_ENGINES_EXT_LOAD_BALANCE;
-				load_balance.num_siblings =
+				load_balance->num_siblings =
 					ctx->engine_map_count;
 
 				for (j = 0; j < ctx->engine_map_count; j++)
-					load_balance.engines[j] =
+					load_balance->engines[j] =
 						get_engine(ctx->engine_map[j]);
-			} else {
-				set_engines.extensions = 0;
 			}
 
 			/* Reserve slot for virtual engine. */
-			set_engines.engines[0].engine_class =
+			set_engines->engines[0].engine_class =
 				I915_ENGINE_CLASS_INVALID;
-			set_engines.engines[0].engine_instance =
+			set_engines->engines[0].engine_instance =
 				I915_ENGINE_CLASS_INVALID_NONE;
 
 			for (j = 1; j <= ctx->engine_map_count; j++)
-				set_engines.engines[j] =
+				set_engines->engines[j] =
 					get_engine(ctx->engine_map[j - 1]);
 
+			last = NULL;
 			for (j = 0; j < ctx->bond_count; j++) {
 				unsigned long mask = ctx->bonds[j].mask;
-				I915_DEFINE_CONTEXT_ENGINES_BOND(bond,
-								 __builtin_popcount(mask));
-				struct i915_context_engines_bond *p = NULL, *prev;
+				struct i915_context_engines_bond *bond =
+					alloc_engines_bond(__builtin_popcount(mask));
 				unsigned int b, e;
 
-				prev = p;
-				p = alloca(sizeof(bond));
-				assert(p);
-				memset(p, 0, sizeof(bond));
-
-				if (j == 0)
-					load_balance.base.next_extension =
-						to_user_pointer(p);
-				else if (j < (ctx->bond_count - 1))
-					prev->base.next_extension =
-						to_user_pointer(p);
+				bond->base.next_extension = to_user_pointer(last);
+				bond->base.name = I915_CONTEXT_ENGINES_EXT_BOND;
 
-				p->base.name = I915_CONTEXT_ENGINES_EXT_BOND;
-				p->virtual_index = 0;
-				p->master = get_engine(ctx->bonds[j].master);
+				bond->virtual_index = 0;
+				bond->master = get_engine(ctx->bonds[j].master);
 
 				for (b = 0, e = 0; mask; e++, mask >>= 1) {
 					unsigned int idx;
@@ -1743,44 +1773,58 @@ prepare_workload(unsigned int id, struct workload *wrk, unsigned int flags)
 					if (!(mask & 1))
 						continue;
 
-					idx = find_engine(&set_engines.engines[1],
+					idx = find_engine(&set_engines->engines[1],
 							  ctx->engine_map_count,
 							  e);
-					p->engines[b++] =
-						set_engines.engines[1 + idx];
+					bond->engines[b++] =
+						set_engines->engines[1 + idx];
 				}
+
+				last = bond;
 			}
+			load_balance->base.next_extension = to_user_pointer(last);
 
 			gem_context_set_param(fd, &param);
+
+			while (last) {
+				struct i915_context_engines_bond *next =
+					from_user_pointer(last->base.next_extension);
+				free(last);
+				last = next;
+			}
+			free(load_balance);
+			free(set_engines);
 		} else if (ctx->wants_balance) {
 			const unsigned int count = num_engines_in_class(VCS);
-			I915_DEFINE_CONTEXT_ENGINES_LOAD_BALANCE(load_balance,
-								 count);
-			I915_DEFINE_CONTEXT_PARAM_ENGINES(set_engines,
-							  count + 1);
+			struct i915_context_engines_load_balance *load_balance =
+				alloc_load_balance(count);
+			struct i915_context_param_engines *set_engines =
+				alloc_param_engines(count + 1);
 			struct drm_i915_gem_context_param param = {
 				.ctx_id = ctx_id,
 				.param = I915_CONTEXT_PARAM_ENGINES,
-				.size = sizeof(set_engines),
-				.value = to_user_pointer(&set_engines),
+				.size = sizeof_param_engines(count + 1),
+				.value = to_user_pointer(set_engines),
 			};
 
-			set_engines.extensions = to_user_pointer(&load_balance);
+			set_engines->extensions = to_user_pointer(load_balance);
 
-			set_engines.engines[0].engine_class =
+			set_engines->engines[0].engine_class =
 				I915_ENGINE_CLASS_INVALID;
-			set_engines.engines[0].engine_instance =
+			set_engines->engines[0].engine_instance =
 				I915_ENGINE_CLASS_INVALID_NONE;
-			fill_engines_class(&set_engines.engines[1], VCS);
+			fill_engines_class(&set_engines->engines[1], VCS);
 
-			memset(&load_balance, 0, sizeof(load_balance));
-			load_balance.base.name =
+			load_balance->base.name =
 				I915_CONTEXT_ENGINES_EXT_LOAD_BALANCE;
-			load_balance.num_siblings = count;
+			load_balance->num_siblings = count;
 
-			fill_engines_class(&load_balance.engines[0], VCS);
+			fill_engines_class(&load_balance->engines[0], VCS);
 
 			gem_context_set_param(fd, &param);
+
+			free(set_engines);
+			free(load_balance);
 		}
 
 		if (wrk->sseu) {
-- 
2.20.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* Re: [igt-dev] [PATCH i-g-t] benchmarks/gem_wsim: Heap allocate VLA structs
  2019-05-24  7:25 [igt-dev] [PATCH i-g-t] benchmarks/gem_wsim: Heap allocate VLA structs Chris Wilson
@ 2019-05-24  7:45 ` Ser, Simon
  2019-05-24  7:50 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Ser, Simon @ 2019-05-24  7:45 UTC (permalink / raw)
  To: intel-gfx@lists.freedesktop.org, chris@chris-wilson.co.uk
  Cc: igt-dev@lists.freedesktop.org, Ursulin, Tvrtko

On Fri, 2019-05-24 at 08:25 +0100, Chris Wilson wrote:
> Apparently VLA structs (e.g. struct { int array[count] }) is a gcc
> extension that clang refuses to support as handling memory layout is too
> difficult for it.
> 
> Move the on-stack VLA to the heap.

IMHO using an upper bound would be much simpler.

> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  benchmarks/gem_wsim.c | 146 +++++++++++++++++++++++++++---------------
>  1 file changed, 95 insertions(+), 51 deletions(-)
> 
> diff --git a/benchmarks/gem_wsim.c b/benchmarks/gem_wsim.c
> index e2ffb93a9..0a0032bff 100644
> --- a/benchmarks/gem_wsim.c
> +++ b/benchmarks/gem_wsim.c
> @@ -1441,6 +1441,48 @@ set_ctx_sseu(struct ctx *ctx, uint64_t slice_mask)
>  	return slice_mask;
>  }
>  
> +static size_t sizeof_load_balance(int count)
> +{
> +	struct i915_context_engines_load_balance *ptr;
> +
> +	assert(sizeof(ptr->engines[count]) == count * sizeof(ptr->engines[0]));
> +	return sizeof(*ptr) + sizeof(ptr->engines[count]);
> +}
> +
> +static struct i915_context_engines_load_balance *
> +alloc_load_balance(int count)
> +{
> +	return calloc(1, sizeof_load_balance(count));
> +}
> +
> +static size_t sizeof_param_engines(int count)
> +{
> +	struct i915_context_param_engines *ptr;
> +
> +	assert(sizeof(ptr->engines[count]) == count * sizeof(ptr->engines[0]));
> +	return sizeof(*ptr) + sizeof(ptr->engines[count]);
> +}
> +
> +static struct i915_context_param_engines *
> +alloc_param_engines(int count)
> +{
> +	return calloc(1, sizeof_param_engines(count));
> +}
> +
> +static size_t sizeof_engines_bond(int count)
> +{
> +	struct i915_context_engines_bond *ptr;
> +
> +	assert(sizeof(ptr->engines[count]) == count * sizeof(ptr->engines[0]));
> +	return sizeof(*ptr) + sizeof(ptr->engines[count]);
> +}
> +
> +static struct i915_context_engines_bond *
> +alloc_engines_bond(int count)
> +{
> +	return calloc(1, sizeof_engines_bond(count));
> +}
> +
>  static int
>  prepare_workload(unsigned int id, struct workload *wrk, unsigned int flags)
>  {
> @@ -1676,66 +1718,54 @@ prepare_workload(unsigned int id, struct workload *wrk, unsigned int flags)
>  		}
>  
>  		if (ctx->engine_map) {
> -			I915_DEFINE_CONTEXT_PARAM_ENGINES(set_engines,
> -							  ctx->engine_map_count + 1);
> -			I915_DEFINE_CONTEXT_ENGINES_LOAD_BALANCE(load_balance,
> -								 ctx->engine_map_count);
> +			struct i915_context_param_engines *set_engines =
> +				alloc_param_engines(ctx->engine_map_count + 1);
> +			struct i915_context_engines_load_balance *load_balance =
> +				alloc_load_balance(ctx->engine_map_count);
>  			struct drm_i915_gem_context_param param = {
>  				.ctx_id = ctx_id,
>  				.param = I915_CONTEXT_PARAM_ENGINES,
> -				.size = sizeof(set_engines),
> -				.value = to_user_pointer(&set_engines),
> +				.size = sizeof_param_engines(ctx->engine_map_count + 1),
> +				.value = to_user_pointer(set_engines),
>  			};
> +			struct i915_context_engines_bond *last = NULL;
>  
>  			if (ctx->wants_balance) {
> -				set_engines.extensions =
> -					to_user_pointer(&load_balance);
> +				set_engines->extensions =
> +					to_user_pointer(load_balance);
>  
> -				memset(&load_balance, 0, sizeof(load_balance));
> -				load_balance.base.name =
> +				load_balance->base.name =
>  					I915_CONTEXT_ENGINES_EXT_LOAD_BALANCE;
> -				load_balance.num_siblings =
> +				load_balance->num_siblings =
>  					ctx->engine_map_count;
>  
>  				for (j = 0; j < ctx->engine_map_count; j++)
> -					load_balance.engines[j] =
> +					load_balance->engines[j] =
>  						get_engine(ctx->engine_map[j]);
> -			} else {
> -				set_engines.extensions = 0;
>  			}
>  
>  			/* Reserve slot for virtual engine. */
> -			set_engines.engines[0].engine_class =
> +			set_engines->engines[0].engine_class =
>  				I915_ENGINE_CLASS_INVALID;
> -			set_engines.engines[0].engine_instance =
> +			set_engines->engines[0].engine_instance =
>  				I915_ENGINE_CLASS_INVALID_NONE;
>  
>  			for (j = 1; j <= ctx->engine_map_count; j++)
> -				set_engines.engines[j] =
> +				set_engines->engines[j] =
>  					get_engine(ctx->engine_map[j - 1]);
>  
> +			last = NULL;
>  			for (j = 0; j < ctx->bond_count; j++) {
>  				unsigned long mask = ctx->bonds[j].mask;
> -				I915_DEFINE_CONTEXT_ENGINES_BOND(bond,
> -								 __builtin_popcount(mask));
> -				struct i915_context_engines_bond *p = NULL, *prev;
> +				struct i915_context_engines_bond *bond =
> +					alloc_engines_bond(__builtin_popcount(mask));
>  				unsigned int b, e;
>  
> -				prev = p;
> -				p = alloca(sizeof(bond));
> -				assert(p);
> -				memset(p, 0, sizeof(bond));
> -
> -				if (j == 0)
> -					load_balance.base.next_extension =
> -						to_user_pointer(p);
> -				else if (j < (ctx->bond_count - 1))
> -					prev->base.next_extension =
> -						to_user_pointer(p);
> +				bond->base.next_extension = to_user_pointer(last);
> +				bond->base.name = I915_CONTEXT_ENGINES_EXT_BOND;
>  
> -				p->base.name = I915_CONTEXT_ENGINES_EXT_BOND;
> -				p->virtual_index = 0;
> -				p->master = get_engine(ctx->bonds[j].master);
> +				bond->virtual_index = 0;
> +				bond->master = get_engine(ctx->bonds[j].master);
>  
>  				for (b = 0, e = 0; mask; e++, mask >>= 1) {
>  					unsigned int idx;
> @@ -1743,44 +1773,58 @@ prepare_workload(unsigned int id, struct workload *wrk, unsigned int flags)
>  					if (!(mask & 1))
>  						continue;
>  
> -					idx = find_engine(&set_engines.engines[1],
> +					idx = find_engine(&set_engines->engines[1],
>  							  ctx->engine_map_count,
>  							  e);
> -					p->engines[b++] =
> -						set_engines.engines[1 + idx];
> +					bond->engines[b++] =
> +						set_engines->engines[1 + idx];
>  				}
> +
> +				last = bond;
>  			}
> +			load_balance->base.next_extension = to_user_pointer(last);
>  
>  			gem_context_set_param(fd, &param);
> +
> +			while (last) {
> +				struct i915_context_engines_bond *next =
> +					from_user_pointer(last->base.next_extension);
> +				free(last);
> +				last = next;
> +			}
> +			free(load_balance);
> +			free(set_engines);
>  		} else if (ctx->wants_balance) {
>  			const unsigned int count = num_engines_in_class(VCS);
> -			I915_DEFINE_CONTEXT_ENGINES_LOAD_BALANCE(load_balance,
> -								 count);
> -			I915_DEFINE_CONTEXT_PARAM_ENGINES(set_engines,
> -							  count + 1);
> +			struct i915_context_engines_load_balance *load_balance =
> +				alloc_load_balance(count);
> +			struct i915_context_param_engines *set_engines =
> +				alloc_param_engines(count + 1);
>  			struct drm_i915_gem_context_param param = {
>  				.ctx_id = ctx_id,
>  				.param = I915_CONTEXT_PARAM_ENGINES,
> -				.size = sizeof(set_engines),
> -				.value = to_user_pointer(&set_engines),
> +				.size = sizeof_param_engines(count + 1),
> +				.value = to_user_pointer(set_engines),
>  			};
>  
> -			set_engines.extensions = to_user_pointer(&load_balance);
> +			set_engines->extensions = to_user_pointer(load_balance);
>  
> -			set_engines.engines[0].engine_class =
> +			set_engines->engines[0].engine_class =
>  				I915_ENGINE_CLASS_INVALID;
> -			set_engines.engines[0].engine_instance =
> +			set_engines->engines[0].engine_instance =
>  				I915_ENGINE_CLASS_INVALID_NONE;
> -			fill_engines_class(&set_engines.engines[1], VCS);
> +			fill_engines_class(&set_engines->engines[1], VCS);
>  
> -			memset(&load_balance, 0, sizeof(load_balance));
> -			load_balance.base.name =
> +			load_balance->base.name =
>  				I915_CONTEXT_ENGINES_EXT_LOAD_BALANCE;
> -			load_balance.num_siblings = count;
> +			load_balance->num_siblings = count;
>  
> -			fill_engines_class(&load_balance.engines[0], VCS);
> +			fill_engines_class(&load_balance->engines[0], VCS);
>  
>  			gem_context_set_param(fd, &param);
> +
> +			free(set_engines);
> +			free(load_balance);
>  		}
>  
>  		if (wrk->sseu) {
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* [igt-dev] ✓ Fi.CI.BAT: success for benchmarks/gem_wsim: Heap allocate VLA structs
  2019-05-24  7:25 [igt-dev] [PATCH i-g-t] benchmarks/gem_wsim: Heap allocate VLA structs Chris Wilson
  2019-05-24  7:45 ` Ser, Simon
@ 2019-05-24  7:50 ` Patchwork
  2019-05-24  8:20 ` [igt-dev] [Intel-gfx] [PATCH i-g-t] " Tvrtko Ursulin
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Patchwork @ 2019-05-24  7:50 UTC (permalink / raw)
  To: Chris Wilson; +Cc: igt-dev

== Series Details ==

Series: benchmarks/gem_wsim: Heap allocate VLA structs
URL   : https://patchwork.freedesktop.org/series/61085/
State : success

== Summary ==

CI Bug Log - changes from IGT_5012 -> IGTPW_3051
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/61085/revisions/1/mbox/

Known issues
------------

  Here are the changes found in IGTPW_3051 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@prime_vgem@basic-fence-flip:
    - fi-ilk-650:         [PASS][1] -> [DMESG-WARN][2] ([fdo#106387])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5012/fi-ilk-650/igt@prime_vgem@basic-fence-flip.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3051/fi-ilk-650/igt@prime_vgem@basic-fence-flip.html

  
#### Possible fixes ####

  * igt@i915_module_load@reload-with-fault-injection:
    - {fi-icl-u3}:        [DMESG-WARN][3] ([fdo#110718]) -> [PASS][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5012/fi-icl-u3/igt@i915_module_load@reload-with-fault-injection.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3051/fi-icl-u3/igt@i915_module_load@reload-with-fault-injection.html

  * igt@i915_selftest@live_contexts:
    - fi-skl-gvtdvm:      [DMESG-FAIL][5] ([fdo#110235]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5012/fi-skl-gvtdvm/igt@i915_selftest@live_contexts.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3051/fi-skl-gvtdvm/igt@i915_selftest@live_contexts.html

  * igt@kms_cursor_legacy@basic-flip-after-cursor-legacy:
    - {fi-icl-u3}:        [DMESG-WARN][7] ([fdo#107724]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5012/fi-icl-u3/igt@kms_cursor_legacy@basic-flip-after-cursor-legacy.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3051/fi-icl-u3/igt@kms_cursor_legacy@basic-flip-after-cursor-legacy.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#106387]: https://bugs.freedesktop.org/show_bug.cgi?id=106387
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#110235]: https://bugs.freedesktop.org/show_bug.cgi?id=110235
  [fdo#110718]: https://bugs.freedesktop.org/show_bug.cgi?id=110718


Participating hosts (44 -> 40)
------------------------------

  Additional (1): fi-hsw-4770 
  Missing    (5): fi-kbl-soraka fi-ilk-m540 fi-skl-guc fi-bsw-cyan fi-bdw-samus 


Build changes
-------------

  * IGT: IGT_5012 -> IGTPW_3051

  CI_DRM_6138: a17cee540f7ffd493c7944dea5ea6e2d32fdfdb4 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_3051: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3051/
  IGT_5012: 996b4346b6a7f2bd0919817648a4f7a382e59757 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3051/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] benchmarks/gem_wsim: Heap allocate VLA structs
  2019-05-24  7:25 [igt-dev] [PATCH i-g-t] benchmarks/gem_wsim: Heap allocate VLA structs Chris Wilson
  2019-05-24  7:45 ` Ser, Simon
  2019-05-24  7:50 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork
@ 2019-05-24  8:20 ` Tvrtko Ursulin
  2019-05-24  8:27   ` Ser, Simon
  2019-05-24  8:33   ` Chris Wilson
  2019-05-24  8:45 ` [igt-dev] [PATCH i-g-t v2] benchmarks/gem_wsim: Manually calculate VLA struct sizes Chris Wilson
                   ` (3 subsequent siblings)
  6 siblings, 2 replies; 13+ messages in thread
From: Tvrtko Ursulin @ 2019-05-24  8:20 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: igt-dev


On 24/05/2019 08:25, Chris Wilson wrote:
> Apparently VLA structs (e.g. struct { int array[count] }) is a gcc
> extension that clang refuses to support as handling memory layout is too
> difficult for it.
> 
> Move the on-stack VLA to the heap.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   benchmarks/gem_wsim.c | 146 +++++++++++++++++++++++++++---------------
>   1 file changed, 95 insertions(+), 51 deletions(-)
> 
> diff --git a/benchmarks/gem_wsim.c b/benchmarks/gem_wsim.c
> index e2ffb93a9..0a0032bff 100644
> --- a/benchmarks/gem_wsim.c
> +++ b/benchmarks/gem_wsim.c
> @@ -1441,6 +1441,48 @@ set_ctx_sseu(struct ctx *ctx, uint64_t slice_mask)
>   	return slice_mask;
>   }
>   
> +static size_t sizeof_load_balance(int count)
> +{
> +	struct i915_context_engines_load_balance *ptr;
> +
> +	assert(sizeof(ptr->engines[count]) == count * sizeof(ptr->engines[0]));

This seems wrong - is bound to trigger.

> +	return sizeof(*ptr) + sizeof(ptr->engines[count]);

So size of of engine needs to be multiplied by count.

> +}
> +
> +static struct i915_context_engines_load_balance *
> +alloc_load_balance(int count)
> +{
> +	return calloc(1, sizeof_load_balance(count));

How about alloca so cleanup is simpler? Or is alloca also on the 
unpopular list?

Or possibly what Simon suggested, just a large temporary stack arrays 
would be enough and easiest diff. Just with an assert that it fits.

I can do that if you want?

Regards,

Tvrtko

> +}
> +
> +static size_t sizeof_param_engines(int count)
> +{
> +	struct i915_context_param_engines *ptr;
> +
> +	assert(sizeof(ptr->engines[count]) == count * sizeof(ptr->engines[0]));
> +	return sizeof(*ptr) + sizeof(ptr->engines[count]);
> +}
> +
> +static struct i915_context_param_engines *
> +alloc_param_engines(int count)
> +{
> +	return calloc(1, sizeof_param_engines(count));
> +}
> +
> +static size_t sizeof_engines_bond(int count)
> +{
> +	struct i915_context_engines_bond *ptr;
> +
> +	assert(sizeof(ptr->engines[count]) == count * sizeof(ptr->engines[0]));
> +	return sizeof(*ptr) + sizeof(ptr->engines[count]);
> +}
> +
> +static struct i915_context_engines_bond *
> +alloc_engines_bond(int count)
> +{
> +	return calloc(1, sizeof_engines_bond(count));
> +}
> +
>   static int
>   prepare_workload(unsigned int id, struct workload *wrk, unsigned int flags)
>   {
> @@ -1676,66 +1718,54 @@ prepare_workload(unsigned int id, struct workload *wrk, unsigned int flags)
>   		}
>   
>   		if (ctx->engine_map) {
> -			I915_DEFINE_CONTEXT_PARAM_ENGINES(set_engines,
> -							  ctx->engine_map_count + 1);
> -			I915_DEFINE_CONTEXT_ENGINES_LOAD_BALANCE(load_balance,
> -								 ctx->engine_map_count);
> +			struct i915_context_param_engines *set_engines =
> +				alloc_param_engines(ctx->engine_map_count + 1);
> +			struct i915_context_engines_load_balance *load_balance =
> +				alloc_load_balance(ctx->engine_map_count);
>   			struct drm_i915_gem_context_param param = {
>   				.ctx_id = ctx_id,
>   				.param = I915_CONTEXT_PARAM_ENGINES,
> -				.size = sizeof(set_engines),
> -				.value = to_user_pointer(&set_engines),
> +				.size = sizeof_param_engines(ctx->engine_map_count + 1),
> +				.value = to_user_pointer(set_engines),
>   			};
> +			struct i915_context_engines_bond *last = NULL;
>   
>   			if (ctx->wants_balance) {
> -				set_engines.extensions =
> -					to_user_pointer(&load_balance);
> +				set_engines->extensions =
> +					to_user_pointer(load_balance);
>   
> -				memset(&load_balance, 0, sizeof(load_balance));
> -				load_balance.base.name =
> +				load_balance->base.name =
>   					I915_CONTEXT_ENGINES_EXT_LOAD_BALANCE;
> -				load_balance.num_siblings =
> +				load_balance->num_siblings =
>   					ctx->engine_map_count;
>   
>   				for (j = 0; j < ctx->engine_map_count; j++)
> -					load_balance.engines[j] =
> +					load_balance->engines[j] =
>   						get_engine(ctx->engine_map[j]);
> -			} else {
> -				set_engines.extensions = 0;
>   			}
>   
>   			/* Reserve slot for virtual engine. */
> -			set_engines.engines[0].engine_class =
> +			set_engines->engines[0].engine_class =
>   				I915_ENGINE_CLASS_INVALID;
> -			set_engines.engines[0].engine_instance =
> +			set_engines->engines[0].engine_instance =
>   				I915_ENGINE_CLASS_INVALID_NONE;
>   
>   			for (j = 1; j <= ctx->engine_map_count; j++)
> -				set_engines.engines[j] =
> +				set_engines->engines[j] =
>   					get_engine(ctx->engine_map[j - 1]);
>   
> +			last = NULL;
>   			for (j = 0; j < ctx->bond_count; j++) {
>   				unsigned long mask = ctx->bonds[j].mask;
> -				I915_DEFINE_CONTEXT_ENGINES_BOND(bond,
> -								 __builtin_popcount(mask));
> -				struct i915_context_engines_bond *p = NULL, *prev;
> +				struct i915_context_engines_bond *bond =
> +					alloc_engines_bond(__builtin_popcount(mask));
>   				unsigned int b, e;
>   
> -				prev = p;
> -				p = alloca(sizeof(bond));
> -				assert(p);
> -				memset(p, 0, sizeof(bond));
> -
> -				if (j == 0)
> -					load_balance.base.next_extension =
> -						to_user_pointer(p);
> -				else if (j < (ctx->bond_count - 1))
> -					prev->base.next_extension =
> -						to_user_pointer(p);
> +				bond->base.next_extension = to_user_pointer(last);
> +				bond->base.name = I915_CONTEXT_ENGINES_EXT_BOND;
>   
> -				p->base.name = I915_CONTEXT_ENGINES_EXT_BOND;
> -				p->virtual_index = 0;
> -				p->master = get_engine(ctx->bonds[j].master);
> +				bond->virtual_index = 0;
> +				bond->master = get_engine(ctx->bonds[j].master);
>   
>   				for (b = 0, e = 0; mask; e++, mask >>= 1) {
>   					unsigned int idx;
> @@ -1743,44 +1773,58 @@ prepare_workload(unsigned int id, struct workload *wrk, unsigned int flags)
>   					if (!(mask & 1))
>   						continue;
>   
> -					idx = find_engine(&set_engines.engines[1],
> +					idx = find_engine(&set_engines->engines[1],
>   							  ctx->engine_map_count,
>   							  e);
> -					p->engines[b++] =
> -						set_engines.engines[1 + idx];
> +					bond->engines[b++] =
> +						set_engines->engines[1 + idx];
>   				}
> +
> +				last = bond;
>   			}
> +			load_balance->base.next_extension = to_user_pointer(last);
>   
>   			gem_context_set_param(fd, &param);
> +
> +			while (last) {
> +				struct i915_context_engines_bond *next =
> +					from_user_pointer(last->base.next_extension);
> +				free(last);
> +				last = next;
> +			}
> +			free(load_balance);
> +			free(set_engines);
>   		} else if (ctx->wants_balance) {
>   			const unsigned int count = num_engines_in_class(VCS);
> -			I915_DEFINE_CONTEXT_ENGINES_LOAD_BALANCE(load_balance,
> -								 count);
> -			I915_DEFINE_CONTEXT_PARAM_ENGINES(set_engines,
> -							  count + 1);
> +			struct i915_context_engines_load_balance *load_balance =
> +				alloc_load_balance(count);
> +			struct i915_context_param_engines *set_engines =
> +				alloc_param_engines(count + 1);
>   			struct drm_i915_gem_context_param param = {
>   				.ctx_id = ctx_id,
>   				.param = I915_CONTEXT_PARAM_ENGINES,
> -				.size = sizeof(set_engines),
> -				.value = to_user_pointer(&set_engines),
> +				.size = sizeof_param_engines(count + 1),
> +				.value = to_user_pointer(set_engines),
>   			};
>   
> -			set_engines.extensions = to_user_pointer(&load_balance);
> +			set_engines->extensions = to_user_pointer(load_balance);
>   
> -			set_engines.engines[0].engine_class =
> +			set_engines->engines[0].engine_class =
>   				I915_ENGINE_CLASS_INVALID;
> -			set_engines.engines[0].engine_instance =
> +			set_engines->engines[0].engine_instance =
>   				I915_ENGINE_CLASS_INVALID_NONE;
> -			fill_engines_class(&set_engines.engines[1], VCS);
> +			fill_engines_class(&set_engines->engines[1], VCS);
>   
> -			memset(&load_balance, 0, sizeof(load_balance));
> -			load_balance.base.name =
> +			load_balance->base.name =
>   				I915_CONTEXT_ENGINES_EXT_LOAD_BALANCE;
> -			load_balance.num_siblings = count;
> +			load_balance->num_siblings = count;
>   
> -			fill_engines_class(&load_balance.engines[0], VCS);
> +			fill_engines_class(&load_balance->engines[0], VCS);
>   
>   			gem_context_set_param(fd, &param);
> +
> +			free(set_engines);
> +			free(load_balance);
>   		}
>   
>   		if (wrk->sseu) {
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] benchmarks/gem_wsim: Heap allocate VLA structs
  2019-05-24  8:20 ` [igt-dev] [Intel-gfx] [PATCH i-g-t] " Tvrtko Ursulin
@ 2019-05-24  8:27   ` Ser, Simon
  2019-05-24  8:33   ` Chris Wilson
  1 sibling, 0 replies; 13+ messages in thread
From: Ser, Simon @ 2019-05-24  8:27 UTC (permalink / raw)
  To: tvrtko.ursulin@linux.intel.com, intel-gfx@lists.freedesktop.org,
	chris@chris-wilson.co.uk
  Cc: igt-dev@lists.freedesktop.org

On Fri, 2019-05-24 at 09:20 +0100, Tvrtko Ursulin wrote:
> On 24/05/2019 08:25, Chris Wilson wrote:
> > Apparently VLA structs (e.g. struct { int array[count] }) is a gcc
> > extension that clang refuses to support as handling memory layout is too
> > difficult for it.
> > 
> > Move the on-stack VLA to the heap.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > ---
> >   benchmarks/gem_wsim.c | 146 +++++++++++++++++++++++++++---------------
> >   1 file changed, 95 insertions(+), 51 deletions(-)
> > 
> > diff --git a/benchmarks/gem_wsim.c b/benchmarks/gem_wsim.c
> > index e2ffb93a9..0a0032bff 100644
> > --- a/benchmarks/gem_wsim.c
> > +++ b/benchmarks/gem_wsim.c
> > @@ -1441,6 +1441,48 @@ set_ctx_sseu(struct ctx *ctx, uint64_t slice_mask)
> >   	return slice_mask;
> >   }
> >   
> > +static size_t sizeof_load_balance(int count)
> > +{
> > +	struct i915_context_engines_load_balance *ptr;
> > +
> > +	assert(sizeof(ptr->engines[count]) == count * sizeof(ptr->engines[0]));
> 
> This seems wrong - is bound to trigger.
> 
> > +	return sizeof(*ptr) + sizeof(ptr->engines[count]);
> 
> So size of of engine needs to be multiplied by count.
> 
> > +}
> > +
> > +static struct i915_context_engines_load_balance *
> > +alloc_load_balance(int count)
> > +{
> > +	return calloc(1, sizeof_load_balance(count));
> 
> How about alloca so cleanup is simpler? Or is alloca also on the 
> unpopular list?
> 
> Or possibly what Simon suggested, just a large temporary stack arrays 
> would be enough and easiest diff. Just with an assert that it fits.
> 
> I can do that if you want?

I think Arek already has a patch for this.

> Regards,
> 
> Tvrtko
> 
> > +}
> > +
> > +static size_t sizeof_param_engines(int count)
> > +{
> > +	struct i915_context_param_engines *ptr;
> > +
> > +	assert(sizeof(ptr->engines[count]) == count * sizeof(ptr->engines[0]));
> > +	return sizeof(*ptr) + sizeof(ptr->engines[count]);
> > +}
> > +
> > +static struct i915_context_param_engines *
> > +alloc_param_engines(int count)
> > +{
> > +	return calloc(1, sizeof_param_engines(count));
> > +}
> > +
> > +static size_t sizeof_engines_bond(int count)
> > +{
> > +	struct i915_context_engines_bond *ptr;
> > +
> > +	assert(sizeof(ptr->engines[count]) == count * sizeof(ptr->engines[0]));
> > +	return sizeof(*ptr) + sizeof(ptr->engines[count]);
> > +}
> > +
> > +static struct i915_context_engines_bond *
> > +alloc_engines_bond(int count)
> > +{
> > +	return calloc(1, sizeof_engines_bond(count));
> > +}
> > +
> >   static int
> >   prepare_workload(unsigned int id, struct workload *wrk, unsigned int flags)
> >   {
> > @@ -1676,66 +1718,54 @@ prepare_workload(unsigned int id, struct workload *wrk, unsigned int flags)
> >   		}
> >   
> >   		if (ctx->engine_map) {
> > -			I915_DEFINE_CONTEXT_PARAM_ENGINES(set_engines,
> > -							  ctx->engine_map_count + 1);
> > -			I915_DEFINE_CONTEXT_ENGINES_LOAD_BALANCE(load_balance,
> > -								 ctx->engine_map_count);
> > +			struct i915_context_param_engines *set_engines =
> > +				alloc_param_engines(ctx->engine_map_count + 1);
> > +			struct i915_context_engines_load_balance *load_balance =
> > +				alloc_load_balance(ctx->engine_map_count);
> >   			struct drm_i915_gem_context_param param = {
> >   				.ctx_id = ctx_id,
> >   				.param = I915_CONTEXT_PARAM_ENGINES,
> > -				.size = sizeof(set_engines),
> > -				.value = to_user_pointer(&set_engines),
> > +				.size = sizeof_param_engines(ctx->engine_map_count + 1),
> > +				.value = to_user_pointer(set_engines),
> >   			};
> > +			struct i915_context_engines_bond *last = NULL;
> >   
> >   			if (ctx->wants_balance) {
> > -				set_engines.extensions =
> > -					to_user_pointer(&load_balance);
> > +				set_engines->extensions =
> > +					to_user_pointer(load_balance);
> >   
> > -				memset(&load_balance, 0, sizeof(load_balance));
> > -				load_balance.base.name =
> > +				load_balance->base.name =
> >   					I915_CONTEXT_ENGINES_EXT_LOAD_BALANCE;
> > -				load_balance.num_siblings =
> > +				load_balance->num_siblings =
> >   					ctx->engine_map_count;
> >   
> >   				for (j = 0; j < ctx->engine_map_count; j++)
> > -					load_balance.engines[j] =
> > +					load_balance->engines[j] =
> >   						get_engine(ctx->engine_map[j]);
> > -			} else {
> > -				set_engines.extensions = 0;
> >   			}
> >   
> >   			/* Reserve slot for virtual engine. */
> > -			set_engines.engines[0].engine_class =
> > +			set_engines->engines[0].engine_class =
> >   				I915_ENGINE_CLASS_INVALID;
> > -			set_engines.engines[0].engine_instance =
> > +			set_engines->engines[0].engine_instance =
> >   				I915_ENGINE_CLASS_INVALID_NONE;
> >   
> >   			for (j = 1; j <= ctx->engine_map_count; j++)
> > -				set_engines.engines[j] =
> > +				set_engines->engines[j] =
> >   					get_engine(ctx->engine_map[j - 1]);
> >   
> > +			last = NULL;
> >   			for (j = 0; j < ctx->bond_count; j++) {
> >   				unsigned long mask = ctx->bonds[j].mask;
> > -				I915_DEFINE_CONTEXT_ENGINES_BOND(bond,
> > -								 __builtin_popcount(mask));
> > -				struct i915_context_engines_bond *p = NULL, *prev;
> > +				struct i915_context_engines_bond *bond =
> > +					alloc_engines_bond(__builtin_popcount(mask));
> >   				unsigned int b, e;
> >   
> > -				prev = p;
> > -				p = alloca(sizeof(bond));
> > -				assert(p);
> > -				memset(p, 0, sizeof(bond));
> > -
> > -				if (j == 0)
> > -					load_balance.base.next_extension =
> > -						to_user_pointer(p);
> > -				else if (j < (ctx->bond_count - 1))
> > -					prev->base.next_extension =
> > -						to_user_pointer(p);
> > +				bond->base.next_extension = to_user_pointer(last);
> > +				bond->base.name = I915_CONTEXT_ENGINES_EXT_BOND;
> >   
> > -				p->base.name = I915_CONTEXT_ENGINES_EXT_BOND;
> > -				p->virtual_index = 0;
> > -				p->master = get_engine(ctx->bonds[j].master);
> > +				bond->virtual_index = 0;
> > +				bond->master = get_engine(ctx->bonds[j].master);
> >   
> >   				for (b = 0, e = 0; mask; e++, mask >>= 1) {
> >   					unsigned int idx;
> > @@ -1743,44 +1773,58 @@ prepare_workload(unsigned int id, struct workload *wrk, unsigned int flags)
> >   					if (!(mask & 1))
> >   						continue;
> >   
> > -					idx = find_engine(&set_engines.engines[1],
> > +					idx = find_engine(&set_engines->engines[1],
> >   							  ctx->engine_map_count,
> >   							  e);
> > -					p->engines[b++] =
> > -						set_engines.engines[1 + idx];
> > +					bond->engines[b++] =
> > +						set_engines->engines[1 + idx];
> >   				}
> > +
> > +				last = bond;
> >   			}
> > +			load_balance->base.next_extension = to_user_pointer(last);
> >   
> >   			gem_context_set_param(fd, &param);
> > +
> > +			while (last) {
> > +				struct i915_context_engines_bond *next =
> > +					from_user_pointer(last->base.next_extension);
> > +				free(last);
> > +				last = next;
> > +			}
> > +			free(load_balance);
> > +			free(set_engines);
> >   		} else if (ctx->wants_balance) {
> >   			const unsigned int count = num_engines_in_class(VCS);
> > -			I915_DEFINE_CONTEXT_ENGINES_LOAD_BALANCE(load_balance,
> > -								 count);
> > -			I915_DEFINE_CONTEXT_PARAM_ENGINES(set_engines,
> > -							  count + 1);
> > +			struct i915_context_engines_load_balance *load_balance =
> > +				alloc_load_balance(count);
> > +			struct i915_context_param_engines *set_engines =
> > +				alloc_param_engines(count + 1);
> >   			struct drm_i915_gem_context_param param = {
> >   				.ctx_id = ctx_id,
> >   				.param = I915_CONTEXT_PARAM_ENGINES,
> > -				.size = sizeof(set_engines),
> > -				.value = to_user_pointer(&set_engines),
> > +				.size = sizeof_param_engines(count + 1),
> > +				.value = to_user_pointer(set_engines),
> >   			};
> >   
> > -			set_engines.extensions = to_user_pointer(&load_balance);
> > +			set_engines->extensions = to_user_pointer(load_balance);
> >   
> > -			set_engines.engines[0].engine_class =
> > +			set_engines->engines[0].engine_class =
> >   				I915_ENGINE_CLASS_INVALID;
> > -			set_engines.engines[0].engine_instance =
> > +			set_engines->engines[0].engine_instance =
> >   				I915_ENGINE_CLASS_INVALID_NONE;
> > -			fill_engines_class(&set_engines.engines[1], VCS);
> > +			fill_engines_class(&set_engines->engines[1], VCS);
> >   
> > -			memset(&load_balance, 0, sizeof(load_balance));
> > -			load_balance.base.name =
> > +			load_balance->base.name =
> >   				I915_CONTEXT_ENGINES_EXT_LOAD_BALANCE;
> > -			load_balance.num_siblings = count;
> > +			load_balance->num_siblings = count;
> >   
> > -			fill_engines_class(&load_balance.engines[0], VCS);
> > +			fill_engines_class(&load_balance->engines[0], VCS);
> >   
> >   			gem_context_set_param(fd, &param);
> > +
> > +			free(set_engines);
> > +			free(load_balance);
> >   		}
> >   
> >   		if (wrk->sseu) {
> > 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] benchmarks/gem_wsim: Heap allocate VLA structs
  2019-05-24  8:20 ` [igt-dev] [Intel-gfx] [PATCH i-g-t] " Tvrtko Ursulin
  2019-05-24  8:27   ` Ser, Simon
@ 2019-05-24  8:33   ` Chris Wilson
  2019-05-24  8:39     ` Ser, Simon
  2019-05-24  8:44     ` Tvrtko Ursulin
  1 sibling, 2 replies; 13+ messages in thread
From: Chris Wilson @ 2019-05-24  8:33 UTC (permalink / raw)
  To: Tvrtko Ursulin, intel-gfx; +Cc: igt-dev

Quoting Tvrtko Ursulin (2019-05-24 09:20:47)
> 
> On 24/05/2019 08:25, Chris Wilson wrote:
> > Apparently VLA structs (e.g. struct { int array[count] }) is a gcc
> > extension that clang refuses to support as handling memory layout is too
> > difficult for it.
> > 
> > Move the on-stack VLA to the heap.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > ---
> >   benchmarks/gem_wsim.c | 146 +++++++++++++++++++++++++++---------------
> >   1 file changed, 95 insertions(+), 51 deletions(-)
> > 
> > diff --git a/benchmarks/gem_wsim.c b/benchmarks/gem_wsim.c
> > index e2ffb93a9..0a0032bff 100644
> > --- a/benchmarks/gem_wsim.c
> > +++ b/benchmarks/gem_wsim.c
> > @@ -1441,6 +1441,48 @@ set_ctx_sseu(struct ctx *ctx, uint64_t slice_mask)
> >       return slice_mask;
> >   }
> >   
> > +static size_t sizeof_load_balance(int count)
> > +{
> > +     struct i915_context_engines_load_balance *ptr;
> > +
> > +     assert(sizeof(ptr->engines[count]) == count * sizeof(ptr->engines[0]));
> 
> This seems wrong - is bound to trigger.

Why does it seem wrong? That's the calculation used previously, and the
ptr->engines[] was meant to be packed in order for
sizeof(ptr->engines[count]) == count * sizeof(ptr->engines[0]). Anyway,
I threw it in there to check if the calculation was sane.

> > +     return sizeof(*ptr) + sizeof(ptr->engines[count]);
> 
> So size of of engine needs to be multiplied by count.

(Just note this is the what the current VLA evaluates to :)

> > +}
> > +
> > +static struct i915_context_engines_load_balance *
> > +alloc_load_balance(int count)
> > +{
> > +     return calloc(1, sizeof_load_balance(count));
> 
> How about alloca so cleanup is simpler? Or is alloca also on the 
> unpopular list?

I don't mind. Would shave a few lines indeed, but we need the memsets
back. #define alloca0()?

> Or possibly what Simon suggested, just a large temporary stack arrays 
> would be enough and easiest diff. Just with an assert that it fits.

I don't think that is as clean for the long term.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] benchmarks/gem_wsim: Heap allocate VLA structs
  2019-05-24  8:33   ` Chris Wilson
@ 2019-05-24  8:39     ` Ser, Simon
  2019-05-24  8:44     ` Tvrtko Ursulin
  1 sibling, 0 replies; 13+ messages in thread
From: Ser, Simon @ 2019-05-24  8:39 UTC (permalink / raw)
  To: tvrtko.ursulin@linux.intel.com, intel-gfx@lists.freedesktop.org,
	chris@chris-wilson.co.uk
  Cc: igt-dev@lists.freedesktop.org

On Fri, 2019-05-24 at 09:33 +0100, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-05-24 09:20:47)
> > On 24/05/2019 08:25, Chris Wilson wrote:
> > > Apparently VLA structs (e.g. struct { int array[count] }) is a gcc
> > > extension that clang refuses to support as handling memory layout is too
> > > difficult for it.
> > > 
> > > Move the on-stack VLA to the heap.
> > > 
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > > ---
> > >   benchmarks/gem_wsim.c | 146 +++++++++++++++++++++++++++---------------
> > >   1 file changed, 95 insertions(+), 51 deletions(-)
> > > 
> > > diff --git a/benchmarks/gem_wsim.c b/benchmarks/gem_wsim.c
> > > index e2ffb93a9..0a0032bff 100644
> > > --- a/benchmarks/gem_wsim.c
> > > +++ b/benchmarks/gem_wsim.c
> > > @@ -1441,6 +1441,48 @@ set_ctx_sseu(struct ctx *ctx, uint64_t slice_mask)
> > >       return slice_mask;
> > >   }
> > >   
> > > +static size_t sizeof_load_balance(int count)
> > > +{
> > > +     struct i915_context_engines_load_balance *ptr;
> > > +
> > > +     assert(sizeof(ptr->engines[count]) == count * sizeof(ptr->engines[0]));
> > 
> > This seems wrong - is bound to trigger.
> 
> Why does it seem wrong? That's the calculation used previously, and the
> ptr->engines[] was meant to be packed in order for
> sizeof(ptr->engines[count]) == count * sizeof(ptr->engines[0]). Anyway,
> I threw it in there to check if the calculation was sane.
> 
> > > +     return sizeof(*ptr) + sizeof(ptr->engines[count]);
> > 
> > So size of of engine needs to be multiplied by count.
> 
> (Just note this is the what the current VLA evaluates to :)
> 
> > > +}
> > > +
> > > +static struct i915_context_engines_load_balance *
> > > +alloc_load_balance(int count)
> > > +{
> > > +     return calloc(1, sizeof_load_balance(count));
> > 
> > How about alloca so cleanup is simpler? Or is alloca also on the 
> > unpopular list?
> 
> I don't mind. Would shave a few lines indeed, but we need the memsets
> back. #define alloca0()?
> 
> > Or possibly what Simon suggested, just a large temporary stack arrays 
> > would be enough and easiest diff. Just with an assert that it fits.
> 
> I don't think that is as clean for the long term.

I don't understand the motivation here. Can you elaborate?
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* Re: [Intel-gfx] [PATCH i-g-t] benchmarks/gem_wsim: Heap allocate VLA structs
  2019-05-24  8:33   ` Chris Wilson
  2019-05-24  8:39     ` Ser, Simon
@ 2019-05-24  8:44     ` Tvrtko Ursulin
  1 sibling, 0 replies; 13+ messages in thread
From: Tvrtko Ursulin @ 2019-05-24  8:44 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: igt-dev


On 24/05/2019 09:33, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-05-24 09:20:47)
>>
>> On 24/05/2019 08:25, Chris Wilson wrote:
>>> Apparently VLA structs (e.g. struct { int array[count] }) is a gcc
>>> extension that clang refuses to support as handling memory layout is too
>>> difficult for it.
>>>
>>> Move the on-stack VLA to the heap.
>>>
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> ---
>>>    benchmarks/gem_wsim.c | 146 +++++++++++++++++++++++++++---------------
>>>    1 file changed, 95 insertions(+), 51 deletions(-)
>>>
>>> diff --git a/benchmarks/gem_wsim.c b/benchmarks/gem_wsim.c
>>> index e2ffb93a9..0a0032bff 100644
>>> --- a/benchmarks/gem_wsim.c
>>> +++ b/benchmarks/gem_wsim.c
>>> @@ -1441,6 +1441,48 @@ set_ctx_sseu(struct ctx *ctx, uint64_t slice_mask)
>>>        return slice_mask;
>>>    }
>>>    
>>> +static size_t sizeof_load_balance(int count)
>>> +{
>>> +     struct i915_context_engines_load_balance *ptr;
>>> +
>>> +     assert(sizeof(ptr->engines[count]) == count * sizeof(ptr->engines[0]));
>>
>> This seems wrong - is bound to trigger.
> 
> Why does it seem wrong? That's the calculation used previously, and the
> ptr->engines[] was meant to be packed in order for
> sizeof(ptr->engines[count]) == count * sizeof(ptr->engines[0]). Anyway,
> I threw it in there to check if the calculation was sane.

Because sizeof(ptr->engines[0]) == sizeof(ptr->engines[N]), since the 
code is not declaring N big array, just referencing the element N. So 
for more than one engine I expect it explodes. Unless I am way wrong.. I 
guess someone needs to run it.. :)

>>> +     return sizeof(*ptr) + sizeof(ptr->engines[count]);
>>
>> So size of of engine needs to be multiplied by count.
> 
> (Just note this is the what the current VLA evaluates to :)
> 
>>> +}
>>> +
>>> +static struct i915_context_engines_load_balance *
>>> +alloc_load_balance(int count)
>>> +{
>>> +     return calloc(1, sizeof_load_balance(count));
>>
>> How about alloca so cleanup is simpler? Or is alloca also on the
>> unpopular list?
> 
> I don't mind. Would shave a few lines indeed, but we need the memsets
> back. #define alloca0()?

And a helper macro to generically deal with struct header + engines 
array so it doesn't need to be repeated three times. Yadayada too much 
work.. :) ...

>> Or possibly what Simon suggested, just a large temporary stack arrays
>> would be enough and easiest diff. Just with an assert that it fits.
> 
> I don't think that is as clean for the long term.

... this should be just fine for now so I'd vote for it.

Regards,

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

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

* [igt-dev] [PATCH i-g-t v2] benchmarks/gem_wsim: Manually calculate VLA struct sizes
  2019-05-24  7:25 [igt-dev] [PATCH i-g-t] benchmarks/gem_wsim: Heap allocate VLA structs Chris Wilson
                   ` (2 preceding siblings ...)
  2019-05-24  8:20 ` [igt-dev] [Intel-gfx] [PATCH i-g-t] " Tvrtko Ursulin
@ 2019-05-24  8:45 ` Chris Wilson
  2019-05-24  9:35   ` [igt-dev] [Intel-gfx] " Tvrtko Ursulin
  2019-05-24 11:07 ` [igt-dev] ✓ Fi.CI.BAT: success for benchmarks/gem_wsim: Heap allocate VLA structs (rev2) Patchwork
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 13+ messages in thread
From: Chris Wilson @ 2019-05-24  8:45 UTC (permalink / raw)
  To: intel-gfx; +Cc: igt-dev, Tvrtko Ursulin

Apparently VLA structs (e.g. struct { int array[count] }) is a gcc
extension that clang refuses to support as handling memory layout is too
difficult for it. So calculate the size by hand!

v2: Use alloca().

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 benchmarks/gem_wsim.c | 115 +++++++++++++++++++++++-------------------
 1 file changed, 64 insertions(+), 51 deletions(-)

diff --git a/benchmarks/gem_wsim.c b/benchmarks/gem_wsim.c
index e2ffb93a9..db19925b1 100644
--- a/benchmarks/gem_wsim.c
+++ b/benchmarks/gem_wsim.c
@@ -1441,6 +1441,29 @@ set_ctx_sseu(struct ctx *ctx, uint64_t slice_mask)
 	return slice_mask;
 }
 
+static size_t sizeof_load_balance(int count)
+{
+	struct i915_context_engines_load_balance *ptr;
+
+	return sizeof(*ptr) + count * sizeof(ptr->engines[0]);
+}
+
+static size_t sizeof_param_engines(int count)
+{
+	struct i915_context_param_engines *ptr;
+
+	return sizeof(*ptr) + count * sizeof(ptr->engines[0]);
+}
+
+static size_t sizeof_engines_bond(int count)
+{
+	struct i915_context_engines_bond *ptr;
+
+	return sizeof(*ptr) + count * sizeof(ptr->engines[0]);
+}
+
+#define alloca0(sz) ({ size_t sz__ = (sz); memset(alloca(sz__), 0, sz__); })
+
 static int
 prepare_workload(unsigned int id, struct workload *wrk, unsigned int flags)
 {
@@ -1676,66 +1699,54 @@ prepare_workload(unsigned int id, struct workload *wrk, unsigned int flags)
 		}
 
 		if (ctx->engine_map) {
-			I915_DEFINE_CONTEXT_PARAM_ENGINES(set_engines,
-							  ctx->engine_map_count + 1);
-			I915_DEFINE_CONTEXT_ENGINES_LOAD_BALANCE(load_balance,
-								 ctx->engine_map_count);
+			struct i915_context_param_engines *set_engines =
+				alloca0(sizeof_param_engines(ctx->engine_map_count + 1));
+			struct i915_context_engines_load_balance *load_balance =
+				alloca0(sizeof_load_balance(ctx->engine_map_count));
 			struct drm_i915_gem_context_param param = {
 				.ctx_id = ctx_id,
 				.param = I915_CONTEXT_PARAM_ENGINES,
-				.size = sizeof(set_engines),
-				.value = to_user_pointer(&set_engines),
+				.size = sizeof_param_engines(ctx->engine_map_count + 1),
+				.value = to_user_pointer(set_engines),
 			};
+			struct i915_context_engines_bond *last = NULL;
 
 			if (ctx->wants_balance) {
-				set_engines.extensions =
-					to_user_pointer(&load_balance);
+				set_engines->extensions =
+					to_user_pointer(load_balance);
 
-				memset(&load_balance, 0, sizeof(load_balance));
-				load_balance.base.name =
+				load_balance->base.name =
 					I915_CONTEXT_ENGINES_EXT_LOAD_BALANCE;
-				load_balance.num_siblings =
+				load_balance->num_siblings =
 					ctx->engine_map_count;
 
 				for (j = 0; j < ctx->engine_map_count; j++)
-					load_balance.engines[j] =
+					load_balance->engines[j] =
 						get_engine(ctx->engine_map[j]);
-			} else {
-				set_engines.extensions = 0;
 			}
 
 			/* Reserve slot for virtual engine. */
-			set_engines.engines[0].engine_class =
+			set_engines->engines[0].engine_class =
 				I915_ENGINE_CLASS_INVALID;
-			set_engines.engines[0].engine_instance =
+			set_engines->engines[0].engine_instance =
 				I915_ENGINE_CLASS_INVALID_NONE;
 
 			for (j = 1; j <= ctx->engine_map_count; j++)
-				set_engines.engines[j] =
+				set_engines->engines[j] =
 					get_engine(ctx->engine_map[j - 1]);
 
+			last = NULL;
 			for (j = 0; j < ctx->bond_count; j++) {
 				unsigned long mask = ctx->bonds[j].mask;
-				I915_DEFINE_CONTEXT_ENGINES_BOND(bond,
-								 __builtin_popcount(mask));
-				struct i915_context_engines_bond *p = NULL, *prev;
+				struct i915_context_engines_bond *bond =
+					alloca0(sizeof_engines_bond(__builtin_popcount(mask)));
 				unsigned int b, e;
 
-				prev = p;
-				p = alloca(sizeof(bond));
-				assert(p);
-				memset(p, 0, sizeof(bond));
+				bond->base.next_extension = to_user_pointer(last);
+				bond->base.name = I915_CONTEXT_ENGINES_EXT_BOND;
 
-				if (j == 0)
-					load_balance.base.next_extension =
-						to_user_pointer(p);
-				else if (j < (ctx->bond_count - 1))
-					prev->base.next_extension =
-						to_user_pointer(p);
-
-				p->base.name = I915_CONTEXT_ENGINES_EXT_BOND;
-				p->virtual_index = 0;
-				p->master = get_engine(ctx->bonds[j].master);
+				bond->virtual_index = 0;
+				bond->master = get_engine(ctx->bonds[j].master);
 
 				for (b = 0, e = 0; mask; e++, mask >>= 1) {
 					unsigned int idx;
@@ -1743,42 +1754,44 @@ prepare_workload(unsigned int id, struct workload *wrk, unsigned int flags)
 					if (!(mask & 1))
 						continue;
 
-					idx = find_engine(&set_engines.engines[1],
+					idx = find_engine(&set_engines->engines[1],
 							  ctx->engine_map_count,
 							  e);
-					p->engines[b++] =
-						set_engines.engines[1 + idx];
+					bond->engines[b++] =
+						set_engines->engines[1 + idx];
 				}
+
+				last = bond;
 			}
+			load_balance->base.next_extension = to_user_pointer(last);
 
 			gem_context_set_param(fd, &param);
 		} else if (ctx->wants_balance) {
 			const unsigned int count = num_engines_in_class(VCS);
-			I915_DEFINE_CONTEXT_ENGINES_LOAD_BALANCE(load_balance,
-								 count);
-			I915_DEFINE_CONTEXT_PARAM_ENGINES(set_engines,
-							  count + 1);
+			struct i915_context_engines_load_balance *load_balance =
+				alloca0(sizeof_load_balance(count));
+			struct i915_context_param_engines *set_engines =
+				alloca0(sizeof_param_engines(count + 1));
 			struct drm_i915_gem_context_param param = {
 				.ctx_id = ctx_id,
 				.param = I915_CONTEXT_PARAM_ENGINES,
-				.size = sizeof(set_engines),
-				.value = to_user_pointer(&set_engines),
+				.size = sizeof_param_engines(count + 1),
+				.value = to_user_pointer(set_engines),
 			};
 
-			set_engines.extensions = to_user_pointer(&load_balance);
+			set_engines->extensions = to_user_pointer(load_balance);
 
-			set_engines.engines[0].engine_class =
+			set_engines->engines[0].engine_class =
 				I915_ENGINE_CLASS_INVALID;
-			set_engines.engines[0].engine_instance =
+			set_engines->engines[0].engine_instance =
 				I915_ENGINE_CLASS_INVALID_NONE;
-			fill_engines_class(&set_engines.engines[1], VCS);
+			fill_engines_class(&set_engines->engines[1], VCS);
 
-			memset(&load_balance, 0, sizeof(load_balance));
-			load_balance.base.name =
+			load_balance->base.name =
 				I915_CONTEXT_ENGINES_EXT_LOAD_BALANCE;
-			load_balance.num_siblings = count;
+			load_balance->num_siblings = count;
 
-			fill_engines_class(&load_balance.engines[0], VCS);
+			fill_engines_class(&load_balance->engines[0], VCS);
 
 			gem_context_set_param(fd, &param);
 		}
-- 
2.20.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t v2] benchmarks/gem_wsim: Manually calculate VLA struct sizes
  2019-05-24  8:45 ` [igt-dev] [PATCH i-g-t v2] benchmarks/gem_wsim: Manually calculate VLA struct sizes Chris Wilson
@ 2019-05-24  9:35   ` Tvrtko Ursulin
  0 siblings, 0 replies; 13+ messages in thread
From: Tvrtko Ursulin @ 2019-05-24  9:35 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: igt-dev


On 24/05/2019 09:45, Chris Wilson wrote:
> Apparently VLA structs (e.g. struct { int array[count] }) is a gcc
> extension that clang refuses to support as handling memory layout is too
> difficult for it. So calculate the size by hand!
> 
> v2: Use alloca().
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   benchmarks/gem_wsim.c | 115 +++++++++++++++++++++++-------------------
>   1 file changed, 64 insertions(+), 51 deletions(-)
> 
> diff --git a/benchmarks/gem_wsim.c b/benchmarks/gem_wsim.c
> index e2ffb93a9..db19925b1 100644
> --- a/benchmarks/gem_wsim.c
> +++ b/benchmarks/gem_wsim.c
> @@ -1441,6 +1441,29 @@ set_ctx_sseu(struct ctx *ctx, uint64_t slice_mask)
>   	return slice_mask;
>   }
>   
> +static size_t sizeof_load_balance(int count)
> +{
> +	struct i915_context_engines_load_balance *ptr;
> +
> +	return sizeof(*ptr) + count * sizeof(ptr->engines[0]);
> +}
> +
> +static size_t sizeof_param_engines(int count)
> +{
> +	struct i915_context_param_engines *ptr;
> +
> +	return sizeof(*ptr) + count * sizeof(ptr->engines[0]);
> +}
> +
> +static size_t sizeof_engines_bond(int count)
> +{
> +	struct i915_context_engines_bond *ptr;
> +
> +	return sizeof(*ptr) + count * sizeof(ptr->engines[0]);
> +}
> +
> +#define alloca0(sz) ({ size_t sz__ = (sz); memset(alloca(sz__), 0, sz__); })
> +
>   static int
>   prepare_workload(unsigned int id, struct workload *wrk, unsigned int flags)
>   {
> @@ -1676,66 +1699,54 @@ prepare_workload(unsigned int id, struct workload *wrk, unsigned int flags)
>   		}
>   
>   		if (ctx->engine_map) {
> -			I915_DEFINE_CONTEXT_PARAM_ENGINES(set_engines,
> -							  ctx->engine_map_count + 1);
> -			I915_DEFINE_CONTEXT_ENGINES_LOAD_BALANCE(load_balance,
> -								 ctx->engine_map_count);
> +			struct i915_context_param_engines *set_engines =
> +				alloca0(sizeof_param_engines(ctx->engine_map_count + 1));
> +			struct i915_context_engines_load_balance *load_balance =
> +				alloca0(sizeof_load_balance(ctx->engine_map_count));
>   			struct drm_i915_gem_context_param param = {
>   				.ctx_id = ctx_id,
>   				.param = I915_CONTEXT_PARAM_ENGINES,
> -				.size = sizeof(set_engines),
> -				.value = to_user_pointer(&set_engines),
> +				.size = sizeof_param_engines(ctx->engine_map_count + 1),
> +				.value = to_user_pointer(set_engines),
>   			};
> +			struct i915_context_engines_bond *last = NULL;
>   
>   			if (ctx->wants_balance) {
> -				set_engines.extensions =
> -					to_user_pointer(&load_balance);
> +				set_engines->extensions =
> +					to_user_pointer(load_balance);
>   
> -				memset(&load_balance, 0, sizeof(load_balance));
> -				load_balance.base.name =
> +				load_balance->base.name =
>   					I915_CONTEXT_ENGINES_EXT_LOAD_BALANCE;
> -				load_balance.num_siblings =
> +				load_balance->num_siblings =
>   					ctx->engine_map_count;
>   
>   				for (j = 0; j < ctx->engine_map_count; j++)
> -					load_balance.engines[j] =
> +					load_balance->engines[j] =
>   						get_engine(ctx->engine_map[j]);
> -			} else {
> -				set_engines.extensions = 0;
>   			}
>   
>   			/* Reserve slot for virtual engine. */
> -			set_engines.engines[0].engine_class =
> +			set_engines->engines[0].engine_class =
>   				I915_ENGINE_CLASS_INVALID;
> -			set_engines.engines[0].engine_instance =
> +			set_engines->engines[0].engine_instance =
>   				I915_ENGINE_CLASS_INVALID_NONE;
>   
>   			for (j = 1; j <= ctx->engine_map_count; j++)
> -				set_engines.engines[j] =
> +				set_engines->engines[j] =
>   					get_engine(ctx->engine_map[j - 1]);
>   
> +			last = NULL;
>   			for (j = 0; j < ctx->bond_count; j++) {
>   				unsigned long mask = ctx->bonds[j].mask;
> -				I915_DEFINE_CONTEXT_ENGINES_BOND(bond,
> -								 __builtin_popcount(mask));
> -				struct i915_context_engines_bond *p = NULL, *prev;
> +				struct i915_context_engines_bond *bond =
> +					alloca0(sizeof_engines_bond(__builtin_popcount(mask)));
>   				unsigned int b, e;
>   
> -				prev = p;
> -				p = alloca(sizeof(bond));
> -				assert(p);
> -				memset(p, 0, sizeof(bond));
> +				bond->base.next_extension = to_user_pointer(last);
> +				bond->base.name = I915_CONTEXT_ENGINES_EXT_BOND;
>   
> -				if (j == 0)
> -					load_balance.base.next_extension =
> -						to_user_pointer(p);
> -				else if (j < (ctx->bond_count - 1))
> -					prev->base.next_extension =
> -						to_user_pointer(p);
> -
> -				p->base.name = I915_CONTEXT_ENGINES_EXT_BOND;
> -				p->virtual_index = 0;
> -				p->master = get_engine(ctx->bonds[j].master);
> +				bond->virtual_index = 0;
> +				bond->master = get_engine(ctx->bonds[j].master);
>   
>   				for (b = 0, e = 0; mask; e++, mask >>= 1) {
>   					unsigned int idx;
> @@ -1743,42 +1754,44 @@ prepare_workload(unsigned int id, struct workload *wrk, unsigned int flags)
>   					if (!(mask & 1))
>   						continue;
>   
> -					idx = find_engine(&set_engines.engines[1],
> +					idx = find_engine(&set_engines->engines[1],
>   							  ctx->engine_map_count,
>   							  e);
> -					p->engines[b++] =
> -						set_engines.engines[1 + idx];
> +					bond->engines[b++] =
> +						set_engines->engines[1 + idx];
>   				}
> +
> +				last = bond;
>   			}
> +			load_balance->base.next_extension = to_user_pointer(last);
>   
>   			gem_context_set_param(fd, &param);
>   		} else if (ctx->wants_balance) {
>   			const unsigned int count = num_engines_in_class(VCS);
> -			I915_DEFINE_CONTEXT_ENGINES_LOAD_BALANCE(load_balance,
> -								 count);
> -			I915_DEFINE_CONTEXT_PARAM_ENGINES(set_engines,
> -							  count + 1);
> +			struct i915_context_engines_load_balance *load_balance =
> +				alloca0(sizeof_load_balance(count));
> +			struct i915_context_param_engines *set_engines =
> +				alloca0(sizeof_param_engines(count + 1));
>   			struct drm_i915_gem_context_param param = {
>   				.ctx_id = ctx_id,
>   				.param = I915_CONTEXT_PARAM_ENGINES,
> -				.size = sizeof(set_engines),
> -				.value = to_user_pointer(&set_engines),
> +				.size = sizeof_param_engines(count + 1),
> +				.value = to_user_pointer(set_engines),
>   			};
>   
> -			set_engines.extensions = to_user_pointer(&load_balance);
> +			set_engines->extensions = to_user_pointer(load_balance);
>   
> -			set_engines.engines[0].engine_class =
> +			set_engines->engines[0].engine_class =
>   				I915_ENGINE_CLASS_INVALID;
> -			set_engines.engines[0].engine_instance =
> +			set_engines->engines[0].engine_instance =
>   				I915_ENGINE_CLASS_INVALID_NONE;
> -			fill_engines_class(&set_engines.engines[1], VCS);
> +			fill_engines_class(&set_engines->engines[1], VCS);
>   
> -			memset(&load_balance, 0, sizeof(load_balance));
> -			load_balance.base.name =
> +			load_balance->base.name =
>   				I915_CONTEXT_ENGINES_EXT_LOAD_BALANCE;
> -			load_balance.num_siblings = count;
> +			load_balance->num_siblings = count;
>   
> -			fill_engines_class(&load_balance.engines[0], VCS);
> +			fill_engines_class(&load_balance->engines[0], VCS);
>   
>   			gem_context_set_param(fd, &param);
>   		}
> 

Seems correct.

You couldn't resist throwing in the reverse bond list building 
simplification as well. :)

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* [igt-dev] ✓ Fi.CI.BAT: success for benchmarks/gem_wsim: Heap allocate VLA structs (rev2)
  2019-05-24  7:25 [igt-dev] [PATCH i-g-t] benchmarks/gem_wsim: Heap allocate VLA structs Chris Wilson
                   ` (3 preceding siblings ...)
  2019-05-24  8:45 ` [igt-dev] [PATCH i-g-t v2] benchmarks/gem_wsim: Manually calculate VLA struct sizes Chris Wilson
@ 2019-05-24 11:07 ` Patchwork
  2019-05-25 13:11 ` [igt-dev] ✓ Fi.CI.IGT: success for benchmarks/gem_wsim: Heap allocate VLA structs Patchwork
  2019-05-25 16:10 ` [igt-dev] ✓ Fi.CI.IGT: success for benchmarks/gem_wsim: Heap allocate VLA structs (rev2) Patchwork
  6 siblings, 0 replies; 13+ messages in thread
From: Patchwork @ 2019-05-24 11:07 UTC (permalink / raw)
  To: Chris Wilson; +Cc: igt-dev

== Series Details ==

Series: benchmarks/gem_wsim: Heap allocate VLA structs (rev2)
URL   : https://patchwork.freedesktop.org/series/61085/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6140 -> IGTPW_3053
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/61085/revisions/2/mbox/

Known issues
------------

  Here are the changes found in IGTPW_3053 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@userptr:
    - fi-kbl-8809g:       [PASS][1] -> [DMESG-WARN][2] ([fdo#108965])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6140/fi-kbl-8809g/igt@amdgpu/amd_basic@userptr.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3053/fi-kbl-8809g/igt@amdgpu/amd_basic@userptr.html

  * igt@gem_exec_suspend@basic-s3:
    - fi-glk-dsi:         [PASS][3] -> [INCOMPLETE][4] ([fdo#103359] / [k.org#198133])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6140/fi-glk-dsi/igt@gem_exec_suspend@basic-s3.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3053/fi-glk-dsi/igt@gem_exec_suspend@basic-s3.html

  
#### Possible fixes ####

  * igt@gem_ctx_param@basic:
    - {fi-icl-u3}:        [DMESG-WARN][5] ([fdo#107724]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6140/fi-icl-u3/igt@gem_ctx_param@basic.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3053/fi-icl-u3/igt@gem_ctx_param@basic.html

  * igt@gem_exec_suspend@basic-s3:
    - fi-blb-e6850:       [INCOMPLETE][7] ([fdo#107718]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6140/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3053/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html

  * igt@i915_selftest@live_contexts:
    - {fi-icl-dsi}:       [INCOMPLETE][9] ([fdo#107713] / [fdo#108569]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6140/fi-icl-dsi/igt@i915_selftest@live_contexts.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3053/fi-icl-dsi/igt@i915_selftest@live_contexts.html

  * igt@i915_selftest@live_hangcheck:
    - fi-skl-iommu:       [INCOMPLETE][11] ([fdo#108602] / [fdo#108744]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6140/fi-skl-iommu/igt@i915_selftest@live_hangcheck.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3053/fi-skl-iommu/igt@i915_selftest@live_hangcheck.html

  * igt@i915_selftest@live_requests:
    - fi-bsw-n3050:       [INCOMPLETE][13] ([fdo#105876]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6140/fi-bsw-n3050/igt@i915_selftest@live_requests.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3053/fi-bsw-n3050/igt@i915_selftest@live_requests.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#105876]: https://bugs.freedesktop.org/show_bug.cgi?id=105876
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108602]: https://bugs.freedesktop.org/show_bug.cgi?id=108602
  [fdo#108744]: https://bugs.freedesktop.org/show_bug.cgi?id=108744
  [fdo#108965]: https://bugs.freedesktop.org/show_bug.cgi?id=108965
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (52 -> 46)
------------------------------

  Additional (1): fi-apl-guc 
  Missing    (7): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * IGT: IGT_5012 -> IGTPW_3053

  CI_DRM_6140: 0dc04d35a90b3a884e00a31f0c843c433cb5540f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_3053: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3053/
  IGT_5012: 996b4346b6a7f2bd0919817648a4f7a382e59757 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3053/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* [igt-dev] ✓ Fi.CI.IGT: success for benchmarks/gem_wsim: Heap allocate VLA structs
  2019-05-24  7:25 [igt-dev] [PATCH i-g-t] benchmarks/gem_wsim: Heap allocate VLA structs Chris Wilson
                   ` (4 preceding siblings ...)
  2019-05-24 11:07 ` [igt-dev] ✓ Fi.CI.BAT: success for benchmarks/gem_wsim: Heap allocate VLA structs (rev2) Patchwork
@ 2019-05-25 13:11 ` Patchwork
  2019-05-25 16:10 ` [igt-dev] ✓ Fi.CI.IGT: success for benchmarks/gem_wsim: Heap allocate VLA structs (rev2) Patchwork
  6 siblings, 0 replies; 13+ messages in thread
From: Patchwork @ 2019-05-25 13:11 UTC (permalink / raw)
  To: Chris Wilson; +Cc: igt-dev

== Series Details ==

Series: benchmarks/gem_wsim: Heap allocate VLA structs
URL   : https://patchwork.freedesktop.org/series/61085/
State : success

== Summary ==

CI Bug Log - changes from IGT_5012_full -> IGTPW_3051_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/61085/revisions/1/mbox/

Known issues
------------

  Here are the changes found in IGTPW_3051_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_workarounds@suspend-resume-context:
    - shard-apl:          [PASS][1] -> [DMESG-WARN][2] ([fdo#108566]) +5 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5012/shard-apl7/igt@gem_workarounds@suspend-resume-context.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3051/shard-apl6/igt@gem_workarounds@suspend-resume-context.html

  * igt@i915_pm_rps@min-max-config-idle:
    - shard-kbl:          [PASS][3] -> [INCOMPLETE][4] ([fdo#103665])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5012/shard-kbl4/igt@i915_pm_rps@min-max-config-idle.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3051/shard-kbl7/igt@i915_pm_rps@min-max-config-idle.html

  * igt@kms_rotation_crc@multiplane-rotation-cropping-bottom:
    - shard-glk:          [PASS][5] -> [INCOMPLETE][6] ([fdo#103359] / [k.org#198133])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5012/shard-glk2/igt@kms_rotation_crc@multiplane-rotation-cropping-bottom.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3051/shard-glk1/igt@kms_rotation_crc@multiplane-rotation-cropping-bottom.html

  * igt@kms_rotation_crc@primary-rotation-180:
    - shard-kbl:          [PASS][7] -> [DMESG-WARN][8] ([fdo#103558] / [fdo#105602]) +26 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5012/shard-kbl6/igt@kms_rotation_crc@primary-rotation-180.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3051/shard-kbl3/igt@kms_rotation_crc@primary-rotation-180.html

  * igt@kms_setmode@basic:
    - shard-kbl:          [PASS][9] -> [FAIL][10] ([fdo#99912])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5012/shard-kbl1/igt@kms_setmode@basic.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3051/shard-kbl1/igt@kms_setmode@basic.html

  * igt@kms_universal_plane@universal-plane-pipe-a-functional:
    - shard-kbl:          [PASS][11] -> [FAIL][12] ([fdo#110037])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5012/shard-kbl3/igt@kms_universal_plane@universal-plane-pipe-a-functional.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3051/shard-kbl7/igt@kms_universal_plane@universal-plane-pipe-a-functional.html

  * igt@perf_pmu@rc6-runtime-pm-long:
    - shard-apl:          [PASS][13] -> [FAIL][14] ([fdo#105010])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5012/shard-apl1/igt@perf_pmu@rc6-runtime-pm-long.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3051/shard-apl4/igt@perf_pmu@rc6-runtime-pm-long.html
    - shard-glk:          [PASS][15] -> [FAIL][16] ([fdo#105010])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5012/shard-glk9/igt@perf_pmu@rc6-runtime-pm-long.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3051/shard-glk3/igt@perf_pmu@rc6-runtime-pm-long.html
    - shard-kbl:          [PASS][17] -> [FAIL][18] ([fdo#105010])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5012/shard-kbl3/igt@perf_pmu@rc6-runtime-pm-long.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3051/shard-kbl7/igt@perf_pmu@rc6-runtime-pm-long.html

  
#### Possible fixes ####

  * igt@i915_pm_rc6_residency@rc6-accuracy:
    - shard-kbl:          [SKIP][19] ([fdo#109271]) -> [PASS][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5012/shard-kbl6/igt@i915_pm_rc6_residency@rc6-accuracy.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3051/shard-kbl6/igt@i915_pm_rc6_residency@rc6-accuracy.html

  * igt@i915_suspend@fence-restore-tiled2untiled:
    - shard-apl:          [DMESG-WARN][21] ([fdo#108566]) -> [PASS][22] +3 similar issues
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5012/shard-apl5/igt@i915_suspend@fence-restore-tiled2untiled.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3051/shard-apl2/igt@i915_suspend@fence-restore-tiled2untiled.html

  * igt@kms_cursor_crc@pipe-a-cursor-alpha-opaque:
    - shard-kbl:          [FAIL][23] ([fdo#103232]) -> [PASS][24]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5012/shard-kbl1/igt@kms_cursor_crc@pipe-a-cursor-alpha-opaque.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3051/shard-kbl4/igt@kms_cursor_crc@pipe-a-cursor-alpha-opaque.html
    - shard-apl:          [FAIL][25] ([fdo#103232]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5012/shard-apl3/igt@kms_cursor_crc@pipe-a-cursor-alpha-opaque.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3051/shard-apl5/igt@kms_cursor_crc@pipe-a-cursor-alpha-opaque.html

  * igt@kms_flip@modeset-vs-vblank-race-interruptible:
    - shard-apl:          [FAIL][27] ([fdo#103060]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5012/shard-apl7/igt@kms_flip@modeset-vs-vblank-race-interruptible.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3051/shard-apl8/igt@kms_flip@modeset-vs-vblank-race-interruptible.html

  * igt@kms_setmode@basic:
    - shard-apl:          [FAIL][29] ([fdo#99912]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5012/shard-apl3/igt@kms_setmode@basic.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3051/shard-apl4/igt@kms_setmode@basic.html

  
#### Warnings ####

  * igt@kms_atomic_transition@2x-modeset-transitions-nonblocking:
    - shard-kbl:          [SKIP][31] ([fdo#109271]) -> [SKIP][32] ([fdo#105602] / [fdo#109271]) +12 similar issues
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5012/shard-kbl7/igt@kms_atomic_transition@2x-modeset-transitions-nonblocking.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3051/shard-kbl3/igt@kms_atomic_transition@2x-modeset-transitions-nonblocking.html

  * igt@kms_concurrent@pipe-d:
    - shard-kbl:          [SKIP][33] ([fdo#109271] / [fdo#109278]) -> [SKIP][34] ([fdo#105602] / [fdo#109271] / [fdo#109278]) +1 similar issue
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5012/shard-kbl1/igt@kms_concurrent@pipe-d.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3051/shard-kbl3/igt@kms_concurrent@pipe-d.html

  * igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb:
    - shard-kbl:          [FAIL][35] ([fdo#108145]) -> [DMESG-FAIL][36] ([fdo#103558] / [fdo#105602] / [fdo#108145])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5012/shard-kbl7/igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3051/shard-kbl3/igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103060]: https://bugs.freedesktop.org/show_bug.cgi?id=103060
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#105010]: https://bugs.freedesktop.org/show_bug.cgi?id=105010
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#110037]: https://bugs.freedesktop.org/show_bug.cgi?id=110037
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (6 -> 4)
------------------------------

  Missing    (2): shard-skl shard-iclb 


Build changes
-------------

  * IGT: IGT_5012 -> IGTPW_3051

  CI_DRM_6138: a17cee540f7ffd493c7944dea5ea6e2d32fdfdb4 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_3051: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3051/
  IGT_5012: 996b4346b6a7f2bd0919817648a4f7a382e59757 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3051/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* [igt-dev] ✓ Fi.CI.IGT: success for benchmarks/gem_wsim: Heap allocate VLA structs (rev2)
  2019-05-24  7:25 [igt-dev] [PATCH i-g-t] benchmarks/gem_wsim: Heap allocate VLA structs Chris Wilson
                   ` (5 preceding siblings ...)
  2019-05-25 13:11 ` [igt-dev] ✓ Fi.CI.IGT: success for benchmarks/gem_wsim: Heap allocate VLA structs Patchwork
@ 2019-05-25 16:10 ` Patchwork
  6 siblings, 0 replies; 13+ messages in thread
From: Patchwork @ 2019-05-25 16:10 UTC (permalink / raw)
  To: Chris Wilson; +Cc: igt-dev

== Series Details ==

Series: benchmarks/gem_wsim: Heap allocate VLA structs (rev2)
URL   : https://patchwork.freedesktop.org/series/61085/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6140_full -> IGTPW_3053_full
====================================================

Summary
-------

  **WARNING**

  Minor unknown changes coming with IGTPW_3053_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_3053_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/61085/revisions/2/mbox/

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in IGTPW_3053_full:

### IGT changes ###

#### Warnings ####

  * igt@gem_tiled_swapping@non-threaded:
    - shard-hsw:          [INCOMPLETE][1] ([fdo#103540]) -> [TIMEOUT][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6140/shard-hsw8/igt@gem_tiled_swapping@non-threaded.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3053/shard-hsw7/igt@gem_tiled_swapping@non-threaded.html

  * igt@prime_vgem@busy-bsd1:
    - shard-snb:          [INCOMPLETE][3] ([fdo#105411]) -> [FAIL][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6140/shard-snb4/igt@prime_vgem@busy-bsd1.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3053/shard-snb2/igt@prime_vgem@busy-bsd1.html

  
Known issues
------------

  Here are the changes found in IGTPW_3053_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_tiled_swapping@non-threaded:
    - shard-iclb:         [PASS][5] -> [FAIL][6] ([fdo#108686])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6140/shard-iclb7/igt@gem_tiled_swapping@non-threaded.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3053/shard-iclb6/igt@gem_tiled_swapping@non-threaded.html

  * igt@gem_workarounds@suspend-resume:
    - shard-apl:          [PASS][7] -> [DMESG-WARN][8] ([fdo#108566]) +7 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6140/shard-apl4/igt@gem_workarounds@suspend-resume.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3053/shard-apl3/igt@gem_workarounds@suspend-resume.html

  * igt@i915_pm_rc6_residency@rc6-accuracy:
    - shard-snb:          [PASS][9] -> [SKIP][10] ([fdo#109271])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6140/shard-snb4/igt@i915_pm_rc6_residency@rc6-accuracy.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3053/shard-snb1/igt@i915_pm_rc6_residency@rc6-accuracy.html

  * igt@kms_color@pipe-b-legacy-gamma:
    - shard-glk:          [PASS][11] -> [FAIL][12] ([fdo#104782])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6140/shard-glk2/igt@kms_color@pipe-b-legacy-gamma.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3053/shard-glk3/igt@kms_color@pipe-b-legacy-gamma.html
    - shard-apl:          [PASS][13] -> [FAIL][14] ([fdo#104782])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6140/shard-apl6/igt@kms_color@pipe-b-legacy-gamma.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3053/shard-apl1/igt@kms_color@pipe-b-legacy-gamma.html
    - shard-kbl:          [PASS][15] -> [FAIL][16] ([fdo#104782])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6140/shard-kbl7/igt@kms_color@pipe-b-legacy-gamma.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3053/shard-kbl4/igt@kms_color@pipe-b-legacy-gamma.html

  * igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy:
    - shard-hsw:          [PASS][17] -> [FAIL][18] ([fdo#105767])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6140/shard-hsw7/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3053/shard-hsw6/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy.html

  * igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic:
    - shard-hsw:          [PASS][19] -> [SKIP][20] ([fdo#109271]) +12 similar issues
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6140/shard-hsw7/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3053/shard-hsw1/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic.html

  * igt@kms_cursor_legacy@cursor-vs-flip-atomic-transitions:
    - shard-hsw:          [PASS][21] -> [FAIL][22] ([fdo#103355])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6140/shard-hsw2/igt@kms_cursor_legacy@cursor-vs-flip-atomic-transitions.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3053/shard-hsw6/igt@kms_cursor_legacy@cursor-vs-flip-atomic-transitions.html

  * igt@kms_dp_dsc@basic-dsc-enable-edp:
    - shard-iclb:         [PASS][23] -> [SKIP][24] ([fdo#109349])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6140/shard-iclb2/igt@kms_dp_dsc@basic-dsc-enable-edp.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3053/shard-iclb1/igt@kms_dp_dsc@basic-dsc-enable-edp.html

  * igt@kms_flip@flip-vs-suspend:
    - shard-hsw:          [PASS][25] -> [INCOMPLETE][26] ([fdo#103540]) +1 similar issue
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6140/shard-hsw7/igt@kms_flip@flip-vs-suspend.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3053/shard-hsw2/igt@kms_flip@flip-vs-suspend.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render:
    - shard-iclb:         [PASS][27] -> [FAIL][28] ([fdo#103167]) +2 similar issues
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6140/shard-iclb1/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3053/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-fullscreen:
    - shard-kbl:          [PASS][29] -> [FAIL][30] ([fdo#103167])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6140/shard-kbl7/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-fullscreen.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3053/shard-kbl3/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-fullscreen.html

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-draw-pwrite:
    - shard-glk:          [PASS][31] -> [FAIL][32] ([fdo#103167])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6140/shard-glk5/igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-draw-pwrite.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3053/shard-glk1/igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-draw-pwrite.html

  * igt@kms_psr@no_drrs:
    - shard-iclb:         [PASS][33] -> [FAIL][34] ([fdo#108341])
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6140/shard-iclb6/igt@kms_psr@no_drrs.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3053/shard-iclb1/igt@kms_psr@no_drrs.html

  * igt@kms_psr@psr2_basic:
    - shard-iclb:         [PASS][35] -> [SKIP][36] ([fdo#109441]) +2 similar issues
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6140/shard-iclb2/igt@kms_psr@psr2_basic.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3053/shard-iclb7/igt@kms_psr@psr2_basic.html

  
#### Possible fixes ####

  * igt@i915_suspend@fence-restore-tiled2untiled:
    - shard-apl:          [DMESG-WARN][37] ([fdo#108566]) -> [PASS][38] +2 similar issues
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6140/shard-apl4/igt@i915_suspend@fence-restore-tiled2untiled.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3053/shard-apl2/igt@i915_suspend@fence-restore-tiled2untiled.html

  * igt@kms_cursor_crc@pipe-a-cursor-alpha-opaque:
    - shard-kbl:          [FAIL][39] ([fdo#103232]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6140/shard-kbl1/igt@kms_cursor_crc@pipe-a-cursor-alpha-opaque.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3053/shard-kbl3/igt@kms_cursor_crc@pipe-a-cursor-alpha-opaque.html

  * igt@kms_cursor_crc@pipe-b-cursor-64x21-sliding:
    - shard-apl:          [FAIL][41] ([fdo#103232]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6140/shard-apl5/igt@kms_cursor_crc@pipe-b-cursor-64x21-sliding.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3053/shard-apl8/igt@kms_cursor_crc@pipe-b-cursor-64x21-sliding.html

  * igt@kms_cursor_legacy@2x-nonblocking-modeset-vs-cursor-atomic:
    - shard-glk:          [FAIL][43] ([fdo#106509] / [fdo#107409]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6140/shard-glk6/igt@kms_cursor_legacy@2x-nonblocking-modeset-vs-cursor-atomic.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3053/shard-glk1/igt@kms_cursor_legacy@2x-nonblocking-modeset-vs-cursor-atomic.html

  * igt@kms_flip@2x-plain-flip:
    - shard-hsw:          [SKIP][45] ([fdo#109271]) -> [PASS][46] +27 similar issues
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6140/shard-hsw1/igt@kms_flip@2x-plain-flip.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3053/shard-hsw4/igt@kms_flip@2x-plain-flip.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-pri-indfb-multidraw:
    - shard-iclb:         [FAIL][47] ([fdo#103167]) -> [PASS][48] +5 similar issues
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6140/shard-iclb8/igt@kms_frontbuffer_tracking@fbcpsr-1p-pri-indfb-multidraw.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3053/shard-iclb3/igt@kms_frontbuffer_tracking@fbcpsr-1p-pri-indfb-multidraw.html

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
    - shard-glk:          [INCOMPLETE][49] ([fdo#103359] / [k.org#198133]) -> [PASS][50]
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6140/shard-glk3/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3053/shard-glk3/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html

  * igt@kms_plane_lowres@pipe-a-tiling-x:
    - shard-iclb:         [FAIL][51] ([fdo#103166]) -> [PASS][52] +1 similar issue
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6140/shard-iclb8/igt@kms_plane_lowres@pipe-a-tiling-x.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3053/shard-iclb3/igt@kms_plane_lowres@pipe-a-tiling-x.html

  * igt@kms_psr@psr2_cursor_plane_onoff:
    - shard-iclb:         [SKIP][53] ([fdo#109441]) -> [PASS][54] +2 similar issues
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6140/shard-iclb6/igt@kms_psr@psr2_cursor_plane_onoff.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3053/shard-iclb2/igt@kms_psr@psr2_cursor_plane_onoff.html

  
#### Warnings ####

  * igt@gem_mmap_gtt@forked-big-copy-odd:
    - shard-iclb:         [TIMEOUT][55] ([fdo#109673]) -> [INCOMPLETE][56] ([fdo#107713] / [fdo#109100])
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6140/shard-iclb3/igt@gem_mmap_gtt@forked-big-copy-odd.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3053/shard-iclb1/igt@gem_mmap_gtt@forked-big-copy-odd.html

  * igt@prime_vgem@wait-bsd1:
    - shard-snb:          [FAIL][57] -> [INCOMPLETE][58] ([fdo#105411])
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6140/shard-snb1/igt@prime_vgem@wait-bsd1.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3053/shard-snb1/igt@prime_vgem@wait-bsd1.html

  
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103355]: https://bugs.freedesktop.org/show_bug.cgi?id=103355
  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
  [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
  [fdo#105767]: https://bugs.freedesktop.org/show_bug.cgi?id=105767
  [fdo#106509]: https://bugs.freedesktop.org/show_bug.cgi?id=106509
  [fdo#107409]: https://bugs.freedesktop.org/show_bug.cgi?id=107409
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108341]: https://bugs.freedesktop.org/show_bug.cgi?id=108341
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
  [fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109349]: https://bugs.freedesktop.org/show_bug.cgi?id=109349
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109673]: https://bugs.freedesktop.org/show_bug.cgi?id=109673
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (10 -> 6)
------------------------------

  Missing    (4): pig-skl-6260u shard-skl pig-hsw-4770r pig-glk-j5005 


Build changes
-------------

  * IGT: IGT_5012 -> IGTPW_3053
  * Piglit: piglit_4509 -> None

  CI_DRM_6140: 0dc04d35a90b3a884e00a31f0c843c433cb5540f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_3053: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3053/
  IGT_5012: 996b4346b6a7f2bd0919817648a4f7a382e59757 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3053/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

end of thread, other threads:[~2019-05-25 16:10 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-05-24  7:25 [igt-dev] [PATCH i-g-t] benchmarks/gem_wsim: Heap allocate VLA structs Chris Wilson
2019-05-24  7:45 ` Ser, Simon
2019-05-24  7:50 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork
2019-05-24  8:20 ` [igt-dev] [Intel-gfx] [PATCH i-g-t] " Tvrtko Ursulin
2019-05-24  8:27   ` Ser, Simon
2019-05-24  8:33   ` Chris Wilson
2019-05-24  8:39     ` Ser, Simon
2019-05-24  8:44     ` Tvrtko Ursulin
2019-05-24  8:45 ` [igt-dev] [PATCH i-g-t v2] benchmarks/gem_wsim: Manually calculate VLA struct sizes Chris Wilson
2019-05-24  9:35   ` [igt-dev] [Intel-gfx] " Tvrtko Ursulin
2019-05-24 11:07 ` [igt-dev] ✓ Fi.CI.BAT: success for benchmarks/gem_wsim: Heap allocate VLA structs (rev2) Patchwork
2019-05-25 13:11 ` [igt-dev] ✓ Fi.CI.IGT: success for benchmarks/gem_wsim: Heap allocate VLA structs Patchwork
2019-05-25 16:10 ` [igt-dev] ✓ Fi.CI.IGT: success for benchmarks/gem_wsim: Heap allocate VLA structs (rev2) Patchwork

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