On Wed, Aug 7, 2013 at 11:29 AM, Chris Wilson chris@chris-wilson.co.uk 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@canonical.com Cc: Maarten Lankhorst maarten.lankhorst@canonical.com Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch
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