Xext/shm: Detach SHM segment after Pixmap is released

Submitted by Chris Wilson on Oct. 16, 2014, 1:09 p.m.

Details

Message ID 1413464948-24828-1-git-send-email-chris@chris-wilson.co.uk
State Accepted
Commit 9b29fa957a397664463c7c78fbcc2f34d1993271
Headers show

Not browsing as part of any series.

Commit Message

Chris Wilson Oct. 16, 2014, 1:09 p.m.
The GPU may still have a reference to the SHM segment which would only
be finally released when the Pixmap is destroy. So we can only detach
the SHM segment (and thereby making the memory unaccessible) after the
backend has had a chance to flush any remaining references.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=85058
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 Xext/shm.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

Patch hide | download patch | download mbox

diff --git a/Xext/shm.c b/Xext/shm.c
index 4dad8b6..b787918 100644
--- a/Xext/shm.c
+++ b/Xext/shm.c
@@ -248,21 +248,20 @@  ShmDestroyPixmap(PixmapPtr pPixmap)
 {
     ScreenPtr pScreen = pPixmap->drawable.pScreen;
     ShmScrPrivateRec *screen_priv = ShmGetScreenPriv(pScreen);
+    void *shmdesc = NULL;
     Bool ret;
 
-    if (pPixmap->refcnt == 1) {
-        ShmDescPtr shmdesc;
-
-        shmdesc = (ShmDescPtr) dixLookupPrivate(&pPixmap->devPrivates,
-                                                shmPixmapPrivateKey);
-        if (shmdesc)
-            ShmDetachSegment((void *) shmdesc, pPixmap->drawable.id);
-    }
+    if (pPixmap->refcnt == 1)
+        shmdesc = dixLookupPrivate(&pPixmap->devPrivates, shmPixmapPrivateKey);
 
     pScreen->DestroyPixmap = screen_priv->destroyPixmap;
     ret = (*pScreen->DestroyPixmap) (pPixmap);
     screen_priv->destroyPixmap = pScreen->DestroyPixmap;
     pScreen->DestroyPixmap = ShmDestroyPixmap;
+
+    if (shmdesc)
+	ShmDetachSegment(shmdesc, pPixmap->drawable.id);
+
     return ret;
 }
 

Comments

On Thu, Oct 16, 2014 at 02:09:08PM +0100, Chris Wilson wrote:
> The GPU may still have a reference to the SHM segment which would only
> be finally released when the Pixmap is destroy. So we can only detach
> the SHM segment (and thereby making the memory unaccessible) after the
> backend has had a chance to flush any remaining references.
 
Reported-and-tested-by: gedgon@gmail.com
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=85058
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
On Thu, 2014-10-16 at 16:22 +0100, Chris Wilson wrote:
> On Thu, Oct 16, 2014 at 02:09:08PM +0100, Chris Wilson wrote:
> > The GPU may still have a reference to the SHM segment which would only
> > be finally released when the Pixmap is destroy. So we can only detach
> > the SHM segment (and thereby making the memory unaccessible) after the
> > backend has had a chance to flush any remaining references.
>  
> Reported-and-tested-by: gedgon@gmail.com

Reviewed-by: Adam Jackson <ajax@redhat.com>

- ajax
Adam Jackson <ajax@nwnk.net> writes:

> On Thu, 2014-10-16 at 16:22 +0100, Chris Wilson wrote:
>> On Thu, Oct 16, 2014 at 02:09:08PM +0100, Chris Wilson wrote:
>> > The GPU may still have a reference to the SHM segment which would only
>> > be finally released when the Pixmap is destroy. So we can only detach
>> > the SHM segment (and thereby making the memory unaccessible) after the
>> > backend has had a chance to flush any remaining references.
>>  
>> Reported-and-tested-by: gedgon@gmail.com
>
> Reviewed-by: Adam Jackson <ajax@redhat.com>

Merged.
   5adc201..9b29fa9  master -> master