Hello Sai Praneeth Prakhya,
The patch 7f4d257e3a2a: "selftests/resctrl: Add callback to start a benchmark" from Jan 16, 2020, leads to the following static checker warning:
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
tools/testing/selftests/resctrl/resctrl_val.c 531 static int 532 measure_vals(struct resctrl_val_param *param, unsigned long *bw_resc_start) 533 { 534 unsigned long bw_imc, bw_resc, bw_resc_end; 535 int ret; 536 537 /* 538 * Measure memory bandwidth from resctrl and from 539 * another source which is perf imc value or could 540 * be something else if perf imc event is not available. 541 * Compare the two values to validate resctrl value. 542 * It takes 1sec to measure the data. 543 */ 544 bw_imc = get_mem_bw_imc(param->cpu_no, param->bw_report); 545 if (bw_imc <= 0) ^^^^^^^^^^^ Unsigned. Also the comments for get_mem_bw_imc() says that zero is success.
546 return bw_imc; 547 548 bw_resc_end = get_mem_bw_resctrl(); 549 if (bw_resc_end <= 0) ^^^^^^^^^^^^^^^^ Unsigned
550 return bw_resc_end; 551 552 bw_resc = (bw_resc_end - *bw_resc_start) / MB; 553 ret = print_results_bw(param->filename, bm_pid, bw_imc, bw_resc); 554 if (ret) 555 return ret; 556 557 *bw_resc_start = bw_resc_end; 558 559 return 0; 560 }
regards, dan carpenter
Hi Dan,
Thanks for reporting the issues. They look legitimate to me. I will send a fix.
-----Original Message----- From: Dan Carpenter dan.carpenter@oracle.com Sent: Thursday, May 7, 2020 4:51 AM To: Prakhya, Sai Praneeth sai.praneeth.prakhya@intel.com Cc: linux-kselftest@vger.kernel.org Subject: [bug report] selftests/resctrl: Add callback to start a benchmark
Hello Sai Praneeth Prakhya,
The patch 7f4d257e3a2a: "selftests/resctrl: Add callback to start a benchmark" from Jan 16, 2020, leads to the following static checker warning:
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
I agree. There is no point checking for < 0 for unsigned values.
tools/testing/selftests/resctrl/resctrl_val.c 531 static int 532 measure_vals(struct resctrl_val_param *param, unsigned long *bw_resc_start) 533 { 534 unsigned long bw_imc, bw_resc, bw_resc_end; 535 int ret; 536 537 /* 538 * Measure memory bandwidth from resctrl and from 539 * another source which is perf imc value or could 540 * be something else if perf imc event is not available. 541 * Compare the two values to validate resctrl value. 542 * It takes 1sec to measure the data. 543 */ 544 bw_imc = get_mem_bw_imc(param->cpu_no, param->bw_report); 545 if (bw_imc <= 0) ^^^^^^^^^^^ Unsigned. Also the comments for get_mem_bw_imc() says that zero is success.
The comment is wrong. It should have been < 0 failure and > 0 as success.
546 return bw_imc; 547 548 bw_resc_end = get_mem_bw_resctrl(); 549 if (bw_resc_end <= 0) ^^^^^^^^^^^^^^^^ Unsigned
550 return bw_resc_end; 551 552 bw_resc = (bw_resc_end - *bw_resc_start) / MB; 553 ret = print_results_bw(param->filename, bm_pid, bw_imc, bw_resc); 554 if (ret) 555 return ret; 556 557 *bw_resc_start = bw_resc_end; 558 559 return 0; 560 }
regards, dan carpenter
linux-kselftest-mirror@lists.linaro.org