On Wed, Mar 28, 2018 at 11:52 AM, Tomi Valkeinen <tomi.valkei...@ti.com> wrote:
> Hi,
>
> On 27/03/18 11:23, Daniel Vetter wrote:
>> The only thing that omap_gem_free_object does that might need the
>> magic protection of struct_mutex (of keeping all objects alive if that
>> lock is held, even if the last reference is gone) is the mm_list
>> manipulation.
>>
>> But that is already protected by the separate omapdrm->list_lock,
>> which means struct_mutex is an entirely internal lock for omapdrm.
>> Everything else is just releasing resources, which is all protected
>> already by the various subsystems and allocators.
>>
>> To make this even more obvious we could do an
>> s/dev->struct_mutex/omapdrm->gem_lock/ like I've done for udl. But
>> since omapdrm is a lot bigger and a lot more active I'll refrain from
>> that - this is better done by omapdrm developers at some suitable time
>> in the future.
>>
>> Signed-off-by: Daniel Vetter <daniel.vet...@intel.com>
>> Cc: Tomi Valkeinen <tomi.valkei...@ti.com>
>> ---
>>  drivers/gpu/drm/omapdrm/omap_drv.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c 
>> b/drivers/gpu/drm/omapdrm/omap_drv.c
>> index 3f40f7af3285..6d52877ed25a 100644
>> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
>> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
>> @@ -490,7 +490,7 @@ static struct drm_driver omap_drm_driver = {
>>       .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
>>       .gem_prime_export = omap_gem_prime_export,
>>       .gem_prime_import = omap_gem_prime_import,
>> -     .gem_free_object = omap_gem_free_object,
>> +     .gem_free_object_unlocked = omap_gem_free_object,
>>       .gem_vm_ops = &omap_gem_vm_ops,
>>       .dumb_create = omap_gem_dumb_create,
>>       .dumb_map_offset = omap_gem_dumb_map_offset,
>>
>
> This triggers WARN_ON(!mutex_is_locked(&dev->struct_mutex)), we have a
> few of those. I need to reverse engineer the omap_gem locking a bit to
> understand what's needed. And I need to figure out what exactly does
> struct_mutext protect =).

Oops, I thought I read the code carefully and audited for that. Can
you pls attach some example backtraces?

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to