Re: [PATCH 2/2] randr: cleanup provider properly

Knut Petersen Sat, 12 Jan 2013 02:22:39 -0800

On 11.01.2013 00:02, Peter Hutterer wrote:

On Wed, Jan 09, 2013 at 01:14:55PM +1000, Dave Airlie wrote:

From: Dave Airlie <[email protected]>


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 <[email protected]>

seems correct, as far as I understand this code

Reviewed-by: Peter Hutterer <[email protected]>


Tested here, fixes bug 58174. Definitely needed in master asap.

Tested-by: Knut Petersen <[email protected]>


Cheers,
    Peter

---
  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 45764fc..1905b18 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 a16302f..cc3ab1d 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

_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 2/2] randr: cleanup provider properly

Reply via email to