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

Arkadiusz Hiler arkadiusz.hiler at intel.com
Tue Jun 4 12:59:52 UTC 2019


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

Cc: Chris Wilson <chris at chris-wilson.co.uk>
Cc: Simon Ser <simon.ser at intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler at 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



More information about the igt-dev mailing list