On Fri, Sep 12, 2014 at 11:27:07AM +0100, Lionel Landwerlin wrote: > When using Mesa and LibVA in the same process, one would like to be > able bind buffers from the output of the decoder to a GL texture > through an EGLImage. > > LibVA can reuse buffers allocated by Gbm through a file descriptor. It > will then wrap it into a drm_intel_bo with > drm_intel_bo_gem_create_from_prime(). > > The problem at the moment is that both library get a different > drm_intel_bufmgr object when they call drm_intel_bufmgr_gem_init() > even though they're using the same drm file descriptor. As a result, > instead of manipulating the same buffer object for a given file > descriptor, they get 2 different drm_intel_bo objects and 2 different > refcounts, leading one of the library to get errors from the kernel on > invalid BO when one of the 2 library is done with a shared buffer. > > This patch modifies drm_intel_bufmgr_gem_init() so, given a file > descriptor, it will look for an already existing drm_intel_bufmgr > using the same file descriptor and return that object. > > Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
So close. > +static void > +drm_intel_bufmgr_gem_unref(drm_intel_bufmgr *bufmgr) > +{ > + drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *)bufmgr; > + > + if (atomic_dec_and_test(&bufmgr_gem->refcount)) { > + assert(pthread_mutex_lock(&bufmgr_list_mutex) == 0); (Oh, don't use assert for expressions with side-effects) > + > + if (atomic_read(&bufmgr_gem->refcount)) { As a thought exercise, now consider what happens if thread B grabs a reference to this bufmgr and frees it all before this thread wakes up? Double free, kaboom. This does have to be an atomic_add_unless, which does not yet exist in libdrm: static inline int atomic_add_unless(atomic_t *v, int add, int unless) { int c, old; c = atomic_read(v); while (c != unless && (old = atomic_cmpxchg(v, c, c + add)) != c) c = old; return c == unless; } static void drm_intel_bufmgr_gem_unref(drm_intel_bufmgr *bufmgr) { drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *)bufmgr; if (atomic_add_unless(&bufmgr_gem->refcount, -1, 1)) { pthread_mutex_lock(&bufmgr_list_mutex); if (atomic_dec_and_test(&bufmgr_gem->refcount)) { free stuff; } pthread_mutex_unlock(&bufmgr_list_mutex); } } -Chris -- Chris Wilson, Intel Open Source Technology Centre