Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] Multiplication of `status->busy_time` by 100 overflows the 32-bit `unsigned long` type on 32-bit architectures, leading to completely incorrect utilization percentages in the debug output. - [Medium] Meaningful code and commit message mismatch regarding the clamping logic. --
--- Patch [2]: [PATCH v3 02/13] drm/panfrost: Prevent division by 0 --- commit 5fb0fa4b6ede9db49d32560487844d56d5235d47 Author: Adrián Larumbe <[email protected]> drm/panfrost: Prevent division by 0 When updating and debug-printing devfreq stats, in the very unlikely offchance that total device time is less than 100 ns, clamp it to 0 to avoid division by 0. [Severity: Medium] The commit message says "clamp it to 0 to avoid division by 0". Is this description accurate? Clamping a divisor to 0 would guarantee a division by zero, and the actual code clamps the minimum value to 1 instead of 0. > diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c > b/drivers/gpu/drm/panfrost/panfrost_devfreq.c > index b51c307788110..0fe81d259274e 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c > +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c > @@ -78,7 +78,7 @@ static int panfrost_devfreq_get_dev_status(struct device > *dev, > > dev_dbg(pfdev->base.dev, "busy %lu total %lu %lu %% freq %lu MHz\n", > status->busy_time, status->total_time, > - status->busy_time / (status->total_time / 100), > + status->busy_time * 100 / MAX(status->total_time, 1), [Severity: Medium] Will this multiplication overflow on 32-bit architectures? Because `status->busy_time` is in nanoseconds, a fully busy GPU over a typical 50ms devfreq polling interval results in 50,000,000 ns. Multiplying this by 100 yields 5,000,000,000, which exceeds ULONG_MAX (4,294,967,295) on 32-bit systems and would wrap around, reporting a roughly 14% utilization instead of 100%. > status->current_frequency / 1000 / 1000); > > return 0; -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
