> Subject: [PATCH v8 3/7] drm/atomic: Allocate atomic_state at the beginning of
> atomic_ioctl
>
> Move atomic_state allocation to the beginning of the atomic_ioctl to
> accommodate drm_mode_atomic_err_code usage for returning error code on
> failures.
> As atomic state is required for drm_mode_atomic_err_code to store the error
> codes.
>
> v7: Reframe commit message (Suraj)
> v8: Moved the clearing fence change to a different patch (Suraj/Louis)
>
> Signed-off-by: Arun R Murthy <[email protected]>
> ---
> drivers/gpu/drm/drm_atomic_uapi.c | 22 +++++++++++++++-------
> 1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index
> dc013a22bf265512a4fa1edf0ae90931ff0d35e6..3042e6c2616fb09403c13a86
> 30c8819a39cf55d4 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -1576,6 +1576,14 @@ int drm_mode_atomic_ioctl(struct drm_device
> *dev,
> if (!drm_core_check_feature(dev, DRIVER_ATOMIC))
> return -EOPNOTSUPP;
>
> + state = drm_atomic_state_alloc(dev);
> + if (!state)
> + return -ENOMEM;
> +
> + drm_modeset_acquire_init(&ctx,
> DRM_MODESET_ACQUIRE_INTERRUPTIBLE);
> + state->acquire_ctx = &ctx;
> + state->allow_modeset = !!(arg->flags &
> DRM_MODE_ATOMIC_ALLOW_MODESET);
> +
I think this patch alone creates a memory leak since we usually return in case
of any invalid flags are given and do not allocate state.
Now we just return after allocating state.
You do fix this in 5th patch which add the goto out line so I suggest you get
that part of the code in this patch.
Other than that in goto out we only clear state if ret == -EDEADLK so that
condition needs to be modified too.
Regards,
Suraj Kandpal
> /* disallow for userspace that has not enabled atomic cap (even
> * though this may be a bit overkill, since legacy userspace
> * wouldn't know how to call this ioctl) @@ -1614,13 +1622,6 @@ int
> drm_mode_atomic_ioctl(struct drm_device *dev,
> return -EINVAL;
> }
>
> - state = drm_atomic_state_alloc(dev);
> - if (!state)
> - return -ENOMEM;
> -
> - drm_modeset_acquire_init(&ctx,
> DRM_MODESET_ACQUIRE_INTERRUPTIBLE);
> - state->acquire_ctx = &ctx;
> - state->allow_modeset = !!(arg->flags &
> DRM_MODE_ATOMIC_ALLOW_MODESET);
> state->plane_color_pipeline = file_priv->plane_color_pipeline;
>
> retry:
> @@ -1719,6 +1720,13 @@ int drm_mode_atomic_ioctl(struct drm_device
> *dev,
> }
>
> out:
> + /* Update the error code if any error to allow user handling it */
> + if (ret < 0 && arg->reserved) {
> + if (copy_to_user(error_code_ptr, &state->error_code,
> + sizeof(state->error_code)))
> + return -EFAULT;
> + }
> +
> complete_signaling(dev, state, fence_state, num_fences, !ret);
>
> if (ret == -EDEADLK) {
>
> --
> 2.25.1
