[PATCH] intel: make bufmgr_gem shareable from different API

[Chris Wilson]

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