[10/10] drm/vkms: No need for ->pages_lock in crc work anymore
diff mbox series

Message ID 20190606222751.32567-11-daniel.vetter@ffwll.ch
State New
Headers show
Series
  • drm/vkms: rework crc worker
Related show

Commit Message

Daniel Vetter June 6, 2019, 10:27 p.m. UTC
We're now guaranteed to no longer race against prepare_fb/cleanup_fb,
which means we can access ->vaddr without having to hold a lock.

Before the previous patches it was fairly easy to observe the cursor
->vaddr being invalid, but that's now gone, so we can upgrade to a
full WARN_ON.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
---
 drivers/gpu/drm/vkms/vkms_crc.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

Comments

Rodrigo Siqueira June 12, 2019, 1:47 p.m. UTC | #1
On Thu, Jun 6, 2019 at 7:28 PM Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>
> We're now guaranteed to no longer race against prepare_fb/cleanup_fb,
> which means we can access ->vaddr without having to hold a lock.
>
> Before the previous patches it was fairly easy to observe the cursor
> ->vaddr being invalid, but that's now gone, so we can upgrade to a
> full WARN_ON.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> ---
>  drivers/gpu/drm/vkms/vkms_crc.c | 13 ++-----------
>  1 file changed, 2 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_crc.c b/drivers/gpu/drm/vkms/vkms_crc.c
> index 0d31cfc32042..4b3146d83265 100644
> --- a/drivers/gpu/drm/vkms/vkms_crc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crc.c
> @@ -97,16 +97,10 @@ static void compose_cursor(struct vkms_crc_data *cursor_crc,
>         cursor_obj = drm_gem_fb_get_obj(&cursor_crc->fb, 0);
>         cursor_vkms_obj = drm_gem_to_vkms_gem(cursor_obj);
>
> -       mutex_lock(&cursor_vkms_obj->pages_lock);
> -       if (!cursor_vkms_obj->vaddr) {
> -               DRM_WARN("cursor plane vaddr is NULL");
> -               goto out;
> -       }
> +       if (WARN_ON(!cursor_vkms_obj->vaddr))
> +               return;
>
>         blend(vaddr_out, cursor_vkms_obj->vaddr, primary_crc, cursor_crc);
> -
> -out:
> -       mutex_unlock(&cursor_vkms_obj->pages_lock);
>  }
>
>  static uint32_t _vkms_get_crc(struct vkms_crc_data *primary_crc,
> @@ -123,15 +117,12 @@ static uint32_t _vkms_get_crc(struct vkms_crc_data *primary_crc,
>                 return 0;
>         }
>
> -       mutex_lock(&vkms_obj->pages_lock);
>         if (WARN_ON(!vkms_obj->vaddr)) {
> -               mutex_unlock(&vkms_obj->pages_lock);
>                 kfree(vaddr_out);
>                 return crc;
>         }
>
>         memcpy(vaddr_out, vkms_obj->vaddr, vkms_obj->gem.size);
> -       mutex_unlock(&vkms_obj->pages_lock);
>
>         if (cursor_crc)
>                 compose_cursor(cursor_crc, primary_crc, vaddr_out);
> --
> 2.20.1
>
Reviewed-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
Tested-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>

Patch
diff mbox series

diff --git a/drivers/gpu/drm/vkms/vkms_crc.c b/drivers/gpu/drm/vkms/vkms_crc.c
index 0d31cfc32042..4b3146d83265 100644
--- a/drivers/gpu/drm/vkms/vkms_crc.c
+++ b/drivers/gpu/drm/vkms/vkms_crc.c
@@ -97,16 +97,10 @@  static void compose_cursor(struct vkms_crc_data *cursor_crc,
 	cursor_obj = drm_gem_fb_get_obj(&cursor_crc->fb, 0);
 	cursor_vkms_obj = drm_gem_to_vkms_gem(cursor_obj);
 
-	mutex_lock(&cursor_vkms_obj->pages_lock);
-	if (!cursor_vkms_obj->vaddr) {
-		DRM_WARN("cursor plane vaddr is NULL");
-		goto out;
-	}
+	if (WARN_ON(!cursor_vkms_obj->vaddr))
+		return;
 
 	blend(vaddr_out, cursor_vkms_obj->vaddr, primary_crc, cursor_crc);
-
-out:
-	mutex_unlock(&cursor_vkms_obj->pages_lock);
 }
 
 static uint32_t _vkms_get_crc(struct vkms_crc_data *primary_crc,
@@ -123,15 +117,12 @@  static uint32_t _vkms_get_crc(struct vkms_crc_data *primary_crc,
 		return 0;
 	}
 
-	mutex_lock(&vkms_obj->pages_lock);
 	if (WARN_ON(!vkms_obj->vaddr)) {
-		mutex_unlock(&vkms_obj->pages_lock);
 		kfree(vaddr_out);
 		return crc;
 	}
 
 	memcpy(vaddr_out, vkms_obj->vaddr, vkms_obj->gem.size);
-	mutex_unlock(&vkms_obj->pages_lock);
 
 	if (cursor_crc)
 		compose_cursor(cursor_crc, primary_crc, vaddr_out);