[PATCH 13/29] drm/armada: Use a private mutex to protect priv->linear
Daniel Vetter
daniel.vetter at ffwll.ch
Mon Nov 23 12:17:25 PST 2015
On Mon, Nov 23, 2015 at 6:40 PM, Russell King - ARM Linux
<linux at arm.linux.org.uk> wrote:
> On Mon, Nov 23, 2015 at 10:32:46AM +0100, Daniel Vetter wrote:
>> diff --git a/drivers/gpu/drm/armada/armada_gem.c b/drivers/gpu/drm/armada/armada_gem.c
>> index e3a86b96af2a..a43690ab18b9 100644
>> --- a/drivers/gpu/drm/armada/armada_gem.c
>> +++ b/drivers/gpu/drm/armada/armada_gem.c
>> @@ -46,7 +46,7 @@ static size_t roundup_gem_size(size_t size)
>> return roundup(size, PAGE_SIZE);
>> }
>>
>> -/* dev->struct_mutex is held here */
>> +/* dev_priv->linear_lock is held here */
>> void armada_gem_free_object(struct drm_gem_object *obj)
>
> This is wrong (unless there's changes which I'm not aware of.)
>
> This function is called from drm_gem_object_free(), which is called from
> drm_gem_object_unreference(), both of which contain:
>
> WARN_ON(!mutex_is_locked(&dev->struct_mutex));
>
> Now, unless you're changing the conditions on which
> drm_gem_object_unreference() to be under dev_priv->linear_lock, the
> above comment becomes misleading.
>
> It'd also need drm_gem_object_unreference_unlocked() to become per-
> driver, so it can take dev_priv->linear_lock, and I don't see anything
> in the patches which I've received which does that.
>
> So, I suspect the new comment is basically false.
Well the patch is false. I should have grabbed the new
dev_priv->linear_lock around the cleanup functions in
armada_gem_free_object. Which is possible since armada never calls an
gem unref function while holding the new lock. Sorry for not
double-checking what I've done, I'll revise.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
More information about the dri-devel
mailing list