On Mon, Apr 02, 2018 at 06:05:22PM +0300, Laurent Pinchart wrote: > 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. Hm, indeed I missed that one. I'm not entirely sure how the tiler locking will pan out though, and whether there's not some stuff that won't be protected anymore. Specifically omapdrm_priv->usergart looks like it actually requires struct_mutex :-/ -Daniel > > -- > Regards, > > Laurent Pinchart > > > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
