On Wed, Aug 7, 2013 at 11:29 AM, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote: > On Wed, Aug 07, 2013 at 11:15:07AM +0200, Daniel Vetter wrote: >> This fixes a WARN in i915_gem_free_object when the >> obj->pages_pin_count isn't 0. >> >> Reported-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxx> >> Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxx> >> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxx> > > Papers over the WARN with iffy locking. Not all callers hold > struct_mutex, right? Worse some do, some don't... Oops, that needs mutex locking here. I've checked the code and none of the callers here should ever hold our own dev->struct_mutex (due to the self-import checks we bypass dma-buf for our own objects) so no immediate deadlock. But it's easy to create circles and piss off lockded ofc. > What's the long term plan here? Per-bo locking with ww mutexes. Locking is done by the callers of map/unmap, sonce only those can properly do the ww locking dance on all relevent buffers of a batch upfront. It's going to be fun ;-) Looking closer I've also spotted that our map_buf callback has a call to i915_mutex_lock_interruptible, which means the map can fail with -EINTR. Currently it seems unspec'ed whether map is allowed to do that, but current callers certainly can't cope with this. I'll throw a 2nd patch on top. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
