| Message ID | 20220502163417.2635462-7-matthew.d.roper@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | i915: Introduce Ponte Vecchio | expand |
On Mon, 2022-05-02 at 09:34 -0700, 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. Reviewed-by: José Roberto de Souza <jose.souza@intel.com> > > 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; > > /* 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); > + 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;
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;
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; /* 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); + 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;