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@xxxxxxxxxxxxxxxxxx> > > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > Cc: Dan Carpenter <dan.carpenter@xxxxxxxxxx> > > 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 Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
