Hello Dan,
On Mon, 17 Feb 2025 10:35:20 +0300 Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
> Hello Joshua Hahn,
>
> Commit aab5f6eb05fd ("mm/mempolicy: Weighted Interleave Auto-tuning")
> from Feb 7, 2025 (linux-next), leads to the following Smatch static
> checker warning:
>
> mm/mempolicy.c:220 mempolicy_set_node_perf()
> warn: assigned value is less than '1844674407370955161'
>
> mm/mempolicy.c
> 209 int mempolicy_set_node_perf(unsigned int node, struct access_coordinate *coords)
> 210 {
> 211 uint64_t *old_bw, *new_bw;
> 212 uint64_t bw_val;
> 213 u8 *old_iw, *new_iw;
> 214
> 215 /*
> 216 * Bandwidths above this limit cause rounding errors when reducing
> 217 * weights. This value is ~16 exabytes, which is unreasonable anyways.
>
> I see this comment about exabytes
>
> 218 */
> 219 bw_val = min(coords->read_bandwidth, coords->write_bandwidth);
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> but these values are u32s
>
> --> 220 if (bw_val > (U64_MAX / 10))
> ^^^^^^^^^^^^
> There is no way they're going to be more than U64_MAX / 10.
Thank you for bringing this to my attention. Ying Huang also brought this
up in a patch review [1], and this bug report confirms that it is indeed
problematic.
I am not entirely sure why I thought the access coordinates were 64 bits, but
if I am to imagine what my thought process was at that time, I was probably
overthinking the overflow problem and casting all the values into a large
data type. I will change all bandwidth-related fields into unsigned int
and any aggregate field (sum) to u64.
> regards,
> dan carpenter
Thank you again for your report and your work with Smatch! Have a great day : -)
Joshua
[1] https://lore.kernel.org/all/87a5ar3mir.fsf@DESKTOP-5N7EMDA/
Sent using hkml (https://github.com/sjp38/hackermail)
