radeon: Add protection against recursive DRM locking.
authorPauli Nieminen <suokkos@gmail.com>
Thu, 30 Jul 2009 17:17:29 +0000 (20:17 +0300)
committerAlex Deucher <alexdeucher@gmail.com>
Wed, 12 Aug 2009 18:14:29 +0000 (14:14 -0400)
Reference counting protects DRM lock call from recursive locking that would
cause hang. Code also adds optional debugging output for recursive call that
is compiled only if NDEBUG is not defined.

This code is not 100% thread safe because mesa doesn't include increment and
test atomic operation. There is built-in gcc functions but they are only
available from gcc 4.2.

src/mesa/drivers/dri/radeon/radeon_common_context.c
src/mesa/drivers/dri/radeon/radeon_common_context.h
src/mesa/drivers/dri/radeon/radeon_lock.c
src/mesa/drivers/dri/radeon/radeon_lock.h

index 5e744f9deb724490a7dc621c8217c1f0bef896f5..d68aa2bd62a8e25e48d073e1e868fdba5f37dac7 100644 (file)
@@ -211,6 +211,7 @@ GLboolean radeonInitContext(radeonContextPtr radeon,
        radeon->dri.screen = sPriv;
        radeon->dri.hwContext = driContextPriv->hHWContext;
        radeon->dri.hwLock = &sPriv->pSAREA->lock;
+       radeon->dri.hwLockCount = 0;
        radeon->dri.fd = sPriv->fd;
        radeon->dri.drmMinor = sPriv->drm_version.minor;
 
index d7e94a689493e14f4f819173df69b64b19e6925a..f8e1a25c9fa7e2bd26cb48d860ebba5027dacee5 100644 (file)
@@ -365,6 +365,7 @@ struct radeon_dri_mirror {
 
        drm_context_t hwContext;
        drm_hw_lock_t *hwLock;
+       int hwLockCount;
        int fd;
        int drmMinor;
 };
index 2f0ed1cfceddf2d70aa8574ca1883633369bd8b0..6294b7e42be94264876207246f191cab1c06179e 100644 (file)
@@ -86,8 +86,34 @@ void radeonGetLock(radeonContextPtr rmesa, GLuint flags)
 
        rmesa->vtbl.get_lock(rmesa);
 }
-
-void radeon_lock_hardware(radeonContextPtr radeon)
+#ifndef NDEBUG
+struct lock_debug {
+       const char* function;
+       const char* file;
+       int line;
+};
+
+static struct lock_debug ldebug = {0};
+#endif
+
+#if 0
+/** TODO: use atomic operations for reference counting **/
+/** gcc 4.2 has builtin functios for this **/
+#define ATOMIC_INC_AND_FETCH(atomic) __sync_add_and_fetch(&atomic, 1)
+#define ATOMIC_DEC_AND_FETCH(atomic) __sync_sub_and_fetch(&atomic, 1)
+#else
+#define ATOMIC_INC_AND_FETCH(atomic) (++atomic)
+#define ATOMIC_DEC_AND_FETCH(atomic) (--atomic)
+#endif
+
+
+void radeon_lock_hardware(radeonContextPtr radeon
+#ifndef NDEBUG
+               ,const char* function
+               ,const char* file
+               ,const int line
+#endif
+               )
 {
        char ret = 0;
        struct radeon_framebuffer *rfb = NULL;
@@ -102,16 +128,39 @@ void radeon_lock_hardware(radeonContextPtr radeon)
        }
 
        if (!radeon->radeonScreen->driScreen->dri2.enabled) {
+               if (ATOMIC_INC_AND_FETCH(radeon->dri.hwLockCount) > 1)
+               {
+#ifndef NDEBUG
+                       if ( RADEON_DEBUG & DEBUG_SANITY )
+                               fprintf(stderr, "*** %d times of recursive call to %s ***\n"
+                                               "Original call was from %s (file: %s line: %d)\n"
+                                               "Now call is coming from %s (file: %s line: %d)\n"
+                                               , radeon->dri.hwLockCount, __FUNCTION__
+                                               , ldebug.function, ldebug.file, ldebug.line
+                                               , function, file, line
+                                          );
+#endif
+                       return;
+               }
                DRM_CAS(radeon->dri.hwLock, radeon->dri.hwContext,
                         (DRM_LOCK_HELD | radeon->dri.hwContext), ret );
                if (ret)
                        radeonGetLock(radeon, 0);
+#ifndef NDEBUG
+               ldebug.function = function;
+               ldebug.file = file;
+               ldebug.line = line;
+#endif
        }
 }
 
 void radeon_unlock_hardware(radeonContextPtr radeon)
 {
        if (!radeon->radeonScreen->driScreen->dri2.enabled) {
+               if (ATOMIC_DEC_AND_FETCH(radeon->dri.hwLockCount) > 0)
+               {
+                       return;
+               }
                DRM_UNLOCK( radeon->dri.fd,
                            radeon->dri.hwLock,
                            radeon->dri.hwContext );
index 2817709eed6a3a8f8c2456bfa9948445230668be..da5a5b43715d0d231e0021f3fa96aa1aefc91c5e 100644 (file)
@@ -48,12 +48,22 @@ WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 extern void radeonGetLock(radeonContextPtr rmesa, GLuint flags);
 
-void radeon_lock_hardware(radeonContextPtr rmesa);
+void radeon_lock_hardware(radeonContextPtr rmesa
+#ifndef NDEBUG
+               ,const char* function
+               ,const char* file
+               ,const int line
+#endif
+               );
 void radeon_unlock_hardware(radeonContextPtr rmesa);
 
 /* Lock the hardware and validate our state.
  */
+#ifdef NDEBUG
 #define LOCK_HARDWARE( rmesa ) radeon_lock_hardware(rmesa)
+#else
+#define LOCK_HARDWARE( rmesa ) radeon_lock_hardware(rmesa, __FUNCTION__, __FILE__, __LINE__)
+#endif
 #define UNLOCK_HARDWARE( rmesa )  radeon_unlock_hardware(rmesa)
 
 #endif