linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Kajol Jain <kjain@linux.ibm.com>
To: acme@kernel.org, linuxppc-dev@lists.ozlabs.org,
	mpe@ellerman.id.au, sukadev@linux.vnet.ibm.com
Cc: mark.rutland@arm.com, maddy@linux.vnet.ibm.com,
	peterz@infradead.org, yao.jin@linux.intel.com, mingo@kernel.org,
	kan.liang@linux.intel.com, ak@linux.intel.com,
	alexander.shishkin@linux.intel.com, anju@linux.vnet.ibm.com,
	mamatha4@linux.vnet.ibm.com, ravi.bangoria@linux.ibm.com,
	kjain@linux.ibm.com, jmario@redhat.com, namhyung@kernel.org,
	tglx@linutronix.de, mpetlan@redhat.com,
	gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
	linux-perf-users@vger.kernel.org, jolsa@kernel.org
Subject: [PATCH v3 1/8] powerpc/perf/hv-24x7: Fix inconsistent output values incase multiple hv-24x7 events run
Date: Sat, 29 Feb 2020 15:11:52 +0530	[thread overview]
Message-ID: <20200229094159.25573-2-kjain@linux.ibm.com> (raw)
In-Reply-To: <20200229094159.25573-1-kjain@linux.ibm.com>

Commit 2b206ee6b0df ("powerpc/perf/hv-24x7: Display change in counter
values")' added to print _change_ in the counter value rather then raw
value for 24x7 counters. Incase of transactions, the event count
is set to 0 at the beginning of the transaction. It also sets
the event's prev_count to the raw value at the time of initialization.
Because of setting event count to 0, we are seeing some weird behaviour,
whenever we run multiple 24x7 events at a time.

For example:

command#: ./perf stat -e "{hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/,
			   hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/}"
	  		   -C 0 -I 1000 sleep 100

     1.000121704                120 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
     1.000121704                  5 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/
     2.000357733                  8 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
     2.000357733                 10 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/
     3.000495215 18,446,744,073,709,551,616 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
     3.000495215 18,446,744,073,709,551,616 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/
     4.000641884                 56 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
     4.000641884 18,446,744,073,709,551,616 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/
     5.000791887 18,446,744,073,709,551,616 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/

Getting these large values in case we do -I.

As we are setting event_count to 0, for interval case, overall event_count is not
coming in incremental order. As we may can get new delta lesser then previous count.
Because of which when we print intervals, we are getting negative value which create
these large values.

This patch removes part where we set event_count to 0 in function
'h_24x7_event_read'. There won't be much impact as we do set event->hw.prev_count
to the raw value at the time of initialization to print change value.

With this patch
In power9 platform

command#: ./perf stat -e "{hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/,
		           hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/}"
			   -C 0 -I 1000 sleep 100

     1.000117685                 93 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
     1.000117685                  1 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/
     2.000349331                 98 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
     2.000349331                  2 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/
     3.000495900                131 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
     3.000495900                  4 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/
     4.000645920                204 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
     4.000645920                 61 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/
     4.284169997                 22 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/

Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
Suggested-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
 arch/powerpc/perf/hv-24x7.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
index 573e0b309c0c..48e8f4b17b91 100644
--- a/arch/powerpc/perf/hv-24x7.c
+++ b/arch/powerpc/perf/hv-24x7.c
@@ -1400,16 +1400,6 @@ static void h_24x7_event_read(struct perf_event *event)
 			h24x7hw = &get_cpu_var(hv_24x7_hw);
 			h24x7hw->events[i] = event;
 			put_cpu_var(h24x7hw);
-			/*
-			 * Clear the event count so we can compute the _change_
-			 * in the 24x7 raw counter value at the end of the txn.
-			 *
-			 * Note that we could alternatively read the 24x7 value
-			 * now and save its value in event->hw.prev_count. But
-			 * that would require issuing a hcall, which would then
-			 * defeat the purpose of using the txn interface.
-			 */
-			local64_set(&event->count, 0);
 		}
 
 		put_cpu_var(hv_24x7_reqb);
-- 
2.21.0


  reply	other threads:[~2020-02-29  9:46 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-29  9:41 [PATCH v3 0/8] powerpc/perf: Add json file metric support for the hv_24x7 socket/chip level events Kajol Jain
2020-02-29  9:41 ` Kajol Jain [this message]
2020-02-29  9:41 ` [PATCH v3 2/8] powerpc/hv-24x7: Add rtas call in hv-24x7 driver to get processor details Kajol Jain
2020-02-29  9:41 ` [PATCH v3 3/8] powerpc/hv-24x7: Add sysfs files inside hv-24x7 device to show " Kajol Jain
2020-02-29  9:41 ` [PATCH v3 4/8] Documentation/ABI: Add ABI documentation for chips and sockets Kajol Jain
2020-02-29  9:41 ` [PATCH v3 5/8] powerpc/hv-24x7: Update post_mobility_fixup() to handle migration Kajol Jain
2020-02-29  9:41 ` [PATCH v3 6/8] perf/tools: Enhance JSON/metric infrastructure to handle "?" Kajol Jain
2020-03-02 15:08   ` Jiri Olsa
2020-03-06  4:52     ` kajoljain
2020-03-02 15:08   ` Jiri Olsa
2020-03-02 15:09   ` Jiri Olsa
2020-02-29  9:41 ` [PATCH v3 7/8] tools/perf: Enable Hz/hz prinitg for --metric-only option Kajol Jain
2020-02-29  9:41 ` [PATCH v3 8/8] perf/tools/pmu-events/powerpc: Add hv_24x7 socket/chip level metric events Kajol Jain
  -- strict thread matches above, loose matches on Subject: below --
2020-02-29  9:13 [PATCH v3 0/8] powerpc/perf: Add json file metric support for the hv_24x7 socket/chip level events Kajol Jain
2020-02-29  9:13 ` [PATCH v3 1/8] powerpc/perf/hv-24x7: Fix inconsistent output values incase multiple hv-24x7 events run Kajol Jain

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=20200229094159.25573-2-kjain@linux.ibm.com \
    --to=kjain@linux.ibm.com \
    --cc=acme@kernel.org \
    --cc=ak@linux.intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=anju@linux.vnet.ibm.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jmario@redhat.com \
    --cc=jolsa@kernel.org \
    --cc=kan.liang@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=maddy@linux.vnet.ibm.com \
    --cc=mamatha4@linux.vnet.ibm.com \
    --cc=mark.rutland@arm.com \
    --cc=mingo@kernel.org \
    --cc=mpe@ellerman.id.au \
    --cc=mpetlan@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=ravi.bangoria@linux.ibm.com \
    --cc=sukadev@linux.vnet.ibm.com \
    --cc=tglx@linutronix.de \
    --cc=yao.jin@linux.intel.com \
    /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).