| Message ID | 20190606222751.32567-11-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/vkms: rework crc worker | expand |
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>
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);
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(-)