Re: [Intel-gfx] [PATCH 15/28] drm/i915: Add lock for unbinding to i915_gem_object_ggtt_pin_ww

[Maarten Lankhorst <maarten.lankhorst@linux.intel.com>]

On 21-10-2021 19:48, Matthew Auld wrote:
> On Thu, 21 Oct 2021 at 11:37, Maarten Lankhorst
> <maarten.lankhorst@linux.intel.com> wrote:
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Needs a proper commit message.

What about this?


>> ---
>>  drivers/gpu/drm/i915/i915_gem.c | 9 ++++++++-
>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>> index 981e383d1a5d..6aa9e465b48e 100644
>> --- a/drivers/gpu/drm/i915/i915_gem.c
>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>> @@ -931,7 +931,14 @@ i915_gem_object_ggtt_pin_ww(struct drm_i915_gem_object *obj,
>>                         goto new_vma;
>>                 }
>>
>> -               ret = i915_vma_unbind(vma);
>> +               ret = 0;
>> +               if (!ww)
>> +                       ret = i915_gem_object_lock_interruptible(obj, NULL);
>> +               if (!ret) {
>> +                       ret = i915_vma_unbind(vma);
>> +                       if (!ww)
>> +                               i915_gem_object_unlock(obj);
>> +               }
> There is also a wait_for_bind below. Do we need the lock for that also?

Hmm good find.

Not sure if required in this patch series. I have some patches on top that require the lock because of async binding / unbinding, that would need it for sure.

I will fix this function to use WARN_ON(!ww), and add ww handling to i915_gem_object_ggtt_pin(). That should fix all issues without special casing !ww.

~Maarten