Am 27.05.26 um 17:22 schrieb [email protected]:
Thank you for your contribution! Sashiko AI review found 1 potential issue(s)
to consider:
Pre-existing issues:
- [High] Plane atomic_update callbacks indirectly call sleeping functions
(mutex_lock/mutex_lock_interruptible) from an atomic context.
--
Preexisting issues should be fixed separately.
commit deccb03fba43ed6006d88ae9933240eb52b52fee
Author: Thomas Zimmermann <[email protected]>
drm/vmwgfx: Remove unused field struct vmwgfx_du_update_plane.old_state
This commit removes the unused old_state field from the
vmw_du_update_plane structure and updates all caller functions to
accommodate this change.
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
[ ... ]
@@ -721,7 +717,6 @@ static void
vmw_sou_primary_plane_atomic_update(struct drm_plane *plane,
struct drm_atomic_commit *state)
{
- struct drm_plane_state *old_state =
drm_atomic_get_old_plane_state(state, plane);
struct drm_plane_state *new_state =
drm_atomic_get_new_plane_state(state, plane);
struct drm_crtc *crtc = new_state->crtc;
struct vmw_fence_obj *fence = NULL;
@@ -734,12 +729,9 @@ vmw_sou_primary_plane_atomic_update(struct drm_plane
*plane,
vmw_framebuffer_to_vfb(new_state->fb);
if (vfb->bo)
- ret = vmw_sou_plane_update_bo(dev_priv, plane,
- old_state, vfb, &fence);
+ ret = vmw_sou_plane_update_bo(dev_priv, plane, vfb,
&fence);
else
- ret = vmw_sou_plane_update_surface(dev_priv, plane,
- old_state, vfb,
- &fence);
+ ret = vmw_sou_plane_update_surface(dev_priv, plane, vfb,
&fence);
[Severity: High]
This is a pre-existing issue, but does this code path end up calling sleeping
functions from an atomic context?
Looking at the call chain, vmw_sou_primary_plane_atomic_update() is a plane
atomic update callback, which runs in atomic context where sleeping is
forbidden. However, it calls vmw_sou_plane_update_surface(), which assigns a
mutex pointer to the update structure:
vmw_sou_plane_update_surface()
srf_update.base.mutex = &dev_priv->cmdbuf_mutex;
vmw_du_helper_plane_update(&srf_update.base);
This closure is then passed down to vmw_du_helper_plane_update(), which calls
vmw_validation_prepare(). That function then takes the lock using
mutex_lock() or mutex_lock_interruptible().
Could this lead to system instability or deadlocks if the atomic commit runs
in a non-blocking path where sleeping is not allowed?
[ ... ]
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)
