| Message ID | 20191120093302.3723715-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/9] drm/i915/selftests: Take a ref to the request we wait upon | expand |
On Wed, 20 Nov 2019 at 09:33, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > i915_request_add() consumes the passed in reference to the i915_request, > so if the selftest caller wishes to wait upon it afterwards, it needs to > take a reference for itself. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > .../drm/i915/gem/selftests/i915_gem_context.c | 38 ++++++++++++++----- > 1 file changed, 29 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c > index 9a509c18b7c7..16df814f3efd 100644 > --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c > @@ -73,25 +73,34 @@ static int live_nop_switch(void *arg) > } > > for_each_uabi_engine(engine, i915) { > - struct i915_request *rq; > + struct i915_request *rq = NULL; > unsigned long end_time, prime; > ktime_t times[2] = {}; > > times[0] = ktime_get_raw(); > for (n = 0; n < nctx; n++) { > - rq = igt_request_alloc(ctx[n], engine); > - if (IS_ERR(rq)) { > - err = PTR_ERR(rq); > + struct i915_request *this; > + > + this = igt_request_alloc(ctx[n], engine); > + if (IS_ERR(this)) { > + err = PTR_ERR(this); > goto out_file; > } > - i915_request_add(rq); > + if (rq) { > + i915_request_await_dma_fence(this, &rq->fence); > + i915_request_put(rq); > + } > + rq = i915_request_get(this); > + i915_request_add(this); > } > if (i915_request_wait(rq, 0, HZ / 5) < 0) { > pr_err("Failed to populated %d contexts\n", nctx); > intel_gt_set_wedged(&i915->gt); > + i915_request_put(rq); > err = -EIO; > goto out_file; > } > + i915_request_put(rq); > > times[1] = ktime_get_raw(); > > @@ -106,13 +115,21 @@ static int live_nop_switch(void *arg) > for_each_prime_number_from(prime, 2, 8192) { > times[1] = ktime_get_raw(); > > + rq = NULL; > for (n = 0; n < prime; n++) { > - rq = igt_request_alloc(ctx[n % nctx], engine); > - if (IS_ERR(rq)) { > - err = PTR_ERR(rq); > + struct i915_request *this; > + > + this = igt_request_alloc(ctx[n % nctx], engine); > + if (IS_ERR(this)) { > + err = PTR_ERR(this); > goto out_file; > } > > + if (this) { /* Force submission order */ if (rq) ? > + i915_request_await_dma_fence(this, &rq->fence); > + i915_request_put(rq); > + } > + > /* > * This space is left intentionally blank. > * > @@ -127,14 +144,17 @@ static int live_nop_switch(void *arg) > * for latency. > */ > > - i915_request_add(rq); rq = i915_request_get(this) ?
Quoting Matthew Auld (2019-11-20 10:19:56) > On Wed, 20 Nov 2019 at 09:33, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > > > i915_request_add() consumes the passed in reference to the i915_request, > > so if the selftest caller wishes to wait upon it afterwards, it needs to > > take a reference for itself. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > --- > > .../drm/i915/gem/selftests/i915_gem_context.c | 38 ++++++++++++++----- > > 1 file changed, 29 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c > > index 9a509c18b7c7..16df814f3efd 100644 > > --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c > > +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c > > @@ -73,25 +73,34 @@ static int live_nop_switch(void *arg) > > } > > > > for_each_uabi_engine(engine, i915) { > > - struct i915_request *rq; > > + struct i915_request *rq = NULL; > > unsigned long end_time, prime; > > ktime_t times[2] = {}; > > > > times[0] = ktime_get_raw(); > > for (n = 0; n < nctx; n++) { > > - rq = igt_request_alloc(ctx[n], engine); > > - if (IS_ERR(rq)) { > > - err = PTR_ERR(rq); > > + struct i915_request *this; > > + > > + this = igt_request_alloc(ctx[n], engine); > > + if (IS_ERR(this)) { > > + err = PTR_ERR(this); > > goto out_file; > > } > > - i915_request_add(rq); > > + if (rq) { > > + i915_request_await_dma_fence(this, &rq->fence); > > + i915_request_put(rq); > > + } > > + rq = i915_request_get(this); > > + i915_request_add(this); > > } > > if (i915_request_wait(rq, 0, HZ / 5) < 0) { > > pr_err("Failed to populated %d contexts\n", nctx); > > intel_gt_set_wedged(&i915->gt); > > + i915_request_put(rq); > > err = -EIO; > > goto out_file; > > } > > + i915_request_put(rq); > > > > times[1] = ktime_get_raw(); > > > > @@ -106,13 +115,21 @@ static int live_nop_switch(void *arg) > > for_each_prime_number_from(prime, 2, 8192) { > > times[1] = ktime_get_raw(); > > > > + rq = NULL; > > for (n = 0; n < prime; n++) { > > - rq = igt_request_alloc(ctx[n % nctx], engine); > > - if (IS_ERR(rq)) { > > - err = PTR_ERR(rq); > > + struct i915_request *this; > > + > > + this = igt_request_alloc(ctx[n % nctx], engine); > > + if (IS_ERR(this)) { > > + err = PTR_ERR(this); > > goto out_file; > > } > > > > + if (this) { /* Force submission order */ > > if (rq) ? Yes. Still distinct lack of coffee. -Chris
Quoting Patchwork (2019-11-20 14:19:15) > == Series Details == > > Series: series starting with drm/i915/selftests: Take a ref to the request we wait upon (rev2) > URL : https://patchwork.freedesktop.org/series/69724/ > State : failure > > == Summary == > > CI Bug Log - changes from CI_DRM_7384 -> Patchwork_15342 > ==================================================== > > Summary > ------- > > **FAILURE** > > Serious unknown changes coming with Patchwork_15342 absolutely need to be > verified manually. > > If you think the reported changes have nothing to do with the changes > introduced in Patchwork_15342, please notify your bug team to allow them > to document this new failure mode, which will reduce false positives in CI. > > External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15342/index.html > > Possible new issues > ------------------- > > Here are the unknown changes that may have been introduced in Patchwork_15342: > > ### IGT changes ### > > #### Possible regressions #### > > * igt@i915_selftest@live_gt_heartbeat: > - fi-kbl-soraka: [PASS][1] -> [INCOMPLETE][2] > [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7384/fi-kbl-soraka/igt@i915_selftest@live_gt_heartbeat.html > [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15342/fi-kbl-soraka/igt@i915_selftest@live_gt_heartbeat.html > > * igt@runner@aborted: > - fi-kbl-soraka: NOTRUN -> [FAIL][3] > [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15342/fi-kbl-soraka/igt@runner@aborted.html Here's where gitlab's /tableflip would be so handy. -Chris
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c index 9a509c18b7c7..16df814f3efd 100644 --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c @@ -73,25 +73,34 @@ static int live_nop_switch(void *arg) } for_each_uabi_engine(engine, i915) { - struct i915_request *rq; + struct i915_request *rq = NULL; unsigned long end_time, prime; ktime_t times[2] = {}; times[0] = ktime_get_raw(); for (n = 0; n < nctx; n++) { - rq = igt_request_alloc(ctx[n], engine); - if (IS_ERR(rq)) { - err = PTR_ERR(rq); + struct i915_request *this; + + this = igt_request_alloc(ctx[n], engine); + if (IS_ERR(this)) { + err = PTR_ERR(this); goto out_file; } - i915_request_add(rq); + if (rq) { + i915_request_await_dma_fence(this, &rq->fence); + i915_request_put(rq); + } + rq = i915_request_get(this); + i915_request_add(this); } if (i915_request_wait(rq, 0, HZ / 5) < 0) { pr_err("Failed to populated %d contexts\n", nctx); intel_gt_set_wedged(&i915->gt); + i915_request_put(rq); err = -EIO; goto out_file; } + i915_request_put(rq); times[1] = ktime_get_raw(); @@ -106,13 +115,21 @@ static int live_nop_switch(void *arg) for_each_prime_number_from(prime, 2, 8192) { times[1] = ktime_get_raw(); + rq = NULL; for (n = 0; n < prime; n++) { - rq = igt_request_alloc(ctx[n % nctx], engine); - if (IS_ERR(rq)) { - err = PTR_ERR(rq); + struct i915_request *this; + + this = igt_request_alloc(ctx[n % nctx], engine); + if (IS_ERR(this)) { + err = PTR_ERR(this); goto out_file; } + if (this) { /* Force submission order */ + i915_request_await_dma_fence(this, &rq->fence); + i915_request_put(rq); + } + /* * This space is left intentionally blank. * @@ -127,14 +144,17 @@ static int live_nop_switch(void *arg) * for latency. */ - i915_request_add(rq); + i915_request_add(this); } + GEM_BUG_ON(!rq); if (i915_request_wait(rq, 0, HZ / 5) < 0) { pr_err("Switching between %ld contexts timed out\n", prime); intel_gt_set_wedged(&i915->gt); + i915_request_put(rq); break; } + i915_request_put(rq); times[1] = ktime_sub(ktime_get_raw(), times[1]); if (prime == 2)
i915_request_add() consumes the passed in reference to the i915_request, so if the selftest caller wishes to wait upon it afterwards, it needs to take a reference for itself. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- .../drm/i915/gem/selftests/i915_gem_context.c | 38 ++++++++++++++----- 1 file changed, 29 insertions(+), 9 deletions(-)