frederik
/
xserver
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

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@redhat.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
This commit is contained in:
Chris Wilson 2016-02-12 11:59:53 +00:00 committed by Adam Jackson
parent bc3634010c
commit d888295457
1 changed files with 71 additions and 59 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

130
hw/xfree86/dri2/dri2.c
View File

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