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 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.