* [igt-dev] [PATCH i-g-t] benchmarks/gem_wsim: Heap allocate VLA structs
@ 2019-05-24 7:25 ` Chris Wilson
0 siblings, 0 replies; 22+ 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, ¶m);
+
+ 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, ¶m);
+
+ 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] 22+ messages in thread* Re: [igt-dev] [PATCH i-g-t] benchmarks/gem_wsim: Heap allocate VLA structs
2019-05-24 7:25 ` [igt-dev] " Chris Wilson
@ 2019-05-24 7:45 ` Ser, Simon
-1 siblings, 0 replies; 22+ messages in thread
From: Ser, Simon @ 2019-05-24 7:45 UTC (permalink / raw)
To: intel-gfx, chris; +Cc: igt-dev
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, ¶m);
> +
> + 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, ¶m);
> +
> + free(set_engines);
> + free(load_balance);
> }
>
> if (wrk->sseu) {
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [igt-dev] [PATCH i-g-t] benchmarks/gem_wsim: Heap allocate VLA structs
@ 2019-05-24 7:45 ` Ser, Simon
0 siblings, 0 replies; 22+ messages in thread
From: Ser, Simon @ 2019-05-24 7:45 UTC (permalink / raw)
To: intel-gfx, chris; +Cc: igt-dev, 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, ¶m);
> +
> + 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, ¶m);
> +
> + 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] 22+ messages in thread
* [igt-dev] ✓ Fi.CI.BAT: success for benchmarks/gem_wsim: Heap allocate VLA structs
2019-05-24 7:25 ` [igt-dev] " Chris Wilson
(?)
(?)
@ 2019-05-24 7:50 ` Patchwork
-1 siblings, 0 replies; 22+ 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] 22+ messages in thread* Re: [PATCH i-g-t] benchmarks/gem_wsim: Heap allocate VLA structs
2019-05-24 7:25 ` [igt-dev] " Chris Wilson
@ 2019-05-24 8:20 ` Tvrtko Ursulin
-1 siblings, 0 replies; 22+ 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, ¶m);
> +
> + 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, ¶m);
> +
> + free(set_engines);
> + free(load_balance);
> }
>
> if (wrk->sseu) {
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] benchmarks/gem_wsim: Heap allocate VLA structs
@ 2019-05-24 8:20 ` Tvrtko Ursulin
0 siblings, 0 replies; 22+ 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, ¶m);
> +
> + 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, ¶m);
> +
> + 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] 22+ messages in thread* Re: [igt-dev] [PATCH i-g-t] benchmarks/gem_wsim: Heap allocate VLA structs
2019-05-24 8:20 ` [igt-dev] [Intel-gfx] " Tvrtko Ursulin
@ 2019-05-24 8:27 ` Ser, Simon
-1 siblings, 0 replies; 22+ messages in thread
From: Ser, Simon @ 2019-05-24 8:27 UTC (permalink / raw)
To: tvrtko.ursulin, intel-gfx, chris; +Cc: igt-dev
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, ¶m);
> > +
> > + 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, ¶m);
> > +
> > + 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] benchmarks/gem_wsim: Heap allocate VLA structs
@ 2019-05-24 8:27 ` Ser, Simon
0 siblings, 0 replies; 22+ messages in thread
From: Ser, Simon @ 2019-05-24 8:27 UTC (permalink / raw)
To: tvrtko.ursulin, intel-gfx, chris; +Cc: igt-dev
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, ¶m);
> > +
> > + 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, ¶m);
> > +
> > + 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] 22+ messages in thread
* Re: [PATCH i-g-t] benchmarks/gem_wsim: Heap allocate VLA structs
2019-05-24 8:20 ` [igt-dev] [Intel-gfx] " Tvrtko Ursulin
@ 2019-05-24 8:33 ` Chris Wilson
-1 siblings, 0 replies; 22+ 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ 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
0 siblings, 0 replies; 22+ 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] 22+ messages in thread* Re: [igt-dev] [PATCH i-g-t] benchmarks/gem_wsim: Heap allocate VLA structs
2019-05-24 8:33 ` [igt-dev] [Intel-gfx] " Chris Wilson
@ 2019-05-24 8:39 ` Ser, Simon
-1 siblings, 0 replies; 22+ messages in thread
From: Ser, Simon @ 2019-05-24 8:39 UTC (permalink / raw)
To: tvrtko.ursulin, intel-gfx, chris; +Cc: igt-dev
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?
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] benchmarks/gem_wsim: Heap allocate VLA structs
@ 2019-05-24 8:39 ` Ser, Simon
0 siblings, 0 replies; 22+ messages in thread
From: Ser, Simon @ 2019-05-24 8:39 UTC (permalink / raw)
To: tvrtko.ursulin, intel-gfx, chris; +Cc: igt-dev
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] 22+ messages in thread
* Re: [PATCH i-g-t] benchmarks/gem_wsim: Heap allocate VLA structs
2019-05-24 8:33 ` [igt-dev] [Intel-gfx] " Chris Wilson
@ 2019-05-24 8:44 ` Tvrtko Ursulin
-1 siblings, 0 replies; 22+ 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] 22+ messages in thread* Re: [Intel-gfx] [PATCH i-g-t] benchmarks/gem_wsim: Heap allocate VLA structs
@ 2019-05-24 8:44 ` Tvrtko Ursulin
0 siblings, 0 replies; 22+ 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] 22+ messages in thread
* [PATCH i-g-t v2] benchmarks/gem_wsim: Manually calculate VLA struct sizes
2019-05-24 7:25 ` [igt-dev] " Chris Wilson
@ 2019-05-24 8:45 ` Chris Wilson
-1 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2019-05-24 8:45 UTC (permalink / raw)
To: intel-gfx; +Cc: igt-dev
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, ¶m);
} 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, ¶m);
}
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 22+ messages in thread* [igt-dev] [PATCH i-g-t v2] benchmarks/gem_wsim: Manually calculate VLA struct sizes
@ 2019-05-24 8:45 ` Chris Wilson
0 siblings, 0 replies; 22+ 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, ¶m);
} 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, ¶m);
}
--
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] 22+ messages in thread* Re: [PATCH i-g-t v2] benchmarks/gem_wsim: Manually calculate VLA struct sizes
2019-05-24 8:45 ` [igt-dev] " Chris Wilson
@ 2019-05-24 9:35 ` Tvrtko Ursulin
-1 siblings, 0 replies; 22+ 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, ¶m);
> } 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, ¶m);
> }
>
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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t v2] benchmarks/gem_wsim: Manually calculate VLA struct sizes
@ 2019-05-24 9:35 ` Tvrtko Ursulin
0 siblings, 0 replies; 22+ 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, ¶m);
> } 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, ¶m);
> }
>
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] 22+ 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] " Chris Wilson
` (4 preceding siblings ...)
(?)
@ 2019-05-24 11:07 ` Patchwork
-1 siblings, 0 replies; 22+ 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] 22+ messages in thread* [igt-dev] ✓ Fi.CI.IGT: success for benchmarks/gem_wsim: Heap allocate VLA structs
2019-05-24 7:25 ` [igt-dev] " Chris Wilson
` (5 preceding siblings ...)
(?)
@ 2019-05-25 13:11 ` Patchwork
-1 siblings, 0 replies; 22+ 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] 22+ 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] " Chris Wilson
` (6 preceding siblings ...)
(?)
@ 2019-05-25 16:10 ` Patchwork
-1 siblings, 0 replies; 22+ 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] 22+ messages in thread