[PATCH 2/4] Remove the cacheing of the last scratch PixmapRec
Chris Wilson
chris at chris-wilson.co.uk
Sun Jun 5 22:36:07 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.
In addition we introduce a new hint, CREATE_PIXMAP_USAGE_SCRATCH_HEADER,
so that the driver can be warned during pixmap creation (and lifetime)
if the pixmap points to other data.
A side-effect of removing the cache is that several members of the
ScreenInfo structure and associated routines become redundant and
deleted as well. So we bump the ABI version as well.
Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
---
dix/dispatch.c | 1 -
dix/main.c | 4 +--
dix/pixmap.c | 58 ++++++++++++++++------------------------
fb/fbpixmap.c | 2 +-
hw/xfree86/common/xf86Module.h | 2 +-
include/pixmap.h | 7 +---
include/scrnintstr.h | 8 +++---
7 files changed, 32 insertions(+), 50 deletions(-)
diff --git a/dix/dispatch.c b/dix/dispatch.c
index 192c8c3..3f73175 100644
--- a/dix/dispatch.c
+++ b/dix/dispatch.c
@@ -3855,7 +3855,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 955b7ea..7eadf36 100644
--- a/dix/main.c
+++ b/dix/main.c
@@ -206,12 +206,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");
@@ -321,7 +320,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..0b43592 100644
--- a/dix/pixmap.c
+++ b/dix/pixmap.c
@@ -53,20 +53,19 @@ 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,
+ CREATE_PIXMAP_USAGE_SCRATCH_HEADER);
+ if (pPixmap == NullPixmap)
+ return NullPixmap;
+
+ if ((*pScreen->ModifyPixmapHeader)(pPixmap, width, height, depth,
+ bitsPerPixel, devKind, pPixData))
+ return pPixmap;
+
+ (*pScreen->DestroyPixmap)(pPixmap);
return NullPixmap;
}
@@ -79,33 +78,22 @@ FreeScratchPixmapHeader(PixmapPtr 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;
+ if (pPixmap->refcnt != 1)
+ FatalError("Scratch pixmap still in use when finalized, refcnt=%d\n",
+ pPixmap->refcnt);
+
+ (*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 +103,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 a356c67..aebdff0 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 2a5c805..cf7557e 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(11, 0)
+#define ABI_VIDEODRV_VERSION SET_ABI_VERSION(12, 0)
#define ABI_XINPUT_VERSION SET_ABI_VERSION(13, 0)
#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..617eadb 100644
--- a/include/scrnintstr.h
+++ b/include/scrnintstr.h
@@ -202,6 +202,8 @@ typedef void (* ClipNotifyProcPtr)(
#define CREATE_PIXMAP_USAGE_BACKING_PIXMAP 2
/* pixmap will contain a glyph */
#define CREATE_PIXMAP_USAGE_GLYPH_PICTURE 3
+/* pixmap will only be as a header for transient (e.g. on-stack) pixels */
+#define CREATE_PIXMAP_USAGE_SCRATCH_HEADER 4
typedef PixmapPtr (* CreatePixmapProcPtr)(
ScreenPtr /*pScreen*/,
@@ -518,10 +520,6 @@ typedef struct _Screen {
GetScreenPixmapProcPtr GetScreenPixmap;
SetScreenPixmapProcPtr SetScreenPixmap;
- PixmapPtr pScratchPixmap; /* scratch pixmap "pool" */
-
- unsigned int totalPixmapSize;
-
MarkWindowProcPtr MarkWindow;
MarkOverlappedWindowsProcPtr MarkOverlappedWindows;
ConfigNotifyProcPtr ConfigNotify;
@@ -556,6 +554,8 @@ typedef struct _ScreenInfo {
int bitmapScanlineUnit;
int bitmapScanlinePad;
int bitmapBitOrder;
+ unsigned int totalPixmapSize;
+
int numPixmapFormats;
PixmapFormatRec
formats[MAXFORMATS];
--
1.7.5.3
More information about the xorg-devel
mailing list