[PATCH] Back out recent change to exclude hypervisor samples and counts

Back out recent change to exclude hypervisor samples and counts

Recent commits, 3f93a3b3 and 9c662bfa, made changes to exclude the
collection of hypervisor samples (for operf) and counts (for ocount).
I learned later that S390 and Alpha architectures do not support mode
exclusion, and so fixes were required to avoid excluding hypervisor
on those architectures. Now it seems that under certain conditions,
the ARM architecture also cannot do mode exclusion, and this exclusion
of hypervisor was causing operf and ocount to fail.

The original changes (in commits 3f93a3b3 and 9c662bfa) were not made
due to a bug report, but because I had noted oprofile had always been
silently collecting hypervisor samples/counts, without ever having a
means of identifying them as such. Additionally, the event specification
(for passing events to operf and ocount) has no support for explicit
inclusion/exclusion of hypervisor data. I thought it would be better to
simply always exclude hypervisor until such time that we expanded the
event specification and operf/ocount interfaces to properly support
hypervisor. But in retrospect, that was a bad decision, causing too
much breakage from various architectures. This patch backs out the
exclusion of hypervisor, as well as the S390 and Alpha architecutre-
specific conditional compilation involving same.

Signed-off-by: Maynard Johnson <[email protected]>
---
 libpe_utils/op_pe_utils.cpp      |   29 ++---------------------------
 libperf_events/operf_counter.cpp |    4 +---
 pe_counting/ocount_counter.cpp   |    3 +--
 3 files changed, 4 insertions(+), 32 deletions(-)

diff --git a/libpe_utils/op_pe_utils.cpp b/libpe_utils/op_pe_utils.cpp
index 7c9691b..8c69894 100644
--- a/libpe_utils/op_pe_utils.cpp
+++ b/libpe_utils/op_pe_utils.cpp
@@ -880,24 +880,6 @@ void op_pe_utils::op_process_events_list(set<string> & passed_evts,
 		event.evt_um = 0UL;
 		event.no_kernel = 0;
 		event.no_user = 0;
-		/* Explicitly exclude hypervisor samples since we currently do not have any
-		 * interface support for such.  If we did not do this, we could see situations
-		 * on hypervisor-controlled systems like the following:
-		 * 	$ ocount -e PM_RUN_CYC:0:0:0 /bin/true
-		 *
-		 * 	Events were actively counted for 398213 nanoseconds.
-		 * 	Event counts (actual) for /bin/true:
-		 *	        Event                         Count                    % time counted
-		 *	        PM_RUN_CYC_GRP1:0x0:0:0       123,260                  100.00
-		 *
-		 * Note that the event spec explicitly excludes both kernel and user events, yet
-		 * the output shows a non-zero count. The user could "assume" those counts are from
-		 * hypervisor, but that's ugly.
-		 *
-		 * FIXME: Add full hypervisor support by adding another bit in the event specification
-		 * and documenting it in the man pages and user guide.
-		 */
-		event.no_hv = 1;
 		event.throttled = false;
 		event.mode_specified = false;
 		event.umask_specified = false;
@@ -960,10 +942,8 @@ void op_pe_utils::op_process_events_list(set<string> & passed_evts,
 #endif
 
 #ifdef __alpha__
-		// Alpha arch does not support any mode exclusion.  We'll just silently enable
-		// hypervisor, but if either user or kernel mode are excluded by the user, we'll
-		// exit with an error message.
-		event.no_hv = 0;
+		// Alpha arch does not support any mode exclusion, so if either user or kernel
+		// mode are excluded by the user, we'll exit with an error message.
 		if (event.no_kernel || event.no_user) {
 			cerr << "Mode exclusion is not supported on Alpha." << endl
 			     << "Re-run the command and simply pass the event name " << endl
@@ -1022,11 +1002,6 @@ void op_pe_utils::op_get_default_event(bool do_callgraph)
 		dft_evt.count = descr.count;
 	}
 	dft_evt.evt_um = descr.um;
-#ifndef __alpha__
-	// See comment in op_process_events_list for why we set no_hv to 1.
-	// Alpha arch does not support any mode exclusion.
-	dft_evt.no_hv = 1;
-#endif
 	strncpy(dft_evt.name, descr.name, OP_MAX_EVT_NAME_LEN - 1);
 	_get_event_code(&dft_evt, cpu_type);
 	events.push_back(dft_evt);
diff --git a/libperf_events/operf_counter.cpp b/libperf_events/operf_counter.cpp
index 32d10a6..42c0cd1 100644
--- a/libperf_events/operf_counter.cpp
+++ b/libperf_events/operf_counter.cpp
@@ -220,10 +220,8 @@ operf_counter::operf_counter(operf_event_t & evt,  bool enable_on_exec, bool do_
 
 #ifdef __s390__
 	attr.type = PERF_TYPE_HARDWARE;
-	attr.exclude_hv = 0;
 #else
 	attr.type = PERF_TYPE_RAW;
-	attr.exclude_hv = evt.no_hv;
 #endif
 #if ((defined(__i386__) || defined(__x86_64__)) && (HAVE_PERF_PRECISE_IP))
 	if (evt.evt_code & EXTRA_PEBS) {
@@ -231,7 +229,7 @@ operf_counter::operf_counter(operf_event_t & evt,  bool enable_on_exec, bool do_
 		evt.evt_code ^= EXTRA_PEBS;
 	}
 #endif
-	
+	attr.exclude_hv = evt.no_hv;
 	attr.config = evt.evt_code;
 	attr.sample_period = evt.count;
 	attr.inherit = inherit ? 1 : 0;
diff --git a/pe_counting/ocount_counter.cpp b/pe_counting/ocount_counter.cpp
index 2dd6210..1573ed4 100644
--- a/pe_counting/ocount_counter.cpp
+++ b/pe_counting/ocount_counter.cpp
@@ -71,13 +71,12 @@ ocount_counter::ocount_counter(operf_event_t & evt,  bool enable_on_exec,
 	attr.config = evt.evt_code;
 #ifdef __s390__
 	attr.type = PERF_TYPE_HARDWARE;
-	attr.exclude_hv = 0;
 	if (evt.no_kernel && !evt.no_user)
 		attr.config |= 32;
 #else
 	attr.type = PERF_TYPE_RAW;
-	attr.exclude_hv = evt.no_hv;
 #endif
+	attr.exclude_hv = evt.no_hv;
 	attr.inherit = inherit ? 1 : 0;
 	attr.enable_on_exec = enable_on_exec ? 1 : 0;
 	attr.disabled  = attr.enable_on_exec;
-- 
1.7.1


------------------------------------------------------------------------------
Slashdot TV.  
Video for Nerds.  Stuff that matters.
http://tv.slashdot.org/