No discussions on this yet, so I'm giving it a second try.
David, what do you think, should this go into master before 7.5 hits the
shelf? At least I'm getting server segfaults without it with intel and
KMS, and it seems to run fine with the patch included.
OTOH I'm pretty unsure about potential memory leaks. Shouldn't be heavy,
though.
I'd probably nuke the second part, which tries to fix the original issue
(and fails so far). Even though my corrected code looks more sane to me
than the original one.
Matthias
--
Matthias Hopf <[email protected]> __ __ __
Maxfeldstr. 5 / 90409 Nuernberg (_ | | (_ |__ [email protected]
Phone +49-911-74053-715 __) |_| __) |__ R & D www.mshopf.de
--- Begin Message ---
On Sep 04, 09 17:34:00 +0200, Matthias Hopf wrote:
> Sometimes the current Xserver (with intel 2.8.1 with KMS) crashes after
> resume from RAM (or disk, but that is broken anyway ATM). I managed to
> get a core and analyze it (see attached gdb log).
>
> Long story short, sometimes after resume ScreenPriv->SavedCursor->bits
> is NULL, and the server crashes.
>
> The attached patch works around this issue, but it doesn't solve the
> mysterious initial issue (why bits is actually NULL). If anybody has a
> good idea, or a better idea how to actually solve this...
Forget the original patch, it didn't work at all.
This patch
1) tries (unsuccessfully so far) fix the issue
2) works around the crash by not cleaning up the old cursor
Could very well be that this introduces a memory leak. Well, better than
a server crash :-/
Comments welcome
Matthias
--
Matthias Hopf <[email protected]> __ __ __
Maxfeldstr. 5 / 90409 Nuernberg (_ | | (_ |__ [email protected]
Phone +49-911-74053-715 __) |_| __) |__ R & D www.mshopf.de
>From 1efa035209aea97d452d829e29af2e5f1ac94272 Mon Sep 17 00:00:00 2001
From: Matthias Hopf <[email protected]>
Date: Fri, 4 Sep 2009 17:32:45 +0200
Subject: [PATCH] Work around sporadic segfault on resume with intel/KMS due to
cursor->bits == NULL.
Apparently SavedCursor is sometime tried to be set while already being set.
---
hw/xfree86/modes/xf86Cursors.c | 8 ++++++++
hw/xfree86/ramdac/xf86Cursor.c | 17 ++++++++++-------
2 files changed, 18 insertions(+), 7 deletions(-)
diff --git a/hw/xfree86/modes/xf86Cursors.c b/hw/xfree86/modes/xf86Cursors.c
index 8c5a94c..3436636 100644
--- a/hw/xfree86/modes/xf86Cursors.c
+++ b/hw/xfree86/modes/xf86Cursors.c
@@ -461,6 +461,10 @@ xf86_use_hw_cursor (ScreenPtr screen, CursorPtr cursor)
xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(scrn);
xf86CursorInfoPtr cursor_info = xf86_config->cursor_info;
+ if (xf86_config->cursor == cursor) {
+ xf86DrvMsg(index, X_ERROR, "Trying to set already set cursor.\n");
+ return FALSE;
+ }
if (xf86_config->cursor)
FreeCursor (xf86_config->cursor, None);
xf86_config->cursor = cursor;
@@ -480,6 +484,10 @@ xf86_use_hw_cursor_argb (ScreenPtr screen, CursorPtr
cursor)
xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(scrn);
xf86CursorInfoPtr cursor_info = xf86_config->cursor_info;
+ if (xf86_config->cursor == cursor) {
+ xf86DrvMsg(index, X_ERROR, "Trying to set already set cursor.\n");
+ return FALSE;
+ }
if (xf86_config->cursor)
FreeCursor (xf86_config->cursor, None);
xf86_config->cursor = cursor;
diff --git a/hw/xfree86/ramdac/xf86Cursor.c b/hw/xfree86/ramdac/xf86Cursor.c
index 6b71f46..7aa7039 100644
--- a/hw/xfree86/ramdac/xf86Cursor.c
+++ b/hw/xfree86/ramdac/xf86Cursor.c
@@ -209,13 +209,16 @@ xf86CursorEnableDisableFBAccess(
xf86CursorScreenPtr ScreenPriv = (xf86CursorScreenPtr)dixLookupPrivate(
&pScreen->devPrivates, xf86CursorScreenKey);
- if (!enable && ScreenPriv->CurrentCursor != NullCursor) {
- CursorPtr currentCursor = ScreenPriv->CurrentCursor;
- xf86CursorSetCursor(pDev, pScreen, NullCursor, ScreenPriv->x,
- ScreenPriv->y);
- ScreenPriv->isUp = FALSE;
- ScreenPriv->SWCursor = TRUE;
- ScreenPriv->SavedCursor = currentCursor;
+ if (!enable) {
+ if (ScreenPriv->CurrentCursor != NullCursor) {
+ CursorPtr currentCursor = ScreenPriv->CurrentCursor;
+ xf86CursorSetCursor(pDev, pScreen, NullCursor, ScreenPriv->x,
+ ScreenPriv->y);
+ ScreenPriv->isUp = FALSE;
+ ScreenPriv->SWCursor = TRUE;
+ ScreenPriv->SavedCursor = currentCursor;
+ } else
+ ScreenPriv->SavedCursor = NULL;
}
if (ScreenPriv->EnableDisableFBAccess)
--
1.6.0.2
_______________________________________________
xorg-devel mailing list
[email protected]
http://lists.x.org/mailman/listinfo/xorg-devel
--- End Message ---
_______________________________________________
xorg-devel mailing list
[email protected]
http://lists.x.org/mailman/listinfo/xorg-devel
