drm/atomic: Cleanup on error properly in the atomic ioctl.

It's probably allowed to leave old_fb set to garbage when unlocking,
but to prevent undefined behavior unset it just in case.

Also crtc_state->event could be NULL on memory allocation failure,
in which case event_space is increased for no reason.

Note: Contains some general simplification of the cleanup code too.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
[danvet: Add note about the other changes in here. And fix long line
while at it.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

1 files changed, 30 insertions, 34 deletions

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index bd7f723c708e..acebd1617264 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1463,18 +1463,18 @@ retry:
 
                 if (get_user(obj_id, objs_ptr + copied_objs)) {
                         ret = -EFAULT;
-                        goto fail;
+                        goto out;
                 }
 
                 obj = drm_mode_object_find(dev, obj_id, DRM_MODE_OBJECT_ANY);
                 if (!obj || !obj->properties) {
                         ret = -ENOENT;
-                        goto fail;
+                        goto out;
                 }
 
                 if (get_user(count_props, count_props_ptr + copied_objs)) {
                         ret = -EFAULT;
-                        goto fail;
+                        goto out;
                 }
 
                 copied_objs++;
@@ -1486,25 +1486,25 @@ retry:
 
                         if (get_user(prop_id, props_ptr + copied_props)) {
                                 ret = -EFAULT;
-                                goto fail;
+                                goto out;
                         }
 
                         prop = drm_property_find(dev, prop_id);
                         if (!prop) {
                                 ret = -ENOENT;
-                                goto fail;
+                                goto out;
                         }
 
                         if (copy_from_user(&prop_value,
                                            prop_values_ptr + copied_props,
                                            sizeof(prop_value))) {
                                 ret = -EFAULT;
-                                goto fail;
+                                goto out;
                         }
 
                         ret = atomic_set_prop(state, obj, prop, prop_value);
                         if (ret)
-                                goto fail;
+                                goto out;
 
                         copied_props++;
                 }
@@ -1523,7 +1523,7 @@ retry:
                         e = create_vblank_event(dev, file_priv, arg->user_data);
                         if (!e) {
                                 ret = -ENOMEM;
-                                goto fail;
+                                goto out;
                         }
 
                         crtc_state->event = e;
@@ -1533,13 +1533,15 @@ retry:
         if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY) {
                 ret = drm_atomic_check_only(state);
                 /* _check_only() does not free state, unlike _commit() */
-                drm_atomic_state_free(state);
+                if (!ret)
+                        drm_atomic_state_free(state);
         } else if (arg->flags & DRM_MODE_ATOMIC_NONBLOCK) {
                 ret = drm_atomic_async_commit(state);
         } else {
                 ret = drm_atomic_commit(state);
         }
 
+out:
         /* if succeeded, fixup legacy plane crtc/fb ptrs before dropping
          * locks (ie. while it is still safe to deref plane->state).  We
          * need to do this here because the driver entry points cannot
@@ -1552,41 +1554,35 @@ retry:
                                 drm_framebuffer_reference(new_fb);
                         plane->fb = new_fb;
                         plane->crtc = plane->state->crtc;
-                } else {
-                        plane->old_fb = NULL;
-                }
-                if (plane->old_fb) {
-                        drm_framebuffer_unreference(plane->old_fb);
-                        plane->old_fb = NULL;
+
+                        if (plane->old_fb)
+                                drm_framebuffer_unreference(plane->old_fb);
                 }
+                plane->old_fb = NULL;
         }
 
-        drm_modeset_drop_locks(&ctx);
-        drm_modeset_acquire_fini(&ctx);
-
-        return ret;
+        if (ret == -EDEADLK) {
+                drm_atomic_state_clear(state);
+                drm_modeset_backoff(&ctx);
+                goto retry;
+        }
 
-fail:
-        if (ret == -EDEADLK)
-                goto backoff;
+        if (ret) {
+                if (arg->flags & DRM_MODE_PAGE_FLIP_EVENT) {
+                        for_each_crtc_in_state(state, crtc, crtc_state, i) {
+                                if (!crtc_state->event)
+                                        continue;
 
-        if (arg->flags & DRM_MODE_PAGE_FLIP_EVENT) {
-                for_each_crtc_in_state(state, crtc, crtc_state, i) {
-                        destroy_vblank_event(dev, file_priv, crtc_state->event);
-                        crtc_state->event = NULL;
+                                destroy_vblank_event(dev, file_priv,
+                                                     crtc_state->event);
+                        }
                 }
-        }
 
-        drm_atomic_state_free(state);
+                drm_atomic_state_free(state);
+        }
 
         drm_modeset_drop_locks(&ctx);
         drm_modeset_acquire_fini(&ctx);
 
         return ret;
-
-backoff:
-        drm_atomic_state_clear(state);
-        drm_modeset_backoff(&ctx);
-
-        goto retry;
 }