Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] drm/xe/pcode: duplicate left shift of wait in pcode_try_request() - [High] drm/xe/pcode: uncapped wait added to slept in pcode_try_request() --
commit b0181a87e207629275e050ae1f5d781e540c4b82 Author: Alan Previn <[email protected]> drm/xe/pcode: Fix pcode_try_request polling delays This commit clamps the atomic udelay() argument in pcode_try_request() to MAX_UDELAY_MS * 1000 and adds a 100 ms upper bound to the exponential backoff wait interval. > diff --git a/drivers/gpu/drm/xe/xe_pcode.c b/drivers/gpu/drm/xe/xe_pcode.c > index 266deecbb100a..25d1a354ee369 100644 > --- a/drivers/gpu/drm/xe/xe_pcode.c > +++ b/drivers/gpu/drm/xe/xe_pcode.c > @@ -146,6 +146,7 @@ int xe_pcode_read(struct xe_tile *tile, u32 mbox, u32 > *val0, u32 *val1) > return err; > } > > +#define __XE_MMIO_WAIT_MAX_BACKOFF_100MS (100 * USEC_PER_MSEC) > static int pcode_try_request(struct xe_tile *tile, u32 mbox, > u32 request, u32 reply_mask, u32 reply, > int *status, bool atomic, int timeout_us, bool > locked) > @@ -165,10 +166,11 @@ static int pcode_try_request(struct xe_tile *tile, u32 > mbox, > return 0; > > if (atomic) > - udelay(wait); > + udelay(min_t(int, wait, (MAX_UDELAY_MS * 1000))); > else > usleep_range(wait, wait << 1); > wait <<= 1; > + wait = min_t(int, wait << 1, __XE_MMIO_WAIT_MAX_BACKOFF_100MS); [Severity: High] Should the existing wait <<= 1 statement above be removed? Leaving both shifts in pcode_try_request() shifts wait left twice on every iteration, quadrupling wait at each step (10 -> 40 -> 160 -> 640 -> 2560 us) instead of doubling it. Because the loop header accumulates elapsed time via slept += wait after the loop body finishes: drivers/gpu/drm/xe/xe_pcode.c:pcode_try_request() { for (slept = 0; slept < timeout_us; slept += wait) { slept also advances by 4x the delay just performed, causing xe_pcode_ready() and xe_pcode_request() to time out with -ETIMEDOUT before timeout_us has actually elapsed. [Severity: High] When atomic is true, udelay() is clamped to MAX_UDELAY_MS * 1000 (for example, 5000 us on x86 or 2000 us on arm64), but wait continues to grow up to __XE_MMIO_WAIT_MAX_BACKOFF_100MS (100000 us) and is added directly to slept in the loop header: drivers/gpu/drm/xe/xe_pcode.c:pcode_try_request() { for (slept = 0; slept < timeout_us; slept += wait) { When xe_pcode_request() retries with preemption disabled: drivers/gpu/drm/xe/xe_pcode.c:xe_pcode_request() { preempt_disable(); ret = pcode_try_request(tile, mbox, request, reply_mask, reply, &status, true, 50 * 1000, true); preempt_enable(); won't slept accumulate the unclamped wait value once wait exceeds MAX_UDELAY_MS * 1000, causing the 50 ms atomic retry to return -ETIMEDOUT after only a fraction of the intended 50 ms delay? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
