On Wed, Jan 21, 2015 at 10:17:01AM +0100, Daniel Vetter wrote: > On Tue, Jan 20, 2015 at 10:10:57PM -0800, Kristian Høgsberg wrote: > > However, drm_intel_bo_reference/unreference() is showing up on the > > profiles now that relocations are cheaper, but I think the better way > > to make those cheaper is to move the ref count to the public struct > > and make the ref/unref inline functions (calling out to a destructor > > for the refcount==0 case, of course). On that note, do you know why > > drm_intel_gem_bo_unreference() is so convoluted? What's the deal with > > the atomic_add_unless? > > Smells like the last reference can only be dropped with the mutex held, > probably because someone is assuming that buffers don't go poof while > holding the mutex. You should be able to drop this after some refcounting > audit to make sure no one looks at a bo after the unref is done. To elaborate: I think the reason here is that there's a bunch of weak references (i.e. pointers that don't actually hold a reference) in the various caches in libdrm (both bo cache and import cache). Those are only protected by the mutex, which means if someone could drop the refcount to 0 without holding the mutex they'd try to increase the refcount of a bo which is in the process of final destruction. Not great. The approach the kernel uses is to have plain unlocked atomic_inc/dec for all cases where there's a strong references. For the weak ones you use atomic_in_unless_zero while holding a lock that the destructor also always needs. That gives you enough synchronization to reliable detect when your weak reference points at a zombie (in which case you just fail the lookup). That gives you a nice fastpath and shifts all the cost to the weak refcount lookup and final destruction (where you must grab the mutex and use more expensive atomic ops). In drm there's a bunch of examples using kref_get_unless_zero as reading material. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx