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 <[email protected]>
> > Cc: Ville Syrjälä <[email protected]>
> Cc: Dan Carpenter <[email protected]>
>
> 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
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
