Hi Dan, Thanks for reporting the issues. They look legitimate to me. I will send a fix. > -----Original Message----- > From: Dan Carpenter <dan.carpenter@xxxxxxxxxx> > Sent: Thursday, May 7, 2020 4:51 AM > To: Prakhya, Sai Praneeth <sai.praneeth.prakhya@xxxxxxxxx> > Cc: linux-kselftest@xxxxxxxxxxxxxxx > 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