From: "Prakhya, Sai Praneeth" <sai.praneeth.prakhya@intel.com>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: "linux-kselftest@vger.kernel.org" <linux-kselftest@vger.kernel.org>
Subject: RE: [bug report] selftests/resctrl: Add callback to start a benchmark
Date: Thu, 7 May 2020 21:20:52 +0000 [thread overview]
Message-ID: <FFF73D592F13FD46B8700F0A279B802F5738E47D@ORSMSX114.amr.corp.intel.com> (raw)
In-Reply-To: <20200507115056.GA252660@mwanda>
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
prev parent reply other threads:[~2020-05-07 21:20 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
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 message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=FFF73D592F13FD46B8700F0A279B802F5738E47D@ORSMSX114.amr.corp.intel.com \
--to=sai.praneeth.prakhya@intel.com \
--cc=dan.carpenter@oracle.com \
--cc=linux-kselftest@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).