Hello, On Thursday, 29 March 2018 12:41:33 EEST Tomi Valkeinen wrote: > On 28/03/18 14:41, 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. > > > > v2: Just auditing the code isn't enough, I actually have to remove > > the now wrong locking check in omap_gem_free_object ... > > > > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx> > > Cc: Tomi Valkeinen <tomi.valkeinen@xxxxxx> > > This version works fine. I'll pick this to omapdrm branch. Thanks! Unless I'm mistaken (this is only based on code analysis), a WARN_ON could also be triggered through the following call stack. gem_free_object_unlocked() omap_gem_free_object() evict() evict_entry() mmap_offset() WARN_ON(!mutex_is_locked(&dev->struct_mutex)) There could be other such call stacks. I don't think we should switch to gem_free_object_unlocked() until all usage of struct_mutex is removed from the omapdrm driver. -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
