[PATCH] present: Avoid dereferencing vblank past a successful call to present_flip()

[Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>]

The driver may complete the flip instantaneously resulting in an immediate
call to present_flip_notify(). This has the side-effect of destroying
the vblank. We could add a reference count to the vblank structure to
defer the destruction, or make the assume that further accesses are
invalid and cache all the values we need in local variables before the
call.

Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
---
 present/present.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/present/present.c b/present/present.c
index 73d5f69..75e9e00 100644
--- a/present/present.c
+++ b/present/present.c
@@ -522,6 +522,7 @@ present_wait_fence_triggered(void *param)
 static void
 present_execute(present_vblank_ptr vblank, uint64_t ust, uint64_t crtc_msc)
 {
+    PixmapPtr                   pixmap = vblank->pixmap;
     WindowPtr                   window = vblank->window;
     ScreenPtr                   screen = window->drawable.pScreen;
     present_screen_priv_ptr     screen_priv = present_screen_priv(screen);
@@ -537,11 +538,11 @@ present_execute(present_vblank_ptr vblank, uint64_t ust, uint64_t crtc_msc)
     xorg_list_del(&vblank->event_queue);
     vblank->queued = FALSE;
 
-    if (vblank->pixmap && vblank->window) {
-
+    if (pixmap) {
         if (vblank->flip && screen_priv->flip_pending == NULL && !screen_priv->unflip_event_id) {
+            RegionPtr damage = vblank->update;
 
-            DebugPresent(("\tf %p %8lld: %08lx -> %08lx\n", vblank, crtc_msc, vblank->pixmap->drawable.id, vblank->window->drawable.id));
+            DebugPresent(("\tf %p %8lld: %08lx -> %08lx\n", vblank, crtc_msc, pixmap->drawable.id, window->drawable.id));
             /* Prepare to flip by placing it in the flip queue and
              * and sticking it into the flip_pending field
              */
@@ -550,8 +551,7 @@ present_execute(present_vblank_ptr vblank, uint64_t ust, uint64_t crtc_msc)
             xorg_list_add(&vblank->event_queue, &present_flip_queue);
             /* Try to flip
              */
-            if (present_flip(vblank->crtc, vblank->event_id, vblank->target_msc, vblank->pixmap, vblank->sync_flip)) {
-                RegionPtr damage;
+            if (present_flip(vblank->crtc, vblank->event_id, vblank->target_msc, pixmap, vblank->sync_flip)) {
 
                 /* Fix window pixmaps:
                  *  1) Restore previous flip window pixmap
@@ -560,18 +560,17 @@ present_execute(present_vblank_ptr vblank, uint64_t ust, uint64_t crtc_msc)
                 if (screen_priv->flip_window && screen_priv->flip_window != window)
                     present_set_tree_pixmap(screen_priv->flip_window,
                                               (*screen->GetScreenPixmap)(screen));
-                present_set_tree_pixmap(vblank->window, vblank->pixmap);
-                present_set_tree_pixmap(screen->root, vblank->pixmap);
+                present_set_tree_pixmap(window, pixmap);
+                present_set_tree_pixmap(screen->root, pixmap);
 
                 /* Report update region as damaged
                  */
-                if (vblank->update) {
-                    damage = vblank->update;
+                if (damage)
                     RegionIntersect(damage, damage, &window->clipList);
-                } else
+                else
                     damage = &window->clipList;
 
-                DamageDamageRegion(&vblank->window->drawable, damage);
+                DamageDamageRegion(&window->drawable, damage);
                 return;
             }
 
@@ -581,7 +580,7 @@ present_execute(present_vblank_ptr vblank, uint64_t ust, uint64_t crtc_msc)
             screen_priv->flip_pending = NULL;
             vblank->flip = FALSE;
         }
-        DebugPresent(("\tc %p %8lld: %08lx -> %08lx\n", vblank, crtc_msc, vblank->pixmap->drawable.id, vblank->window->drawable.id));
+        DebugPresent(("\tc %p %8lld: %08lx -> %08lx\n", vblank, crtc_msc, pixmap->drawable.id, window->drawable.id));
         if (screen_priv->flip_pending) {
 
             /* Check pending flip
@@ -595,7 +594,7 @@ present_execute(present_vblank_ptr vblank, uint64_t ust, uint64_t crtc_msc)
             if (window == screen_priv->flip_window)
                 present_unflip(screen);
         }
-        present_copy_region(&window->drawable, vblank->pixmap, vblank->update, vblank->x_off, vblank->y_off);
+        present_copy_region(&window->drawable, pixmap, vblank->update, vblank->x_off, vblank->y_off);
 
         /* present_copy_region sticks the region into a scratch GC,
          * which is then freed, freeing the region
@@ -603,13 +602,13 @@ present_execute(present_vblank_ptr vblank, uint64_t ust, uint64_t crtc_msc)
         vblank->update = NULL;
         present_flush(window);
 
-        present_pixmap_idle(vblank->pixmap, vblank->window, vblank->serial, vblank->idle_fence);
+        present_pixmap_idle(pixmap, window, vblank->serial, vblank->idle_fence);
     }
 
     /* Compute correct CompleteMode
      */
     if (vblank->kind == PresentCompleteKindPixmap) {
-        if (vblank->pixmap && vblank->window)
+        if (pixmap)
             mode = PresentCompleteModeCopy;
         else
             mode = PresentCompleteModeSkip;
-- 
2.0.0.rc4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx