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

Maarten Lankhorst
2021-11-29T13:25:58Z

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