[igt-dev] [PATCH i-g-t 1/4] tests/gem_exec_balancer: Manually calculate VLA struct sizes

Arkadiusz Hiler
2019-06-04T13:00:01Z

VLA in structs (struct { int array[count] }) is a GCC extension, so
let's avoid using it.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Simon Ser <simon.ser@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
---
tests/i915/gem_exec_balancer.c | 230 ++++++++++++++++++---------------
1 file changed, 128 insertions(+), 102 deletions(-)

diff --git a/tests/i915/gem_exec_balancer.c b/tests/i915/gem_exec_balancer.c
index 33b17fc5..c1c0a2f7 100644
--- a/tests/i915/gem_exec_balancer.c
+++ b/tests/i915/gem_exec_balancer.c
@@ -32,6 +32,26 @@ IGT_TEST_DESCRIPTION("Exercise in-kernel load-balancing");

#define INSTANCE_COUNT (1 << I915_PMU_SAMPLE_INSTANCE_BITS)

+static size_t sizeof_load_balance(int count)
+{
+ return offsetof(struct i915_context_engines_load_balance,
+ engines[count]);
+}
+
+static size_t sizeof_param_engines(int count)
+{
+ return offsetof(struct i915_context_param_engines,
+ engines[count]);
+}
+
+static size_t sizeof_engines_bond(int count)
+{
+ return offsetof(struct i915_context_engines_bond,
+ engines[count]);
+}
+
+#define alloca0(sz) ({ size_t sz__ = (sz); memset(alloca(sz__), 0, sz__); })
+
static bool has_class_instance(int i915, uint16_t class, uint16_t instance)
{
int fd;
@@ -93,16 +113,17 @@ static int __set_engines(int i915, uint32_t ctx,
const struct i915_engine_class_instance *ci,
unsigned int count)
{
- I915_DEFINE_CONTEXT_PARAM_ENGINES(engines, count);
+ struct i915_context_param_engines *engines =
+ alloca0(sizeof_param_engines(count));
struct drm_i915_gem_context_param p = {
.ctx_id = ctx,
.param = I915_CONTEXT_PARAM_ENGINES,
- .size = sizeof(engines),
- .value = to_user_pointer(&engines)
+ .size = sizeof_param_engines(count),
+ .value = to_user_pointer(engines)
};

- engines.extensions = 0;
- memcpy(engines.engines, ci, sizeof(engines.engines));
+ engines->extensions = 0;
+ memcpy(engines->engines, ci, sizeof(engines->engines));

return __gem_context_set_param(i915, &p);
}
@@ -119,30 +140,30 @@ static int __set_load_balancer(int i915, uint32_t ctx,
unsigned int count,
void *ext)
{
- I915_DEFINE_CONTEXT_ENGINES_LOAD_BALANCE(balancer, count);
- I915_DEFINE_CONTEXT_PARAM_ENGINES(engines, 1 + count);
+ struct i915_context_engines_load_balance *balancer =
+ alloca0(sizeof_load_balance(count));
+ struct i915_context_param_engines *engines =
+ alloca0(sizeof_param_engines(count + 1));
struct drm_i915_gem_context_param p = {
.ctx_id = ctx,
.param = I915_CONTEXT_PARAM_ENGINES,
- .size = sizeof(engines),
- .value = to_user_pointer(&engines)
+ .size = sizeof_param_engines(count + 1),
+ .value = to_user_pointer(engines)
};

- memset(&balancer, 0, sizeof(balancer));
- balancer.base.name = I915_CONTEXT_ENGINES_EXT_LOAD_BALANCE;
- balancer.base.next_extension = to_user_pointer(ext);
+ balancer->base.name = I915_CONTEXT_ENGINES_EXT_LOAD_BALANCE;
+ balancer->base.next_extension = to_user_pointer(ext);

igt_assert(count);
- balancer.num_siblings = count;
- memcpy(balancer.engines, ci, count * sizeof(*ci));
+ balancer->num_siblings = count;
+ memcpy(balancer->engines, ci, count * sizeof(*ci));

- memset(&engines, 0, sizeof(engines));
- engines.extensions = to_user_pointer(&balancer);
- engines.engines[0].engine_class =
+ engines->extensions = to_user_pointer(balancer);
+ engines->engines[0].engine_class =
I915_ENGINE_CLASS_INVALID;
- engines.engines[0].engine_instance =
+ engines->engines[0].engine_instance =
I915_ENGINE_CLASS_INVALID_NONE;
- memcpy(engines.engines + 1, ci, count * sizeof(*ci));
+ memcpy(engines->engines + 1, ci, count * sizeof(*ci));

return __gem_context_set_param(i915, &p);
}
@@ -185,11 +206,13 @@ static uint32_t batch_create(int i915)

static void invalid_balancer(int i915)
{
- I915_DEFINE_CONTEXT_ENGINES_LOAD_BALANCE(balancer, 64);
- I915_DEFINE_CONTEXT_PARAM_ENGINES(engines, 64);
+ struct i915_context_engines_load_balance *balancer =
+ alloca0(sizeof_load_balance(64));
+ struct i915_context_param_engines *engines =
+ alloca0(sizeof_param_engines(64));
struct drm_i915_gem_context_param p = {
.param = I915_CONTEXT_PARAM_ENGINES,
- .value = to_user_pointer(&engines)
+ .value = to_user_pointer(engines)
};
uint32_t handle;
void *ptr;
@@ -212,91 +235,89 @@ static void invalid_balancer(int i915)

p.ctx_id = gem_context_create(i915);
p.size = (sizeof(struct i915_context_param_engines) +
- (count + 1) * sizeof(*engines.engines));
+ (count + 1) * sizeof(*engines->engines));

- memset(&engines, 0, sizeof(engines));
- engines.engines[0].engine_class = I915_ENGINE_CLASS_INVALID;
- engines.engines[0].engine_instance = I915_ENGINE_CLASS_INVALID_NONE;
- memcpy(engines.engines + 1, ci, count * sizeof(*ci));
+ engines->engines[0].engine_class = I915_ENGINE_CLASS_INVALID;
+ engines->engines[0].engine_instance = I915_ENGINE_CLASS_INVALID_NONE;
+ memcpy(engines->engines + 1, ci, count * sizeof(*ci));
gem_context_set_param(i915, &p);

- engines.extensions = -1ull;
+ engines->extensions = -1ull;
igt_assert_eq(__gem_context_set_param(i915, &p), -EFAULT);

- engines.extensions = 1ull;
+ engines->extensions = 1ull;
igt_assert_eq(__gem_context_set_param(i915, &p), -EFAULT);

- memset(&balancer, 0, sizeof(balancer));
- balancer.base.name = I915_CONTEXT_ENGINES_EXT_LOAD_BALANCE;
- balancer.num_siblings = count;
- memcpy(balancer.engines, ci, count * sizeof(*ci));
+ balancer->base.name = I915_CONTEXT_ENGINES_EXT_LOAD_BALANCE;
+ balancer->num_siblings = count;
+ memcpy(balancer->engines, ci, count * sizeof(*ci));

- engines.extensions = to_user_pointer(&balancer);
+ engines->extensions = to_user_pointer(balancer);
gem_context_set_param(i915, &p);

- balancer.engine_index = 1;
+ balancer->engine_index = 1;
igt_assert_eq(__gem_context_set_param(i915, &p), -EEXIST);

- balancer.engine_index = count;
+ balancer->engine_index = count;
igt_assert_eq(__gem_context_set_param(i915, &p), -EEXIST);

- balancer.engine_index = count + 1;
+ balancer->engine_index = count + 1;
igt_assert_eq(__gem_context_set_param(i915, &p), -EINVAL);

- balancer.engine_index = 0;
+ balancer->engine_index = 0;
gem_context_set_param(i915, &p);

- balancer.base.next_extension = to_user_pointer(&balancer);
+ balancer->base.next_extension = to_user_pointer(balancer);
igt_assert_eq(__gem_context_set_param(i915, &p), -EEXIST);

- balancer.base.next_extension = -1ull;
+ balancer->base.next_extension = -1ull;
igt_assert_eq(__gem_context_set_param(i915, &p), -EFAULT);

handle = gem_create(i915, 4096 * 3);
ptr = gem_mmap__gtt(i915, handle, 4096 * 3, PROT_WRITE);
gem_close(i915, handle);

- memset(&engines, 0, sizeof(engines));
- engines.engines[0].engine_class = I915_ENGINE_CLASS_INVALID;
- engines.engines[0].engine_instance = I915_ENGINE_CLASS_INVALID_NONE;
- engines.engines[1].engine_class = I915_ENGINE_CLASS_INVALID;
- engines.engines[1].engine_instance = I915_ENGINE_CLASS_INVALID_NONE;
- memcpy(engines.engines + 2, ci, count * sizeof(ci));
+ memset(engines, 0, sizeof_param_engines(64));
+ engines->engines[0].engine_class = I915_ENGINE_CLASS_INVALID;
+ engines->engines[0].engine_instance = I915_ENGINE_CLASS_INVALID_NONE;
+ engines->engines[1].engine_class = I915_ENGINE_CLASS_INVALID;
+ engines->engines[1].engine_instance = I915_ENGINE_CLASS_INVALID_NONE;
+ memcpy(engines->engines + 2, ci, count * sizeof(ci));
p.size = (sizeof(struct i915_context_param_engines) +
- (count + 2) * sizeof(*engines.engines));
+ (count + 2) * sizeof(*engines->engines));
gem_context_set_param(i915, &p);

- balancer.base.next_extension = 0;
- balancer.engine_index = 1;
- engines.extensions = to_user_pointer(&balancer);
+ balancer->base.next_extension = 0;
+ balancer->engine_index = 1;
+ engines->extensions = to_user_pointer(balancer);
gem_context_set_param(i915, &p);

- memcpy(ptr + 4096 - 8, &balancer, sizeof(balancer));
- memcpy(ptr + 8192 - 8, &balancer, sizeof(balancer));
- balancer.engine_index = 0;
+ memcpy(ptr + 4096 - 8, balancer, sizeof_load_balance(64));
+ memcpy(ptr + 8192 - 8, balancer, sizeof_load_balance(64));
+ balancer->engine_index = 0;

- engines.extensions = to_user_pointer(ptr) + 4096 - 8;
+ engines->extensions = to_user_pointer(ptr) + 4096 - 8;
gem_context_set_param(i915, &p);

- balancer.base.next_extension = engines.extensions;
- engines.extensions = to_user_pointer(&balancer);
+ balancer->base.next_extension = engines->extensions;
+ engines->extensions = to_user_pointer(balancer);
gem_context_set_param(i915, &p);

munmap(ptr, 4096);
igt_assert_eq(__gem_context_set_param(i915, &p), -EFAULT);
- engines.extensions = to_user_pointer(ptr) + 4096 - 8;
+ engines->extensions = to_user_pointer(ptr) + 4096 - 8;
igt_assert_eq(__gem_context_set_param(i915, &p), -EFAULT);

- engines.extensions = to_user_pointer(ptr) + 8192 - 8;
+ engines->extensions = to_user_pointer(ptr) + 8192 - 8;
gem_context_set_param(i915, &p);

- balancer.base.next_extension = engines.extensions;
- engines.extensions = to_user_pointer(&balancer);
+ balancer->base.next_extension = engines->extensions;
+ engines->extensions = to_user_pointer(balancer);
gem_context_set_param(i915, &p);

munmap(ptr + 8192, 4096);
igt_assert_eq(__gem_context_set_param(i915, &p), -EFAULT);
- engines.extensions = to_user_pointer(ptr) + 8192 - 8;
+ engines->extensions = to_user_pointer(ptr) + 8192 - 8;
igt_assert_eq(__gem_context_set_param(i915, &p), -EFAULT);

munmap(ptr + 4096, 4096);
@@ -308,61 +329,64 @@ static void invalid_balancer(int i915)

static void invalid_bonds(int i915)
{
- I915_DEFINE_CONTEXT_ENGINES_BOND(bonds[16], 1);
- I915_DEFINE_CONTEXT_PARAM_ENGINES(engines, 1);
+ struct i915_context_engines_bond *bonds[16];
+ struct i915_context_param_engines *engines =
+ alloca0(sizeof_param_engines(1));
+
struct drm_i915_gem_context_param p = {
.ctx_id = gem_context_create(i915),
.param = I915_CONTEXT_PARAM_ENGINES,
- .value = to_user_pointer(&engines),
- .size = sizeof(engines),
+ .value = to_user_pointer(engines),
+ .size = sizeof_param_engines(1),
};
uint32_t handle;
void *ptr;

- memset(&engines, 0, sizeof(engines));
+ const size_t bond_size = sizeof_engines_bond(1);
+
gem_context_set_param(i915, &p);

- memset(bonds, 0, sizeof(bonds));
for (int n = 0; n < ARRAY_SIZE(bonds); n++) {
- bonds[n].base.name = I915_CONTEXT_ENGINES_EXT_BOND;
- bonds[n].base.next_extension =
- n ? to_user_pointer(&bonds[n - 1]) : 0;
- bonds[n].num_bonds = 1;
+ bonds[n] = alloca0(bond_size);
+ bonds[n]->base.name = I915_CONTEXT_ENGINES_EXT_BOND;
+ bonds[n]->base.next_extension =
+ n ? to_user_pointer(bonds[n - 1]) : 0;
+ bonds[n]->num_bonds = 1;
}
- engines.extensions = to_user_pointer(&bonds);
+ engines->extensions = to_user_pointer(bonds);
gem_context_set_param(i915, &p);

- bonds[0].base.next_extension = -1ull;
+ bonds[0]->base.next_extension = -1ull;
igt_assert_eq(__gem_context_set_param(i915, &p), -EFAULT);

- bonds[0].base.next_extension = to_user_pointer(&bonds[0]);
+ bonds[0]->base.next_extension = to_user_pointer(bonds[0]);
igt_assert_eq(__gem_context_set_param(i915, &p), -E2BIG);

- engines.extensions = to_user_pointer(&bonds[1]);
+ engines->extensions = to_user_pointer(bonds[1]);
igt_assert_eq(__gem_context_set_param(i915, &p), -E2BIG);
- bonds[0].base.next_extension = 0;
+ bonds[0]->base.next_extension = 0;
gem_context_set_param(i915, &p);

handle = gem_create(i915, 4096 * 3);
ptr = gem_mmap__gtt(i915, handle, 4096 * 3, PROT_WRITE);
gem_close(i915, handle);

- memcpy(ptr + 4096, &bonds[0], sizeof(bonds[0]));
- engines.extensions = to_user_pointer(ptr) + 4096;
+ memcpy(ptr + 4096, bonds[0], bond_size);
+ engines->extensions = to_user_pointer(ptr) + 4096;
gem_context_set_param(i915, &p);

- memcpy(ptr, &bonds[0], sizeof(bonds[0]));
- bonds[0].base.next_extension = to_user_pointer(ptr);
- memcpy(ptr + 4096, &bonds[0], sizeof(bonds[0]));
+ memcpy(ptr, bonds[0], bond_size);
+ bonds[0]->base.next_extension = to_user_pointer(ptr);
+ memcpy(ptr + 4096, bonds[0], bond_size);
gem_context_set_param(i915, &p);

munmap(ptr, 4096);
igt_assert_eq(__gem_context_set_param(i915, &p), -EFAULT);

- bonds[0].base.next_extension = 0;
- memcpy(ptr + 8192, &bonds[0], sizeof(bonds[0]));
- bonds[0].base.next_extension = to_user_pointer(ptr) + 8192;
- memcpy(ptr + 4096, &bonds[0], sizeof(bonds[0]));
+ bonds[0]->base.next_extension = 0;
+ memcpy(ptr + 8192, bonds[0], bond_size);
+ bonds[0]->base.next_extension = to_user_pointer(ptr) + 8192;
+ memcpy(ptr + 4096, bonds[0], bond_size);
gem_context_set_param(i915, &p);

munmap(ptr + 8192, 4096);
@@ -543,10 +567,12 @@ static void individual(int i915)
static void bonded(int i915, unsigned int flags)
#define CORK 0x1
{
- I915_DEFINE_CONTEXT_ENGINES_BOND(bonds[16], 1);
+ struct i915_context_engines_bond *bonds[16];
struct i915_engine_class_instance *master_engines;
uint32_t master;

+ size_t bond_size = sizeof_engines_bond(1);
+
/*
* I915_CONTEXT_PARAM_ENGINE provides an extension that allows us
* to specify which engine(s) to pair with a parallel (EXEC_SUBMIT)
@@ -555,12 +581,12 @@ static void bonded(int i915, unsigned int flags)

master = gem_queue_create(i915);

- memset(bonds, 0, sizeof(bonds));
for (int n = 0; n < ARRAY_SIZE(bonds); n++) {
- bonds[n].base.name = I915_CONTEXT_ENGINES_EXT_BOND;
- bonds[n].base.next_extension =
- n ? to_user_pointer(&bonds[n - 1]) : 0;
- bonds[n].num_bonds = 1;
+ bonds[n] = alloca0(bond_size);
+ bonds[n]->base.name = I915_CONTEXT_ENGINES_EXT_BOND;
+ bonds[n]->base.next_extension =
+ n ? to_user_pointer(bonds[n - 1]) : 0;
+ bonds[n]->num_bonds = 1;
}

for (int class = 0; class < 32; class++) {
@@ -584,14 +610,14 @@ static void bonded(int i915, unsigned int flags)
limit = min(count, limit);
igt_assert(limit <= ARRAY_SIZE(bonds));
for (n = 0; n < limit; n++) {
- bonds[n].master = master_engines[n];
- bonds[n].engines[0] = siblings[n];
+ bonds[n]->master = master_engines[n];
+ bonds[n]->engines[0] = siblings[n];
}

ctx = gem_context_clone(i915,
master, I915_CONTEXT_CLONE_VM,
I915_CONTEXT_CREATE_FLAGS_SINGLE_TIMELINE);
- set_load_balancer(i915, ctx, siblings, count, &bonds[limit - 1]);
+ set_load_balancer(i915, ctx, siblings, count, bonds[limit - 1]);

order = malloc(sizeof(*order) * 8 * limit);
igt_assert(order);
@@ -681,11 +707,12 @@ static void bonded(int i915, unsigned int flags)

static void indices(int i915)
{
- I915_DEFINE_CONTEXT_PARAM_ENGINES(engines, I915_EXEC_RING_MASK + 1);
+ struct i915_context_param_engines *engines =
+ alloca0(sizeof_param_engines(I915_EXEC_RING_MASK + 1));
struct drm_i915_gem_context_param p = {
.ctx_id = gem_context_create(i915),
.param = I915_CONTEXT_PARAM_ENGINES,
- .value = to_user_pointer(&engines)
+ .value = to_user_pointer(engines)
};

struct drm_i915_gem_exec_object2 batch = {
@@ -709,15 +736,14 @@ static void indices(int i915)
continue;

for (int n = 0; n < count; n++) {
- I915_DEFINE_CONTEXT_ENGINES_LOAD_BALANCE(*balancer,
- count);
+ struct i915_context_engines_load_balance *balancer;

- engines.engines[nengines].engine_class =
+ engines->engines[nengines].engine_class =
I915_ENGINE_CLASS_INVALID;
- engines.engines[nengines].engine_instance =
+ engines->engines[nengines].engine_instance =
I915_ENGINE_CLASS_INVALID_NONE;

- balancer = calloc(sizeof(*balancer), 1);
+ balancer = calloc(sizeof_load_balance(1), 1);
igt_assert(balancer);

balancer->base.name =
@@ -736,7 +762,7 @@ static void indices(int i915)
}

igt_require(balancers);
- engines.extensions = to_user_pointer(balancers);
+ engines->extensions = to_user_pointer(balancers);
p.size = (sizeof(struct i915_engine_class_instance) * nengines +
sizeof(struct i915_context_param_engines));
gem_context_set_param(i915, &p);
--
2.21.0