xserver: Branch 'master' - 2 commits

Adam Jackson ajax at kemper.freedesktop.org
Mon Feb 29 19:37:11 UTC 2016


 dix/dixutils.c         |   22 ++++++++
 hw/xfree86/dri2/dri2.c |  130 ++++++++++++++++++++++++++-----------------------
 include/dix.h          |    8 +++
 3 files changed, 101 insertions(+), 59 deletions(-)

New commits:
commit d8882954570aba656d5a7be7d357feaba21cb099
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Fri Feb 12 11:59:53 2016 +0000

    dri2: Allow many blocked clients per-drawable
    
    This patch was motivated by the need to fix the use-after-free in
    dri2ClientWake, but in doing so removes an arbitrary restriction that
    limits DRI2 to only blocking the first client on each drawable. In order
    to fix the use-after-free, we need to avoid touching our privates in the
    ClientSleep callback and so we want to only use that external list as
    our means of controlling sleeps and wakeups. We thus have a list of
    sleeping clients at our disposal and can manage multiple events and
    sources.
    
    Reviewed-by: Adam Jackson <ajax at redhat.com>
    Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>

diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
index f9d818c..d55be19 100644
--- a/hw/xfree86/dri2/dri2.c
+++ b/hw/xfree86/dri2/dri2.c
@@ -90,8 +90,6 @@ typedef struct _DRI2Drawable {
     DRI2BufferPtr *buffers;
     int bufferCount;
     unsigned int swapsPending;
-    ClientPtr blockedClient;
-    Bool blockedOnMsc;
     int swap_interval;
     CARD64 swap_count;
     int64_t target_sbc;         /* -1 means no SBC wait outstanding */
@@ -99,6 +97,7 @@ typedef struct _DRI2Drawable {
     CARD64 last_swap_msc;       /* msc at completion of most recent swap */
     CARD64 last_swap_ust;       /* ust at completion of most recent swap */
     int swap_limit;             /* for N-buffering */
+    unsigned blocked[3];
     Bool needInvalidate;
     int prime_id;
     PixmapPtr prime_slave_pixmap;
@@ -139,6 +138,44 @@ typedef struct _DRI2Screen {
 static void
 destroy_buffer(DrawablePtr pDraw, DRI2BufferPtr buffer, int prime_id);
 
+enum DRI2WakeType {
+    WAKE_SBC,
+    WAKE_MSC,
+    WAKE_SWAP,
+};
+
+#define Wake(c, t) (void *)((uintptr_t)(c) | (t))
+
+static Bool
+dri2WakeClient(ClientPtr client, void *closure)
+{
+    ClientWakeup(client);
+    return TRUE;
+}
+
+static Bool
+dri2WakeAll(ClientPtr client, DRI2DrawablePtr pPriv, enum DRI2WakeType t)
+{
+    int count;
+
+    if (!pPriv->blocked[t])
+        return FALSE;
+
+    count = ClientSignalAll(client, dri2WakeClient, Wake(pPriv, t));
+    pPriv->blocked[t] -= count;
+    return count;
+}
+
+static Bool
+dri2Sleep(ClientPtr client, DRI2DrawablePtr pPriv, enum DRI2WakeType t)
+{
+    if (ClientSleep(client, dri2WakeClient, Wake(pPriv, t))) {
+        pPriv->blocked[t]++;
+        return TRUE;
+    }
+    return FALSE;
+}
+
 static DRI2ScreenPtr
 DRI2GetScreen(ScreenPtr pScreen)
 {
@@ -210,8 +247,6 @@ DRI2AllocateDrawable(DrawablePtr pDraw)
     pPriv->buffers = NULL;
     pPriv->bufferCount = 0;
     pPriv->swapsPending = 0;
-    pPriv->blockedClient = NULL;
-    pPriv->blockedOnMsc = FALSE;
     pPriv->swap_count = 0;
     pPriv->target_sbc = -1;
     pPriv->swap_interval = 1;
@@ -219,6 +254,7 @@ DRI2AllocateDrawable(DrawablePtr pDraw)
     if (!ds->GetMSC || !(*ds->GetMSC) (pDraw, &ust, &pPriv->last_swap_target))
         pPriv->last_swap_target = 0;
 
+    memset(pPriv->blocked, 0, sizeof(pPriv->blocked));
     pPriv->swap_limit = 1;      /* default to double buffering */
     pPriv->last_swap_msc = 0;
     pPriv->last_swap_ust = 0;
@@ -258,12 +294,7 @@ DRI2SwapLimit(DrawablePtr pDraw, int swap_limit)
     if (pPriv->swapsPending >= pPriv->swap_limit)
         return TRUE;
 
-    if (pPriv->target_sbc == -1 && !pPriv->blockedOnMsc) {
-        if (pPriv->blockedClient) {
-            ClientSignal(pPriv->blockedClient);
-        }
-    }
-
+    dri2WakeAll(CLIENT_SIGNAL_ANY, pPriv, WAKE_SWAP);
     return TRUE;
 }
 
@@ -412,8 +443,9 @@ DRI2DrawableGone(void *p, XID id)
         (*pDraw->pScreen->DestroyPixmap)(pPriv->redirectpixmap);
     }
 
-    if (pPriv->blockedClient)
-        ClientSignal(pPriv->blockedClient);
+    dri2WakeAll(CLIENT_SIGNAL_ANY, pPriv, WAKE_SWAP);
+    dri2WakeAll(CLIENT_SIGNAL_ANY, pPriv, WAKE_MSC);
+    dri2WakeAll(CLIENT_SIGNAL_ANY, pPriv, WAKE_SBC);
 
     free(pPriv);
 
@@ -689,15 +721,6 @@ DRI2InvalidateDrawable(DrawablePtr pDraw)
         ref->invalidate(pDraw, ref->priv, ref->id);
 }
 
-static Bool
-dri2ClientWake(ClientPtr client, void *closure)
-{
-    DRI2DrawablePtr pPriv = closure;
-    ClientWakeup(client);
-    pPriv->blockedClient = NULL;
-    return TRUE;
-}
-
 /*
  * In the direct rendered case, we throttle the clients that have more
  * than their share of outstanding swaps (and thus busy buffers) when a
@@ -715,26 +738,17 @@ DRI2ThrottleClient(ClientPtr client, DrawablePtr pDraw)
         return FALSE;
 
     /* Throttle to swap limit */
-    if ((pPriv->swapsPending >= pPriv->swap_limit) && !pPriv->blockedClient) {
-        ResetCurrentRequest(client);
-        client->sequence--;
-        ClientSleep(client, dri2ClientWake, pPriv);
-        pPriv->blockedClient = client;
-        return TRUE;
+    if (pPriv->swapsPending >= pPriv->swap_limit) {
+        if (dri2Sleep(client, pPriv, WAKE_SWAP)) {
+            ResetCurrentRequest(client);
+            client->sequence--;
+            return TRUE;
+        }
     }
 
     return FALSE;
 }
 
-static void
-__DRI2BlockClient(ClientPtr client, DRI2DrawablePtr pPriv)
-{
-    if (pPriv->blockedClient == NULL) {
-        ClientSleep(client, dri2ClientWake, pPriv);
-        pPriv->blockedClient = client;
-    }
-}
-
 void
 DRI2BlockClient(ClientPtr client, DrawablePtr pDraw)
 {
@@ -744,8 +758,7 @@ DRI2BlockClient(ClientPtr client, DrawablePtr pDraw)
     if (pPriv == NULL)
         return;
 
-    __DRI2BlockClient(client, pPriv);
-    pPriv->blockedOnMsc = TRUE;
+    dri2Sleep(client, pPriv, WAKE_MSC);
 }
 
 static inline PixmapPtr GetDrawablePixmap(DrawablePtr drawable)
@@ -978,10 +991,7 @@ DRI2WaitMSCComplete(ClientPtr client, DrawablePtr pDraw, int frame,
     ProcDRI2WaitMSCReply(client, ((CARD64) tv_sec * 1000000) + tv_usec,
                          frame, pPriv->swap_count);
 
-    if (pPriv->blockedClient)
-        ClientSignal(pPriv->blockedClient);
-
-    pPriv->blockedOnMsc = FALSE;
+    dri2WakeAll(client, pPriv, WAKE_MSC);
 }
 
 static void
@@ -1007,16 +1017,14 @@ DRI2WakeClient(ClientPtr client, DrawablePtr pDraw, int frame,
      *   - is not blocked due to an MSC wait
      */
     if (pPriv->target_sbc != -1 && pPriv->target_sbc <= pPriv->swap_count) {
-        ProcDRI2WaitMSCReply(client, ((CARD64) tv_sec * 1000000) + tv_usec,
-                             frame, pPriv->swap_count);
-        pPriv->target_sbc = -1;
-        ClientSignal(pPriv->blockedClient);
-    }
-    else if (pPriv->target_sbc == -1 && !pPriv->blockedOnMsc) {
-        if (pPriv->blockedClient) {
-            ClientSignal(pPriv->blockedClient);
+        if (dri2WakeAll(client, pPriv, WAKE_SBC)) {
+            ProcDRI2WaitMSCReply(client, ((CARD64) tv_sec * 1000000) + tv_usec,
+                                 frame, pPriv->swap_count);
+            pPriv->target_sbc = -1;
         }
     }
+
+    dri2WakeAll(CLIENT_SIGNAL_ANY, pPriv, WAKE_SWAP);
 }
 
 void
@@ -1064,13 +1072,13 @@ DRI2WaitSwap(ClientPtr client, DrawablePtr pDrawable)
     DRI2DrawablePtr pPriv = DRI2GetDrawable(pDrawable);
 
     /* If we're currently waiting for a swap on this drawable, reset
-     * the request and suspend the client.  We only support one
-     * blocked client per drawable. */
-    if (pPriv && pPriv->swapsPending && pPriv->blockedClient == NULL) {
-        ResetCurrentRequest(client);
-        client->sequence--;
-        __DRI2BlockClient(client, pPriv);
-        return TRUE;
+     * the request and suspend the client. */
+    if (pPriv && pPriv->swapsPending) {
+        if (dri2Sleep(client, pPriv, WAKE_SWAP)) {
+            ResetCurrentRequest(client);
+            client->sequence--;
+            return TRUE;
+        }
     }
 
     return FALSE;
@@ -1271,6 +1279,9 @@ DRI2WaitSBC(ClientPtr client, DrawablePtr pDraw, CARD64 target_sbc)
     if (pPriv == NULL)
         return BadDrawable;
 
+    if (pPriv->target_sbc != -1) /* already in use */
+        return BadDrawable;
+
     /* target_sbc == 0 means to block until all pending swaps are
      * finished. Recalculate target_sbc to get that behaviour.
      */
@@ -1287,9 +1298,10 @@ DRI2WaitSBC(ClientPtr client, DrawablePtr pDraw, CARD64 target_sbc)
         return Success;
     }
 
-    pPriv->target_sbc = target_sbc;
-    __DRI2BlockClient(client, pPriv);
+    if (!dri2Sleep(client, pPriv, WAKE_SBC))
+        return BadAlloc;
 
+    pPriv->target_sbc = target_sbc;
     return Success;
 }
 
commit bc3634010c096dffd1935c0c6cf8ba37534ae3d8
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Fri Feb 12 11:59:52 2016 +0000

    dix: Add ClientSignalAll()
    
    This is a variant of ClientSignal() that signals all clients with an
    optional matching sleeping client, function and closure.
    
    Reviewed-by: Adam Jackson <ajax at redhat.com>
    Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>

diff --git a/dix/dixutils.c b/dix/dixutils.c
index 205550e..b6b0023 100644
--- a/dix/dixutils.c
+++ b/dix/dixutils.c
@@ -620,6 +620,28 @@ ClientSignal(ClientPtr client)
     return FALSE;
 }
 
+int
+ClientSignalAll(ClientPtr client, ClientSleepProcPtr function, void *closure)
+{
+    SleepQueuePtr q;
+    int count = 0;
+
+    for (q = sleepQueue; q; q = q->next) {
+        if (!(client == CLIENT_SIGNAL_ANY || q->client == client))
+            continue;
+
+        if (!(function == CLIENT_SIGNAL_ANY || q->function == function))
+            continue;
+
+        if (!(closure == CLIENT_SIGNAL_ANY || q->closure == closure))
+            continue;
+
+        count += QueueWorkProc(q->function, q->client, q->closure);
+    }
+
+    return count;
+}
+
 void
 ClientWakeup(ClientPtr client)
 {
diff --git a/include/dix.h b/include/dix.h
index 921156b..d49d055 100644
--- a/include/dix.h
+++ b/include/dix.h
@@ -255,6 +255,14 @@ extern _X_EXPORT Bool ClientSleep(ClientPtr client,
 extern _X_EXPORT Bool ClientSignal(ClientPtr /*client */ );
 #endif                          /* ___CLIENTSIGNAL_DEFINED___ */
 
+#ifndef ___CLIENTSIGNALALL_DEFINED___
+#define ___CLIENTSIGNALALL_DEFINED___
+#define CLIENT_SIGNAL_ANY ((void *)-1)
+extern _X_EXPORT int ClientSignalAll(ClientPtr /*client*/,
+                                     ClientSleepProcPtr /*function*/,
+                                     void * /*closure*/);
+#endif                          /* ___CLIENTSIGNALALL_DEFINED___ */
+
 extern _X_EXPORT void ClientWakeup(ClientPtr /*client */ );
 
 extern _X_EXPORT Bool ClientIsAsleep(ClientPtr /*client */ );


More information about the xorg-commit mailing list