Re: [PATCH 07/42] drm/i915: Allow i915_sw_fence_await_sw_fence() to allocate

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

On 07/10/2016 17:22, Chris Wilson wrote:
> On Fri, Oct 07, 2016 at 05:10:04PM +0100, Tvrtko Ursulin wrote:
>> On 07/10/2016 10:46, Chris Wilson wrote:
>>> In forthcoming patches, we want to be able to dynamically allocate the
>>> wait_queue_t used whilst awaiting. This is more convenient if we extend
>>> the i915_sw_fence_await_sw_fence() to perform the allocation for us if
>>> we pass in a gfp mask as an alternative than a preallocated struct.
>>>
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/i915_sw_fence.c | 40 ++++++++++++++++++++++++++++++++----
>>>   drivers/gpu/drm/i915/i915_sw_fence.h |  8 ++++++++
>>>   2 files changed, 44 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c b/drivers/gpu/drm/i915/i915_sw_fence.c
>>> index 1e5cbc585ca2..dfb59dced0ce 100644
>>> --- a/drivers/gpu/drm/i915/i915_sw_fence.c
>>> +++ b/drivers/gpu/drm/i915/i915_sw_fence.c
>>> @@ -13,6 +13,8 @@
>>>   #include "i915_sw_fence.h"
>>> +#define I915_SW_FENCE_FLAG_ALLOC BIT(0)
>>> +
>> You collide with WQ_FLAG_EXCLUSIVE here, so I assume i915 will have
>> complete control of the wq in question?
> Hmm, we should move bit as well, but yes the flag is atm private to the
> wake up function for which we use a custom function.
>
>>> +		wq = kmalloc(sizeof(*wq), gfp);
>>> +		if (!wq) {
>>> +			if (!gfpflags_allow_blocking(gfp))
>>> +				return -ENOMEM;
>>> +
>>> +			i915_sw_fence_wait(signaler);
>> This looks like a strange API, unless I am missing something - it
>> normally sets things up for waiting, unless the allocation fails
>> when it actually does the wait?
> Yes. If we blocked on the malloc and failed, we block a little more for
> the completion. iirc, the idea came from the async task.

Still sounds strange to me. I understand it is a unlikely corner case 
but sounds like that sort of decision should happen in the caller. I 
will have to revisit the i915_sw_fence implementation and usage in more 
detail it seems.

Regards,

Tvrtko

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