Re: [PATCH v2 12/15] drm/i915: Replace reset_wait_queue with default_wake_function

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 22/02/2017 11:46, Chris Wilson wrote:
> If we change the wait_queue_t from using the autoremove_wake_function to
> the default_wake_function, we no longer have to restore the wait_queue_t
> entry on the wait_queue_head_t list after being woken up by it, as we
> are unusual in sleeping multiple times on the same wait_queue_t.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_gem_request.c | 30 +++++++-----------------------
>  1 file changed, 7 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
> index 97116e492d01..42250eaf56ec 100644
> --- a/drivers/gpu/drm/i915/i915_gem_request.c
> +++ b/drivers/gpu/drm/i915/i915_gem_request.c
> @@ -946,16 +946,6 @@ void __i915_add_request(struct drm_i915_gem_request *request, bool flush_caches)
>  	local_bh_enable(); /* Kick the execlists tasklet if just scheduled */
>  }
>
> -static void reset_wait_queue(wait_queue_head_t *q, wait_queue_t *wait)
> -{
> -	unsigned long flags;
> -
> -	spin_lock_irqsave(&q->lock, flags);
> -	if (list_empty(&wait->task_list))
> -		__add_wait_queue(q, wait);
> -	spin_unlock_irqrestore(&q->lock, flags);
> -}
> -
>  static unsigned long local_clock_us(unsigned int *cpu)
>  {
>  	unsigned long t;
> @@ -1060,8 +1050,8 @@ long i915_wait_request(struct drm_i915_gem_request *req,
>  	const int state = flags & I915_WAIT_INTERRUPTIBLE ?
>  		TASK_INTERRUPTIBLE : TASK_UNINTERRUPTIBLE;
>  	wait_queue_head_t *errq = &req->i915->gpu_error.wait_queue;
> -	DEFINE_WAIT(reset);
> -	DEFINE_WAIT(exec);
> +	DEFINE_WAIT_FUNC(reset, default_wake_function);
> +	DEFINE_WAIT_FUNC(exec, default_wake_function);
>  	struct intel_wait wait;
>
>  	might_sleep();
> @@ -1080,13 +1070,13 @@ long i915_wait_request(struct drm_i915_gem_request *req,
>
>  	trace_i915_gem_request_wait_begin(req, flags);
>
> +	add_wait_queue(&req->execute, &exec);
>  	if (flags & I915_WAIT_LOCKED)
>  		add_wait_queue(errq, &reset);
>
>  	wait.tsk = current;
>
>  restart:
> -	reset_wait_queue(&req->execute, &exec);
>  	wait.seqno = i915_gem_request_global_seqno(req);
>  	if (!wait.seqno) {
>  		do {
> @@ -1100,26 +1090,21 @@ long i915_wait_request(struct drm_i915_gem_request *req,
>  			    i915_reset_in_progress(&req->i915->gpu_error)) {
>  				__set_current_state(TASK_RUNNING);
>  				i915_reset(req->i915);
> -				reset_wait_queue(errq, &reset);
>  				continue;
>  			}
>
>  			if (signal_pending_state(state, current)) {
>  				timeout = -ERESTARTSYS;
> -				break;
> +				goto complete;
>  			}
>
>  			if (!timeout) {
>  				timeout = -ETIME;
> -				break;
> +				goto complete;
>  			}
>
>  			timeout = io_schedule_timeout(timeout);
>  		} while (1);
> -		finish_wait(&req->execute, &exec);
> -
> -		if (timeout < 0)
> -			goto complete;
>
>  		GEM_BUG_ON(!wait.seqno);
>  	}
> @@ -1179,7 +1164,6 @@ long i915_wait_request(struct drm_i915_gem_request *req,
>  		    i915_reset_in_progress(&req->i915->gpu_error)) {
>  			__set_current_state(TASK_RUNNING);
>  			i915_reset(req->i915);
> -			reset_wait_queue(errq, &reset);
>  			continue;
>  		}
>
> @@ -1194,11 +1178,11 @@ long i915_wait_request(struct drm_i915_gem_request *req,
>  	}
>
>  	intel_engine_remove_wait(req->engine, &wait);
> -	__set_current_state(TASK_RUNNING);
> -
>  complete:
> +	__set_current_state(TASK_RUNNING);
>  	if (flags & I915_WAIT_LOCKED)
>  		remove_wait_queue(errq, &reset);
> +	remove_wait_queue(&req->execute, &exec);
>  	trace_i915_gem_request_wait_end(req);
>
>  	return timeout;
>

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx