[Intel-gfx] [PATCH] drm/i915: Prevent unbounded wm results in g4x_compute_wm()

Dan Carpenter dan.carpenter at oracle.com
Tue Nov 7 13:53:58 UTC 2017


On Tue, Nov 07, 2017 at 12:59:34PM +0000, Chris Wilson wrote:
> Quoting Chris Wilson (2017-11-07 12:56:23)
> > Smatch warns of
> > 
> > drivers/gpu/drm/i915/intel_pm.c:1161 g4x_compute_wm() warn: signedness bug returning '(-33554430)'
> > 
> > which is a result of it believing that wm may be INT_MAX following
> > g4x_tlb_miss_wa(). Just declaring g4x_tlb_miss_wa() as returning an
> > unsigned integer is not sufficient, we need to tell smatch that wm itself
> > is unsigned for it to not worry. So mark up the locals we expect to be
> > non-negative, and so silence smatch.
> > 
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Cc: Dan Carpenter <dan.carpenter at oracle.com>
> 
> Hi Dan, do you mind sharing any insight as to why smatch generates this
> warning and whether the approach in this patch is incorrect?
> 

It comes from this line:

	wm = DIV_ROUND_UP(wm, 64) + 2;

If you take INT_MIN / 64 + 2 then you get -33554430.  The warning is
a false positive, right?  But to me as a reviewer the original min_t()
is suspicious and I would have just done these two lines from the patch:

-       return min_t(int, wm, USHRT_MAX);
+       return min_t(unsigned int, wm, USHRT_MAX);

The rest is pure style preference so it could go either way.

regards,
dan carpenter


More information about the Intel-gfx mailing list