[PATCH 2/2] Remove the cacheing of the last scratch PixmapRec
Chris Wilson
chris at chris-wilson.co.uk
Wed Apr 13 03:50:25 PDT 2011
In order for the driver to be notified of when the resource backing the
scratch pixmap becomes no longer accessible, it needs to be called on
every FreeScratchPixmapHeader(). As we instead maybe cached the
PixmapRec (to avoid the free and malloc overhead), this notification
went astray, and the driver would fail to insert the correct barriers on
the backing resource. That resource would then be reused by the Xserver,
leading to rampant memory corruption as the GPU flushed it write caches
at some point in the future and overwriting random structures.
Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
---
dix/dispatch.c | 1 -
dix/main.c | 4 +--
dix/pixmap.c | 54 +++++++++++++--------------------------
fb/fbpixmap.c | 2 +-
hw/xfree86/common/xf86Module.h | 2 +-
include/pixmap.h | 7 +---
include/scrnintstr.h | 6 +---
7 files changed, 25 insertions(+), 51 deletions(-)
diff --git a/dix/dispatch.c b/dix/dispatch.c
index 601b14a..88cd95a 100644
--- a/dix/dispatch.c
+++ b/dix/dispatch.c
@@ -3849,7 +3849,6 @@ AddScreen(
return -1;
}
pScreen->myNum = i;
- pScreen->totalPixmapSize = 0; /* computed in CreateScratchPixmapForScreen */
pScreen->ClipNotify = 0; /* for R4 ddx compatibility */
pScreen->CreateScreenResources = 0;
diff --git a/dix/main.c b/dix/main.c
index 31e2d48..876b3c6 100644
--- a/dix/main.c
+++ b/dix/main.c
@@ -208,12 +208,11 @@ int main(int argc, char *argv[], char *envp[])
if (screenInfo.numScreens < 1)
FatalError("no screens found");
InitExtensions(argc, argv);
+ InitPixmaps();
for (i = 0; i < screenInfo.numScreens; i++)
{
ScreenPtr pScreen = screenInfo.screens[i];
- if (!CreateScratchPixmapsForScreen(i))
- FatalError("failed to create scratch pixmaps");
if (pScreen->CreateScreenResources &&
!(*pScreen->CreateScreenResources)(pScreen))
FatalError("failed to create screen resources");
@@ -316,7 +315,6 @@ int main(int argc, char *argv[], char *envp[])
for (i = screenInfo.numScreens - 1; i >= 0; i--)
{
- FreeScratchPixmapsForScreen(i);
FreeGCperDepth(i);
FreeDefaultStipple(i);
(* screenInfo.screens[i]->CloseScreen)(i, screenInfo.screens[i]);
diff --git a/dix/pixmap.c b/dix/pixmap.c
index cbb5e7f..9548b9e 100644
--- a/dix/pixmap.c
+++ b/dix/pixmap.c
@@ -53,20 +53,18 @@ PixmapPtr
GetScratchPixmapHeader(ScreenPtr pScreen, int width, int height, int depth,
int bitsPerPixel, int devKind, pointer pPixData)
{
- PixmapPtr pPixmap = pScreen->pScratchPixmap;
+ PixmapPtr pPixmap;
- if (pPixmap)
- pScreen->pScratchPixmap = NULL;
- else
- /* width and height of 0 means don't allocate any pixmap data */
- pPixmap = (*pScreen->CreatePixmap)(pScreen, 0, 0, depth, 0);
-
- if (pPixmap) {
- if ((*pScreen->ModifyPixmapHeader)(pPixmap, width, height, depth,
- bitsPerPixel, devKind, pPixData))
- return pPixmap;
- (*pScreen->DestroyPixmap)(pPixmap);
- }
+ /* width and height of 0 means don't allocate any pixmap data */
+ pPixmap = (*pScreen->CreatePixmap)(pScreen, 0, 0, depth, 0);
+ if (pPixmap == NullPixmap)
+ return NullPixmap;
+
+ if ((*pScreen->ModifyPixmapHeader)(pPixmap, width, height, depth,
+ bitsPerPixel, devKind, pPixData))
+ return pPixmap;
+
+ (*pScreen->DestroyPixmap)(pPixmap);
return NullPixmap;
}
@@ -78,34 +76,18 @@ FreeScratchPixmapHeader(PixmapPtr pPixmap)
if (pPixmap)
{
ScreenPtr pScreen = pPixmap->drawable.pScreen;
-
- pPixmap->devPrivate.ptr = NULL; /* lest ddx chases bad ptr */
- if (pScreen->pScratchPixmap)
- (*pScreen->DestroyPixmap)(pPixmap);
- else
- pScreen->pScratchPixmap = pPixmap;
+ (*pScreen->DestroyPixmap)(pPixmap);
}
}
-Bool
-CreateScratchPixmapsForScreen(int scrnum)
+void
+InitPixmaps(void)
{
unsigned int pixmap_size;
pixmap_size = sizeof(PixmapRec) + dixPrivatesSize(PRIVATE_PIXMAP);
- screenInfo.screens[scrnum]->totalPixmapSize = BitmapBytePad(pixmap_size * 8);
-
- /* let it be created on first use */
- screenInfo.screens[scrnum]->pScratchPixmap = NULL;
- return TRUE;
-}
-
-
-void
-FreeScratchPixmapsForScreen(int scrnum)
-{
- FreeScratchPixmapHeader(screenInfo.screens[scrnum]->pScratchPixmap);
+ screenInfo.totalPixmapSize = BitmapBytePad(pixmap_size * 8);
}
@@ -115,12 +97,12 @@ AllocatePixmap(ScreenPtr pScreen, int pixDataSize)
{
PixmapPtr pPixmap;
- assert(pScreen->totalPixmapSize > 0);
+ assert(screenInfo.totalPixmapSize > 0);
- if (pScreen->totalPixmapSize > ((size_t)-1) - pixDataSize)
+ if (screenInfo.totalPixmapSize > ((size_t)-1) - pixDataSize)
return NullPixmap;
- pPixmap = malloc(pScreen->totalPixmapSize + pixDataSize);
+ pPixmap = malloc(screenInfo.totalPixmapSize + pixDataSize);
if (!pPixmap)
return NullPixmap;
diff --git a/fb/fbpixmap.c b/fb/fbpixmap.c
index 41b12ce..e2f4715 100644
--- a/fb/fbpixmap.c
+++ b/fb/fbpixmap.c
@@ -42,7 +42,7 @@ fbCreatePixmapBpp (ScreenPtr pScreen, int width, int height, int depth, int bpp,
if (paddedWidth / 4 > 32767 || height > 32767)
return NullPixmap;
datasize = height * paddedWidth;
- base = pScreen->totalPixmapSize;
+ base = screenInfo.totalPixmapSize;
adjust = 0;
if (base & 7)
adjust = 8 - (base & 7);
diff --git a/hw/xfree86/common/xf86Module.h b/hw/xfree86/common/xf86Module.h
index 581047d..94f17e9 100644
--- a/hw/xfree86/common/xf86Module.h
+++ b/hw/xfree86/common/xf86Module.h
@@ -82,7 +82,7 @@ typedef enum {
* mask is 0xFFFF0000.
*/
#define ABI_ANSIC_VERSION SET_ABI_VERSION(0, 4)
-#define ABI_VIDEODRV_VERSION SET_ABI_VERSION(10, 0)
+#define ABI_VIDEODRV_VERSION SET_ABI_VERSION(11, 0)
#define ABI_XINPUT_VERSION SET_ABI_VERSION(12, 2)
#define ABI_EXTENSION_VERSION SET_ABI_VERSION(5, 0)
#define ABI_FONT_VERSION SET_ABI_VERSION(0, 6)
diff --git a/include/pixmap.h b/include/pixmap.h
index 014a111..daab76c 100644
--- a/include/pixmap.h
+++ b/include/pixmap.h
@@ -103,11 +103,8 @@ extern _X_EXPORT PixmapPtr GetScratchPixmapHeader(
extern _X_EXPORT void FreeScratchPixmapHeader(
PixmapPtr /*pPixmap*/);
-extern _X_EXPORT Bool CreateScratchPixmapsForScreen(
- int /*scrnum*/);
-
-extern _X_EXPORT void FreeScratchPixmapsForScreen(
- int /*scrnum*/);
+extern _X_EXPORT void InitPixmaps(
+ void);
extern _X_EXPORT PixmapPtr AllocatePixmap(
ScreenPtr /*pScreen*/,
diff --git a/include/scrnintstr.h b/include/scrnintstr.h
index a9357e8..2b5f4de 100644
--- a/include/scrnintstr.h
+++ b/include/scrnintstr.h
@@ -518,10 +518,6 @@ typedef struct _Screen {
GetScreenPixmapProcPtr GetScreenPixmap;
SetScreenPixmapProcPtr SetScreenPixmap;
- PixmapPtr pScratchPixmap; /* scratch pixmap "pool" */
-
- unsigned int totalPixmapSize;
-
MarkWindowProcPtr MarkWindow;
MarkOverlappedWindowsProcPtr MarkOverlappedWindows;
ConfigNotifyProcPtr ConfigNotify;
@@ -556,6 +552,8 @@ typedef struct _ScreenInfo {
int bitmapScanlineUnit;
int bitmapScanlinePad;
int bitmapBitOrder;
+ unsigned int totalPixmapSize;
+
int numPixmapFormats;
PixmapFormatRec
formats[MAXFORMATS];
--
1.7.4.1
More information about the xorg-devel
mailing list