[xorg-x11-server/f18] fix bug on server shutdown + valgrind warnings (#891140)

Dave Airlie airlied at fedoraproject.org
Wed Jan 9 03:34:38 UTC 2013


commit 0cad7a0a3bc92fac0f73ec9b7c56d364b33b6499
Author: Dave Airlie <airlied at redhat.com>
Date:   Wed Jan 9 13:33:59 2013 +1000

    fix bug on server shutdown + valgrind warnings (#891140)

 ...fixup-tell-changed-so-randr-clients-can-t.patch |   79 +++++++-----
 0001-randr-cleanup-provider-properly.patch         |  133 ++++++++++++++++++++
 ...ly-set-the-compat-output-in-the-failure-c.patch |   35 +++++
 xorg-x11-server.spec                               |   11 ++-
 4 files changed, 222 insertions(+), 36 deletions(-)
---
diff --git a/0001-autoconfig-fixup-tell-changed-so-randr-clients-can-t.patch b/0001-autoconfig-fixup-tell-changed-so-randr-clients-can-t.patch
index 3a60ba1..2fc9c8c 100644
--- a/0001-autoconfig-fixup-tell-changed-so-randr-clients-can-t.patch
+++ b/0001-autoconfig-fixup-tell-changed-so-randr-clients-can-t.patch
@@ -1,22 +1,23 @@
-From 6070acd223363941114168968657bcfe71aacd54 Mon Sep 17 00:00:00 2001
+From 4ff93e2e2d5253ac8aa773861e15dd41c0b5d4fd Mon Sep 17 00:00:00 2001
 From: Fedora X Ninjas <x at fedoraproject.org>
 Date: Wed, 12 Dec 2012 15:15:38 +1000
 Subject: [PATCH] autoconfig: fixup tell changed so randr clients can tell
 
 This lets the gnome applet update if a usb device appears/disappears
 ---
- hw/xfree86/common/xf86platformBus.c |  6 ++++--
- hw/xfree86/modes/xf86Crtc.c         |  4 ++++
+ hw/xfree86/common/xf86platformBus.c |  7 +++++--
+ hw/xfree86/modes/xf86Crtc.c         |  2 ++
+ hw/xfree86/modes/xf86RandR12.c      |  2 ++
  randr/randr.c                       | 24 +++++++++++++++++++++++-
  randr/randrstr.h                    |  4 ++++
  randr/rrcrtc.c                      |  2 +-
  randr/rrinfo.c                      |  2 +-
  randr/rroutput.c                    |  2 +-
  randr/rrscreen.c                    |  2 +-
- 8 files changed, 39 insertions(+), 7 deletions(-)
+ 9 files changed, 40 insertions(+), 7 deletions(-)
 
 diff --git a/hw/xfree86/common/xf86platformBus.c b/hw/xfree86/common/xf86platformBus.c
-index 2b02e79..67d03eb 100644
+index 057b5b6..197bde8 100644
 --- a/hw/xfree86/common/xf86platformBus.c
 +++ b/hw/xfree86/common/xf86platformBus.c
 @@ -47,6 +47,7 @@
@@ -27,7 +28,7 @@ index 2b02e79..67d03eb 100644
  int platformSlotClaimed;
  
  int xf86_num_platform_devices;
-@@ -454,7 +455,8 @@ xf86platformAddDevice(int index)
+@@ -462,7 +463,8 @@ xf86platformAddDevice(int index)
     /* attach unbound to 0 protocol screen */
     AttachUnboundGPU(xf86Screens[0]->pScreen, xf86GPUScreens[i]->pScreen);
     xf86AutoConfigOutputDevice(xf86GPUScreens[i], xf86Screens[0]);
@@ -37,7 +38,13 @@ index 2b02e79..67d03eb 100644
     return 0;
  }
  
-@@ -497,7 +499,7 @@ xf86platformRemoveDevice(int index)
+@@ -500,12 +502,13 @@ xf86platformRemoveDevice(int index)
+     xf86GPUScreens[i]->pScreen->CloseScreen(xf86GPUScreens[i]->pScreen);
+ 
+     RemoveGPUScreen(xf86GPUScreens[i]->pScreen);
++
+     xf86DeleteScreen(xf86GPUScreens[i]);
+ 
      xf86UnclaimPlatformSlot(&xf86_platform_devices[index], NULL);
  
      xf86_remove_platform_device(index);
@@ -47,39 +54,43 @@ index 2b02e79..67d03eb 100644
      return;
  }
 diff --git a/hw/xfree86/modes/xf86Crtc.c b/hw/xfree86/modes/xf86Crtc.c
-index 4ad4152..3b22058 100644
+index 1263dd7..263a1dc 100644
 --- a/hw/xfree86/modes/xf86Crtc.c
 +++ b/hw/xfree86/modes/xf86Crtc.c
-@@ -744,10 +744,12 @@ xf86CrtcCloseScreen(ScreenPtr screen)
-     /* detach any providers */
-     if (config->randr_provider) {
-         if (config->randr_provider->offload_sink) {
-+            RRSetChanged(screen);
-             DetachOffloadGPU(screen);
-             config->randr_provider->offload_sink = NULL;
-         }
-         else if (config->randr_provider->output_source) {
-+            RRSetChanged(screen);
-             DetachOutputGPU(screen);
-             config->randr_provider->output_source = NULL;
-         }
-@@ -3348,6 +3350,7 @@ void xf86AutoConfigOutputDevice(ScrnInfoPtr pScrn, ScrnInfoPtr master)
+@@ -3346,6 +3346,7 @@ void xf86AutoConfigOutputDevice(ScrnInfoPtr pScrn, ScrnInfoPtr master)
          unbound = TRUE;
          AttachOffloadGPU(master->pScreen, pScrn->pScreen);
          slave_config->randr_provider->offload_sink = master_provider;
-+        RRSetChanged(master->pScreen);
++	RRSetChanged(master->pScreen);
      }
      if ((master->capabilities & RR_Capability_SourceOutput) &&
                 pScrn->capabilities & RR_Capability_SinkOutput) {
-@@ -3356,5 +3359,6 @@ void xf86AutoConfigOutputDevice(ScrnInfoPtr pScrn, ScrnInfoPtr master)
+@@ -3354,5 +3355,6 @@ void xf86AutoConfigOutputDevice(ScrnInfoPtr pScrn, ScrnInfoPtr master)
              DetachUnboundGPU(pScrn->pScreen);
          AttachOutputGPU(master->pScreen, pScrn->pScreen);
          slave_config->randr_provider->output_source = master_provider;
-+        RRSetChanged(master->pScreen);
++	RRSetChanged(master->pScreen);
      }
  }
+diff --git a/hw/xfree86/modes/xf86RandR12.c b/hw/xfree86/modes/xf86RandR12.c
+index 01fc9c5..9d96c4f 100644
+--- a/hw/xfree86/modes/xf86RandR12.c
++++ b/hw/xfree86/modes/xf86RandR12.c
+@@ -1895,10 +1895,12 @@ xf86RandR14ProviderDestroy(ScreenPtr screen, RRProviderPtr provider)
+         if (config->randr_provider->offload_sink) {
+             DetachOffloadGPU(screen);
+             config->randr_provider->offload_sink = NULL;
++	    RRSetChanged(screen);
+         }
+         else if (config->randr_provider->output_source) {
+             DetachOutputGPU(screen);
+             config->randr_provider->output_source = NULL;
++	    RRSetChanged(screen);
+         }
+         else if (screen->current_master)
+             DetachUnboundGPU(screen);
 diff --git a/randr/randr.c b/randr/randr.c
-index f0decfc..cb6fce7 100644
+index f0decfc..2aebe29 100644
 --- a/randr/randr.c
 +++ b/randr/randr.c
 @@ -464,6 +464,28 @@ TellChanged(WindowPtr pWin, pointer value)
@@ -96,8 +107,8 @@ index f0decfc..cb6fce7 100644
 +
 +    if (pScreen->isGPU) {
 +        master = pScreen->current_master;
-+        if (!master)
-+            return;
++	if (!master)
++	  return;
 +        mastersp = rrGetScrPriv(master);
 +    }
 +    else {
@@ -121,10 +132,10 @@ index f0decfc..cb6fce7 100644
          if (mastersp->configChanged) {
              mastersp->lastConfigTime = currentTime;
 diff --git a/randr/randrstr.h b/randr/randrstr.h
-index 212b0a9..d61f20e 100644
+index 55faaed..73132e3 100644
 --- a/randr/randrstr.h
 +++ b/randr/randrstr.h
-@@ -480,6 +480,10 @@ extern _X_EXPORT void
+@@ -486,6 +486,10 @@ extern _X_EXPORT void
   RRDeliverScreenEvent(ClientPtr client, WindowPtr pWin, ScreenPtr pScreen);
  
  /* randr.c */
@@ -136,7 +147,7 @@ index 212b0a9..d61f20e 100644
   * Send all pending events
   */
 diff --git a/randr/rrcrtc.c b/randr/rrcrtc.c
-index e82d050..d57cc08 100644
+index e82d050..d0add47 100644
 --- a/randr/rrcrtc.c
 +++ b/randr/rrcrtc.c
 @@ -39,7 +39,7 @@ RRCrtcChanged(RRCrtcPtr crtc, Bool layoutChanged)
@@ -144,7 +155,7 @@ index e82d050..d57cc08 100644
          rrScrPriv(pScreen);
  
 -        pScrPriv->changed = TRUE;
-+        RRSetChanged(pScreen);
++	RRSetChanged(pScreen);
          /*
           * Send ConfigureNotify on any layout change
           */
@@ -162,7 +173,7 @@ index 1408d6f..fc57bd4 100644
  }
  
 diff --git a/randr/rroutput.c b/randr/rroutput.c
-index 88781ba..922d61f 100644
+index 88781ba..195bef9 100644
 --- a/randr/rroutput.c
 +++ b/randr/rroutput.c
 @@ -36,7 +36,7 @@ RROutputChanged(RROutputPtr output, Bool configChanged)
@@ -170,7 +181,7 @@ index 88781ba..922d61f 100644
      if (pScreen) {
          rrScrPriv(pScreen);
 -        pScrPriv->changed = TRUE;
-+        RRSetChanged(pScreen);
++	RRSetChanged(pScreen);
          if (configChanged)
              pScrPriv->configChanged = TRUE;
      }
diff --git a/0001-randr-cleanup-provider-properly.patch b/0001-randr-cleanup-provider-properly.patch
new file mode 100644
index 0000000..271108b
--- /dev/null
+++ b/0001-randr-cleanup-provider-properly.patch
@@ -0,0 +1,133 @@
+From 9d9444a0796707eaa6a5635e78e2db6a4c04226e Mon Sep 17 00:00:00 2001
+From: Dave Airlie <airlied at redhat.com>
+Date: Wed, 9 Jan 2013 12:58:28 +1000
+Subject: [PATCH] randr: cleanup provider properly
+
+So in the cold plug server shutdown case, we reap the resources
+before we call CloseScreen handlers, so the config->randr_provider
+is a dangling pointer when the xf86CrtcCloseScreen handler is called,
+
+however in the hot screen unplug case, we can't rely on automatically
+reaped resources, so we need to clean up the provider in the xf86CrtcCloseScreen
+case.
+
+This patch provides a cleanup callback from the randr provider removal
+into the DDX so it can cleanup properly, this then gets called by the automatic
+code for cold plug, or if hot unplug it gets called explicitly.
+
+Fixes a number of random server crashes on shutdown
+Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=58174
+Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=891140
+
+Signed-off-by: Dave Airlie <airlied at redhat.com>
+Signed-off-by: Fedora X Ninjas <x at fedoraproject.org>
+---
+ hw/xfree86/modes/xf86Crtc.c    | 12 ++----------
+ hw/xfree86/modes/xf86RandR12.c | 22 ++++++++++++++++++++++
+ randr/randrstr.h               |  6 ++++++
+ randr/rrprovider.c             |  2 ++
+ 4 files changed, 32 insertions(+), 10 deletions(-)
+
+diff --git a/hw/xfree86/modes/xf86Crtc.c b/hw/xfree86/modes/xf86Crtc.c
+index e527abc..1263dd7 100644
+--- a/hw/xfree86/modes/xf86Crtc.c
++++ b/hw/xfree86/modes/xf86Crtc.c
+@@ -743,16 +743,8 @@ xf86CrtcCloseScreen(ScreenPtr screen)
+     }
+     /* detach any providers */
+     if (config->randr_provider) {
+-        if (config->randr_provider->offload_sink) {
+-            DetachOffloadGPU(screen);
+-            config->randr_provider->offload_sink = NULL;
+-        }
+-        else if (config->randr_provider->output_source) {
+-            DetachOutputGPU(screen);
+-            config->randr_provider->output_source = NULL;
+-        }
+-        else if (screen->current_master)
+-            DetachUnboundGPU(screen);
++        RRProviderDestroy(config->randr_provider);
++        config->randr_provider = NULL;
+     }
+     return TRUE;
+ }
+diff --git a/hw/xfree86/modes/xf86RandR12.c b/hw/xfree86/modes/xf86RandR12.c
+index 3530abf..01fc9c5 100644
+--- a/hw/xfree86/modes/xf86RandR12.c
++++ b/hw/xfree86/modes/xf86RandR12.c
+@@ -1885,6 +1885,27 @@ xf86RandR13ConstrainCursorHarder(DeviceIntPtr dev, ScreenPtr screen, int mode, i
+     }
+ }
+ 
++static void
++xf86RandR14ProviderDestroy(ScreenPtr screen, RRProviderPtr provider)
++{
++    ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
++    xf86CrtcConfigPtr config = XF86_CRTC_CONFIG_PTR(scrn);
++    
++    if (config->randr_provider == provider) {
++        if (config->randr_provider->offload_sink) {
++            DetachOffloadGPU(screen);
++            config->randr_provider->offload_sink = NULL;
++        }
++        else if (config->randr_provider->output_source) {
++            DetachOutputGPU(screen);
++            config->randr_provider->output_source = NULL;
++        }
++        else if (screen->current_master)
++            DetachUnboundGPU(screen);
++    }
++    config->randr_provider = NULL;
++}
++
+ static Bool
+ xf86RandR12Init12(ScreenPtr pScreen)
+ {
+@@ -1914,6 +1935,7 @@ xf86RandR12Init12(ScreenPtr pScreen)
+     rp->rrProviderSetProperty = xf86RandR14ProviderSetProperty;
+     rp->rrProviderGetProperty = xf86RandR14ProviderGetProperty;
+     rp->rrCrtcSetScanoutPixmap = xf86CrtcSetScanoutPixmap;
++    rp->rrProviderDestroy = xf86RandR14ProviderDestroy;
+ 
+     pScrn->PointerMoved = xf86RandR12PointerMoved;
+     pScrn->ChangeGamma = xf86RandR12ChangeGamma;
+diff --git a/randr/randrstr.h b/randr/randrstr.h
+index 212b0a9..55faaed 100644
+--- a/randr/randrstr.h
++++ b/randr/randrstr.h
+@@ -232,6 +232,9 @@ typedef Bool (*RRProviderSetOffloadSinkProcPtr)(ScreenPtr pScreen,
+                                          RRProviderPtr offload_sink);
+ 
+ 
++typedef void (*RRProviderDestroyProcPtr)(ScreenPtr pScreen,
++                                         RRProviderPtr provider);
++
+ /* These are for 1.0 compatibility */
+ 
+ typedef struct _rrRefresh {
+@@ -330,6 +333,9 @@ typedef struct _rrScrPriv {
+     Bool discontiguous;
+ 
+     RRProviderPtr provider;
++
++    RRProviderDestroyProcPtr rrProviderDestroy;
++
+ } rrScrPrivRec, *rrScrPrivPtr;
+ 
+ extern _X_EXPORT DevPrivateKeyRec rrPrivKeyRec;
+diff --git a/randr/rrprovider.c b/randr/rrprovider.c
+index c4ed515..b321e62 100644
+--- a/randr/rrprovider.c
++++ b/randr/rrprovider.c
+@@ -389,6 +389,8 @@ RRProviderDestroyResource (pointer value, XID pid)
+     {
+         rrScrPriv(pScreen);
+ 
++        if (pScrPriv->rrProviderDestroy)
++            (*pScrPriv->rrProviderDestroy)(pScreen, provider);
+         pScrPriv->provider = NULL;
+     }
+     free(provider);
+-- 
+1.8.1
+
diff --git a/0001-xf86-actually-set-the-compat-output-in-the-failure-c.patch b/0001-xf86-actually-set-the-compat-output-in-the-failure-c.patch
new file mode 100644
index 0000000..fc6dae8
--- /dev/null
+++ b/0001-xf86-actually-set-the-compat-output-in-the-failure-c.patch
@@ -0,0 +1,35 @@
+From bc9bbe15919f099d57a43c73b7d8cf28ac5ce1d4 Mon Sep 17 00:00:00 2001
+From: Dave Airlie <airlied at redhat.com>
+Date: Wed, 9 Jan 2013 12:53:14 +1000
+Subject: [PATCH 1/2] xf86: actually set the compat output in the failure case
+
+The previous fix for the previous fix, didn't fully work,
+
+If we don't set compat_output we end up doing derferences
+of arrays with -1, leading to valgrind warnings.
+
+Signed-off-by: Dave Airlie <airlied at redhat.com>
+---
+ hw/xfree86/modes/xf86Crtc.c | 6 ++++--
+ 1 file changed, 4 insertions(+), 2 deletions(-)
+
+diff --git a/hw/xfree86/modes/xf86Crtc.c b/hw/xfree86/modes/xf86Crtc.c
+index 77a0218..45764fc 100644
+--- a/hw/xfree86/modes/xf86Crtc.c
++++ b/hw/xfree86/modes/xf86Crtc.c
+@@ -1848,8 +1848,10 @@ SetCompatOutput(xf86CrtcConfigPtr config)
+     }
+ 
+     /* All outputs are disconnected, select one to fake */
+-    if (!output && config->num_output)
+-        output = config->output[0];
++    if (!output && config->num_output) {
++        config->compat_output = 0;
++        output = config->output[config->compat_output];
++    }
+ 
+     return output;
+ }
+-- 
+1.8.1
+
diff --git a/xorg-x11-server.spec b/xorg-x11-server.spec
index 8cb5592..deda580 100644
--- a/xorg-x11-server.spec
+++ b/xorg-x11-server.spec
@@ -43,7 +43,7 @@
 Summary:   X.Org X11 X server
 Name:      xorg-x11-server
 Version:   1.13.1
-Release:   3%{?gitdate:.%{gitdate}}%{dist}
+Release:   4%{?gitdate:.%{gitdate}}%{dist}
 URL:       http://www.x.org
 License:   MIT
 Group:     User Interface/X
@@ -117,12 +117,16 @@ Patch7064: 0001-mieq-Bump-default-queue-size-to-512.patch
 # some hotplug fixes/workaround
 Patch7065: 0001-xfree86-hotplug-cleanup-properly-if-the-screen-fails.patch
 Patch7066: 0001-xf86crtc-don-t-use-display-for-vx-vy-for-gpu-screens.patch
+
 # autoconfig: send events
-Patch7067: 0001-autoconfig-fixup-tell-changed-so-randr-clients-can-t.patch
 
 # on way upstream: fixes for reverse optimus
 Patch8000: 0001-dix-allow-pixmap-dirty-helper-to-be-used-for-non-sha.patch
 Patch8001: 0001-xserver-call-CSR-for-gpus.patch
+Patch8002: 0001-xf86-actually-set-the-compat-output-in-the-failure-c.patch
+Patch8003: 0001-randr-cleanup-provider-properly.patch
+
+Patch8010: 0001-autoconfig-fixup-tell-changed-so-randr-clients-can-t.patch
 
 %global moduledir	%{_libdir}/xorg/modules
 %global drimoduledir	%{_libdir}/dri
@@ -596,6 +600,9 @@ rm -rf $RPM_BUILD_ROOT
 %{xserver_source_dir}
 
 %changelog
+* Wed Jan 09 2013 Dave Airlie <airlied at redhat.com> 1.13.1-4
+- fix bug on server shutdown + valgrind warnings (#891140)
+
 * Tue Jan 08 2013 Dave Airlie <airlied at redhat.com> 1.13.1-3
 - fixes for reverse optimus support
 


More information about the scm-commits mailing list