Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [Critical] drm_crtc_vblank_start_timer() deadlocks when synchronously spinning on hrtimer cancellation if the timer callback is concurrently running. -- commit ea5cbfbf25c3ea02070a1ec1d83634d5505e475b Author: Roman Ilin <[email protected]> drm/vblank: Don't arm vblank timer with invalid frame duration This commit fixes a bug where an extremely small pixel clock results in a computed frame duration that wraps to a negative value, causing the vblank hrtimer to fail and stall pending page flips. It updates the calculation to use u64 and adds overflow checks, returning an error to fall back to immediate vblank events. > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c > index f90fb2d13e423..629b9fcc7a52d 100644 > --- a/drivers/gpu/drm/drm_vblank.c > +++ b/drivers/gpu/drm/drm_vblank.c [ ... ] > @@ -2221,6 +2239,7 @@ int drm_crtc_vblank_start_timer(struct drm_crtc *crtc) > struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc); > struct drm_vblank_crtc_timer *vtimer = &vblank->vblank_timer; > unsigned long flags; > + int ret; > > if (!vtimer->crtc) { > /* > @@ -2239,7 +2258,9 @@ int drm_crtc_vblank_start_timer(struct drm_crtc *crtc) > hrtimer_try_to_cancel(&vtimer->timer); > } [Severity: Critical] This isn't a bug introduced by this patch, but does this busy-wait loop cause a deadlock if the timer callback is running concurrently? When drm_vblank_enable() is called, it takes dev->vblank_time_lock before calling down to __enable_vblank(), which eventually calls drm_crtc_vblank_start_timer(). If the timer fires on another CPU at this exact moment, this loop will spin indefinitely waiting for the callback to finish. However, the timer callback executes drm_handle_vblank(), which tries to acquire dev->vblank_time_lock: drm_vblank_timer_function() -> drm_handle_vblank() { ... spin_lock(&dev->vblank_time_lock); } Could this result in a deadlock between the spinning thread holding vblank_time_lock and the timer callback trying to acquire it? > - drm_calc_timestamping_constants(crtc, &crtc->mode); > + ret = drm_calc_timestamping_constants(crtc, &crtc->mode); > + if (ret) > + return ret; > > spin_lock_irqsave(&vtimer->interval_lock, flags); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
