Re: [PATCH] drm: damage_helper: Fix race checking plane->state->fb

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Sep 4, 2019 at 10:29 PM Sean Paul <sean@xxxxxxxxxx> wrote:
>
> From: Sean Paul <seanpaul@xxxxxxxxxxxx>
>
> Since the dirtyfb ioctl doesn't give us any hints as to which plane is
> scanning out the fb it's marking as damaged, we need to loop through
> planes to find it.
>
> Currently we just reach into plane state and check, but that can race
> with another commit changing the fb out from under us. This patch locks
> the plane before checking the fb and will release the lock if the plane
> is not displaying the dirty fb.
>
> Fixes: b9fc5e01d1ce ("drm: Add helper to implement legacy dirtyfb")
> Cc: Rob Clark <robdclark@xxxxxxxxx>
> Cc: Deepak Rawat <drawat@xxxxxxxxxx>
> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
> Cc: Thomas Hellstrom <thellstrom@xxxxxxxxxx>
> Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> Cc: Maxime Ripard <maxime.ripard@xxxxxxxxxxx>
> Cc: Sean Paul <sean@xxxxxxxxxx>
> Cc: David Airlie <airlied@xxxxxxxx>
> Cc: Daniel Vetter <daniel@xxxxxxxx>
> Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx
> Cc: <stable@xxxxxxxxxxxxxxx> # v5.0+
> Reported-by: Daniel Vetter <daniel@xxxxxxxx>
> Signed-off-by: Sean Paul <seanpaul@xxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/drm_damage_helper.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_damage_helper.c b/drivers/gpu/drm/drm_damage_helper.c
> index 8230dac01a89..3a4126dc2520 100644
> --- a/drivers/gpu/drm/drm_damage_helper.c
> +++ b/drivers/gpu/drm/drm_damage_helper.c
> @@ -212,8 +212,14 @@ int drm_atomic_helper_dirtyfb(struct drm_framebuffer *fb,
>         drm_for_each_plane(plane, fb->dev) {
>                 struct drm_plane_state *plane_state;
>
> -               if (plane->state->fb != fb)
> +               ret = drm_modeset_lock(&plane->mutex, state->acquire_ctx);
> +               if (ret)

I think for paranoid safety we should have a WARN_ON(ret == -EALREADY)
here. It should be impossible, but if it's not for some oddball
reason, we'll blow up.

With that: Reviewed-by: Daniel Vetter <daniel@xxxxxxxx>

But please give this a spin with some workloads and the ww_mutex
slowpath debugging enabled, just to makre sure.
-Daniel

> +                       goto out;
> +
> +               if (plane->state->fb != fb) {
> +                       drm_modeset_unlock(&plane->mutex);
>                         continue;
> +               }
>
>                 plane_state = drm_atomic_get_plane_state(state, plane);
>                 if (IS_ERR(plane_state)) {
> --
> Sean Paul, Software Engineer, Google / Chromium OS
>


-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux