Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

selftests/resctrl: Fix checking for < 0 for unsigned values

Dan reported following static checker warnings

tools/testing/selftests/resctrl/resctrl_val.c:545 measure_vals()
warn: 'bw_imc' unsigned <= 0

tools/testing/selftests/resctrl/resctrl_val.c:549 measure_vals()
warn: 'bw_resc_end' unsigned <= 0

These warnings are reported because
1. measure_vals() declares 'bw_imc' and 'bw_resc_end' as unsigned long
variables
2. Return value of get_mem_bw_imc() and get_mem_bw_resctrl() are assigned
to 'bw_imc' and 'bw_resc_end' respectively
3. The returned values are checked for <= 0 to see if the calls failed

Checking for < 0 for an unsigned value doesn't make any sense.

Fix this issue by changing the implementation of get_mem_bw_imc() and
get_mem_bw_resctrl() such that they now accept reference to a variable
and set the variable appropriately upon success and return 0, else return
< 0 on error.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Tested-by: Babu Moger <babu.moger@amd.com>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>

authored by

Fenghua Yu and committed by
Shuah Khan
1205b688 d81343b5

+23 -18
+23 -18
tools/testing/selftests/resctrl/resctrl_val.c
··· 300 300 * Memory B/W utilized by a process on a socket can be calculated using 301 301 * iMC counters. Perf events are used to read these counters. 302 302 * 303 - * Return: >= 0 on success. < 0 on failure. 303 + * Return: = 0 on success. < 0 on failure. 304 304 */ 305 - static float get_mem_bw_imc(int cpu_no, char *bw_report) 305 + static int get_mem_bw_imc(int cpu_no, char *bw_report, float *bw_imc) 306 306 { 307 307 float reads, writes, of_mul_read, of_mul_write; 308 308 int imc, j, ret; ··· 373 373 close(imc_counters_config[imc][WRITE].fd); 374 374 } 375 375 376 - if (strcmp(bw_report, "reads") == 0) 377 - return reads; 376 + if (strcmp(bw_report, "reads") == 0) { 377 + *bw_imc = reads; 378 + return 0; 379 + } 378 380 379 - if (strcmp(bw_report, "writes") == 0) 380 - return writes; 381 + if (strcmp(bw_report, "writes") == 0) { 382 + *bw_imc = writes; 383 + return 0; 384 + } 381 385 382 - return (reads + writes); 386 + *bw_imc = reads + writes; 387 + return 0; 383 388 } 384 389 385 390 void set_mbm_path(const char *ctrlgrp, const char *mongrp, int resource_id) ··· 443 438 * 1. If con_mon grp is given, then read from it 444 439 * 2. If con_mon grp is not given, then read from root con_mon grp 445 440 */ 446 - static unsigned long get_mem_bw_resctrl(void) 441 + static int get_mem_bw_resctrl(unsigned long *mbm_total) 447 442 { 448 - unsigned long mbm_total = 0; 449 443 FILE *fp; 450 444 451 445 fp = fopen(mbm_total_path, "r"); ··· 453 449 454 450 return -1; 455 451 } 456 - if (fscanf(fp, "%lu", &mbm_total) <= 0) { 452 + if (fscanf(fp, "%lu", mbm_total) <= 0) { 457 453 perror("Could not get mbm local bytes"); 458 454 fclose(fp); 459 455 ··· 461 457 } 462 458 fclose(fp); 463 459 464 - return mbm_total; 460 + return 0; 465 461 } 466 462 467 463 pid_t bm_pid, ppid; ··· 553 549 static int 554 550 measure_vals(struct resctrl_val_param *param, unsigned long *bw_resc_start) 555 551 { 556 - unsigned long bw_imc, bw_resc, bw_resc_end; 552 + unsigned long bw_resc, bw_resc_end; 553 + float bw_imc; 557 554 int ret; 558 555 559 556 /* ··· 564 559 * Compare the two values to validate resctrl value. 565 560 * It takes 1sec to measure the data. 566 561 */ 567 - bw_imc = get_mem_bw_imc(param->cpu_no, param->bw_report); 568 - if (bw_imc <= 0) 569 - return bw_imc; 562 + ret = get_mem_bw_imc(param->cpu_no, param->bw_report, &bw_imc); 563 + if (ret < 0) 564 + return ret; 570 565 571 - bw_resc_end = get_mem_bw_resctrl(); 572 - if (bw_resc_end <= 0) 573 - return bw_resc_end; 566 + ret = get_mem_bw_resctrl(&bw_resc_end); 567 + if (ret < 0) 568 + return ret; 574 569 575 570 bw_resc = (bw_resc_end - *bw_resc_start) / MB; 576 571 ret = print_results_bw(param->filename, bm_pid, bw_imc, bw_resc);