On Mon, Jul 31, 2023 at 9:23 AM Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
>
> Hello Domenico Cerasuolo,
>
> The patch 7c967f267b1d: "selftests: cgroup: add test_zswap with no
> kmem bypass test" from Jun 21, 2023 (linux-next), leads to the
> following Smatch static checker warning:
>
> tools/testing/selftests/cgroup/test_zswap.c:211 test_no_kmem_bypass()
> warn: unsigned 'stored_pages' is never less than zero.
>
> ./tools/testing/selftests/cgroup/test_zswap.c
> 197 /* Try to wakeup kswapd and let it push child memory to zswap */
> 198 set_min_free_kb(min_free_kb_high);
> 199 for (int i = 0; i < 20; i++) {
> 200 size_t stored_pages;
> 201 char *trigger_allocation = malloc(trigger_allocation_size);
> 202
> 203 if (!trigger_allocation)
> 204 break;
> 205 for (int i = 0; i < trigger_allocation_size; i += 4095)
> 206 trigger_allocation[i] = 'b';
> 207 usleep(100000);
> 208 free(trigger_allocation);
> 209 if (get_zswap_stored_pages(&stored_pages))
> 210 break;
> --> 211 if (stored_pages < 0)
>
> size_t can't be negative. Is there any reason to check this even?
Hi Dan,
Thanks for reporting this, that check is indeed not needed. I should have
removed it when I refactored get_zswap_stored_pages and added the check
at line 209, I'll send a fix.
Thanks,
Domenico
>
> 212 break;
> 213 /* If memory was pushed to zswap, verify it belongs to memcg */
> 214 if (stored_pages > stored_pages_threshold) {
> 215 int zswapped = cg_read_key_long(test_group, "memory.stat", "zswapped ");
> 216 int delta = stored_pages * 4096 - zswapped;
> 217 int result_ok = delta < stored_pages * 4096 / 4;
> 218
> 219 ret = result_ok ? KSFT_PASS : KSFT_FAIL;
> 220 break;
> 221 }
> 222 }
> 223
> 224 kill(child_pid, SIGTERM);
>
> regards,
> dan carpenter
