Re: [Intel-gfx] [PATCH 04/54] drm/i915: Mark up protected uses of 'i915_request_completed'

[Andi Shyti <andi.shyti@intel.com>]

Hi Chris,

> When we know that we are inside the timeline mutex, or inside the
> submission flow (under active.lock or the holder's rcu lock), we know
> that the rq->hwsp is stable and we can use the simpler direct version.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_context.c     |  2 +-
>  drivers/gpu/drm/i915/gt/intel_breadcrumbs.c     |  4 ++--
>  drivers/gpu/drm/i915/gt/intel_engine_cs.c       |  2 +-
>  drivers/gpu/drm/i915/gt/intel_reset.c           |  3 +--
>  drivers/gpu/drm/i915/gt/intel_ring_submission.c |  4 +++-
>  drivers/gpu/drm/i915/gt/intel_timeline.c        |  4 ++--
>  drivers/gpu/drm/i915/i915_request.c             | 15 +++++++--------
>  7 files changed, 17 insertions(+), 17 deletions(-)

have you also missed a couple of cases in
intel_execlists_submission.c and i915_scheduler.c?

[...]

> diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
> index b85b6f3dcd60..e0b4291393ec 100644
> --- a/drivers/gpu/drm/i915/gt/intel_reset.c
> +++ b/drivers/gpu/drm/i915/gt/intel_reset.c
> @@ -151,8 +151,7 @@ static void mark_innocent(struct i915_request *rq)
>  void __i915_request_reset(struct i915_request *rq, bool guilty)
>  {
>  	RQ_TRACE(rq, "guilty? %s\n", yesno(guilty));
> -
> -	GEM_BUG_ON(i915_request_completed(rq));
> +	GEM_BUG_ON(__i915_request_is_complete(rq));

aren't you outside the lock here?

Andi

>  
>  	rcu_read_lock(); /* protect the GEM context */
>  	if (guilty) {
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx