On Thu, Jul 01, 2021 at 01:23:40PM -0700, Matt Roper wrote:
From: John Harrison <John.C.Harrison@xxxxxxxxx> Increasing the engine count causes a couple of local array variables to exceed the kernel stack limit. So make them dynamic allocations instead. Signed-off-by: John Harrison <John.C.Harrison@xxxxxxxxx> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx> Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx> --- drivers/gpu/drm/i915/gt/selftest_execlists.c | 10 ++++-- .../gpu/drm/i915/gt/selftest_workarounds.c | 32 ++++++++++++------- 2 files changed, 29 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/selftest_execlists.c b/drivers/gpu/drm/i915/gt/selftest_execlists.c index 08896ae027d5..1e7fe2222479 100644 --- a/drivers/gpu/drm/i915/gt/selftest_execlists.c +++ b/drivers/gpu/drm/i915/gt/selftest_execlists.c @@ -3561,12 +3561,16 @@ static int smoke_crescendo(struct preempt_smoke *smoke, unsigned int flags) #define BATCH BIT(0) { struct task_struct *tsk[I915_NUM_ENGINES] = {}; - struct preempt_smoke arg[I915_NUM_ENGINES]; + struct preempt_smoke *arg; struct intel_engine_cs *engine; enum intel_engine_id id; unsigned long count; int err = 0; + arg = kmalloc_array(I915_NUM_ENGINES, sizeof(*arg), GFP_KERNEL); + if (!arg) + return -ENOMEM; + for_each_engine(engine, smoke->gt, id) { arg[id] = *smoke; arg[id].engine = engine; @@ -3574,7 +3578,7 @@ static int smoke_crescendo(struct preempt_smoke *smoke, unsigned int flags) arg[id].batch = NULL; arg[id].count = 0; - tsk[id] = kthread_run(smoke_crescendo_thread, &arg, + tsk[id] = kthread_run(smoke_crescendo_thread, arg, "igt/smoke:%d", id); if (IS_ERR(tsk[id])) { err = PTR_ERR(tsk[id]); @@ -3603,6 +3607,8 @@ static int smoke_crescendo(struct preempt_smoke *smoke, unsigned int flags) pr_info("Submitted %lu crescendo:%x requests across %d engines and %d contexts\n", count, flags, smoke->gt->info.num_engines, smoke->ncontext); + + kfree(arg); return 0;
this looks correctly, but apparently this test doesn't test anything as `err` is write-only - there is only one read, but basically to avoid overriding an earlier error. looks like this should be `return err;` ? +Chris This patch itself looks good. Reviewed-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx> Lucas De Marchi