[Intel-gfx] [PATCH 3/7] drm/i915: Keep a count of requests submitted from userspace

[Tvrtko Ursulin]

On 06/04/2018 21:17, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-04-05 13:39:19)
>> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>>
>> Keep a count of requests submitted from userspace and not yet runnable due
>> unresolved dependencies.
>>
>> v2: Rename and move under the container struct. (Chris Wilson)
>> v3: Rebase.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_request.c     | 3 +++
>>   drivers/gpu/drm/i915/intel_engine_cs.c  | 3 ++-
>>   drivers/gpu/drm/i915/intel_ringbuffer.h | 8 ++++++++
>>   3 files changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
>> index 5c01291ad1cc..152321655fe6 100644
>> --- a/drivers/gpu/drm/i915/i915_request.c
>> +++ b/drivers/gpu/drm/i915/i915_request.c
>> @@ -640,6 +640,7 @@ submit_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
>>                  rcu_read_lock();
>>                  request->engine->submit_request(request);
>>                  rcu_read_unlock();
>> +               atomic_dec(&request->engine->request_stats.queued);
> 
> But we use atomic here? Might as well use atomic for
> request_stats.runnable here as well?

I admit it can read a bit uneven.

For runnable I wanted to avoid another atomic by putting it under the 
engine timeline lock.

But for queued I did not want to start taking the same lock when adding 
a request.

Your proposal to make runnable atomic_t and move to submit_notify would 
indeed simplify things, but at a cost of one more atomic in that path. 
Perhaps the code path is heavy enough for one new atomic to be 
completely hidden in it, and code simplification to win?

Regards,

Tvrtko