* [bug report] selftests/resctrl: Add callback to start a benchmark
@ 2020-05-07 11:50 Dan Carpenter
2020-05-07 21:20 ` Prakhya, Sai Praneeth
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2020-05-07 11:50 UTC (permalink / raw)
To: sai.praneeth.prakhya; +Cc: linux-kselftest
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
^ permalink raw reply [flat|nested] 2+ messages in thread
* RE: [bug report] selftests/resctrl: Add callback to start a benchmark
2020-05-07 11:50 [bug report] selftests/resctrl: Add callback to start a benchmark Dan Carpenter
@ 2020-05-07 21:20 ` Prakhya, Sai Praneeth
0 siblings, 0 replies; 2+ messages in thread
From: Prakhya, Sai Praneeth @ 2020-05-07 21:20 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-kselftest
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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-05-07 21:20 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-07 11:50 [bug report] selftests/resctrl: Add callback to start a benchmark Dan Carpenter
2020-05-07 21:20 ` Prakhya, Sai Praneeth
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.