Re: [PATCH 2/2] intel: Use I915_EXEC_NO_RELOC when available

From: Daniel Vetter <daniel@ffwll.ch>
To: "Kristian Høgsberg" <krh@bitplanet.net>
Cc: intel-gfx <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 2/2] intel: Use I915_EXEC_NO_RELOC when available
Date: Wed, 21 Jan 2015 10:24:51 +0100	[thread overview]
Message-ID: <20150121092451.GG10113@phenom.ffwll.local> (raw)
In-Reply-To: <20150121091701.GF10113@phenom.ffwll.local>

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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx