Re: [Intel-gfx] [PATCH 06/11] drm/i915/pvc: Reduce stack usage in reset selftest with extra blitter engine

On 02/05/2022 17:34, Matt Roper wrote:

From: John Harrison John.C.Harrison@Intel.com

PVC adds extra blitter engines (in the following patch). The reset selftest has a local array on the stack which is sized by the number of engines. The increase pushes the size of this array to the point where it trips the 'stack too large' compile warning. This patch takes the allocation of the stack and makes it dynamic instead.

Signed-off-by: John Harrison John.C.Harrison@Intel.com Signed-off-by: Matt Roper matthew.d.roper@intel.com

---

drivers/gpu/drm/i915/gt/selftest_hangcheck.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c index 83ff4c2e57c5..3b9d82276db2 100644 --- a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c +++ b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c @@ -979,6 +979,7 @@ static int __igt_reset_engines(struct intel_gt *gt, enum intel_engine_id id, tmp; struct hang h; int err = 0;

• struct active_engine *threads;

Drive by nits - sticks out like a sore thumb a bit here - I'd put it above "id, tmp" so it's all nicely sorted by width.

/* Check that issuing a reset on one engine does not interfere * with any other engine. @@ -996,8 +997,11 @@ static int __igt_reset_engines(struct intel_gt *gt, h.ctx->sched.priority = 1024; }

• threads = kzalloc(sizeof(*threads) * I915_NUM_ENGINES, GFP_KERNEL);

And this could be kcalloc (or kmalloc_array since zeroing is not needed) if that's any better. Seems selftests use that pattern anyway.

Both comments are optional really.

Regards,

Tvrtko

• if (!threads)

• return -ENOMEM;
  

• for_each_engine(engine, gt, id) {

• struct active_engine threads[I915_NUM_ENGINES] = {};
  

  unsigned long device = i915_reset_count(global); unsigned long count = 0, reported; bool using_guc = intel_engine_uses_guc(engine);

@@ -1016,7 +1020,7 @@ static int __igt_reset_engines(struct intel_gt *gt, break; }

• memset(threads, 0, sizeof(threads));
  

• memset(threads, 0, sizeof(*threads) * I915_NUM_ENGINES);
  

  for_each_engine(other, gt, tmp) { struct task_struct *tsk;

@@ -1236,6 +1240,7 @@ static int __igt_reset_engines(struct intel_gt *gt, break; } }

• kfree(threads);

  if (intel_gt_is_wedged(gt)) err = -EIO;