On Sun, Jul 26, 2026 at 10:57 AM <[email protected]> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s)
> to consider:
> - [Medium] The preemption timeout calculation fails to account for the
> hangcheck progress multiplier, causing premature job termination for
> long-running jobs.
> --
>
> --- Patch [1]: [PATCH] drm/msm: Preemption timeout should be greater than
> hangcheck ---
> commit 5454ddf4018869efeec2d7c8f2767056ad2612d9
> Author: Rob Clark <[email protected]>
>
> drm/msm: Preemption timeout should be greater than hangcheck
>
> This commit changes the preemption timeout from a hard-coded 10 seconds to
> the maximum of 10 seconds and the configured hangcheck period. This is
> intended to prevent the preemption timer from prematurely killing
> long-running jobs that are otherwise compliant with the hangcheck timeout.
>
> > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_preempt.h
> > b/drivers/gpu/drm/msm/adreno/a6xx_preempt.h
> > index df36c945b836b..178017aad26e6 100644
> > --- a/drivers/gpu/drm/msm/adreno/a6xx_preempt.h
> > +++ b/drivers/gpu/drm/msm/adreno/a6xx_preempt.h
> > @@ -35,6 +35,14 @@ static inline void set_preempt_state(struct a6xx_gpu
> > *gpu,
> > smp_mb__after_atomic();
> > }
> >
> > +static inline void set_preempt_timer(struct a6xx_gpu *gpu)
> > +{
> > + struct msm_drm_private *priv = gpu->base.base.dev->dev_private;
> > + unsigned t = max(10000, priv->hangcheck_period);
>
> [Severity: Medium]
> Will this under-calculate the true hangcheck timeout when progress detection
> is enabled?
>
> According to msm_ringbuffer.h, when progress detection is enabled, the
> hangcheck timer duration is halved and allows up to
> DRM_MSM_HANGCHECK_PROGRESS_RETRIES extra cycles.
>
> If a user sets a large hangcheck period via debugfs to allow for long
> compute shaders, wouldn't using priv->hangcheck_period directly here result
> in a preemption timeout that is shorter than the actual allowed hangcheck
> time, potentially still killing the job prematurely?
>
yes, but the progress detection is based on detecting that the CP
advanced, which won't be the case for long running compute shaders
BR,
-R
> > +
> > + mod_timer(&gpu->preempt_timer, jiffies + msecs_to_jiffies(t));
> > +}
> > +
> > /* Write the most recent wptr for the given ring into the hardware */
> > static inline void update_wptr(struct a6xx_gpu *a6xx_gpu, struct
> > msm_ringbuffer *ring)
> > {
>
> --
> Sashiko AI review ·
> https://sashiko.dev/#/patchset/[email protected]?part=1
