Re: [PATCH 5/9] perf kvm: add live mode - v3

From: David Ahern
Date: Mon Aug 05 2013 - 10:43:20 EST

On 8/5/13 1:53 AM, Xiao Guangrong wrote:
Hi David,

Thanks for your nice job! I got some questions.

On 08/03/2013 04:05 AM, David Ahern wrote:

static int kvm_events_hash_fn(u64 key)
return key & (EVENTS_CACHE_SIZE - 1);
@@ -472,7 +501,11 @@ static bool handle_end_event(struct perf_kvm_stat *kvm,
vcpu_record->last_event = NULL;
vcpu_record->start_time = 0;

- BUG_ON(timestamp < time_begin);
+ /* seems to happen once in a while during live mode */
+ if (timestamp < time_begin) {
+ pr_debug("End time before begin time; skipping event.\n");
+ return true;
+ }

No idea why it can happen. :(

I saw it triggering quite often early on (last Fall when I started this) and I changed the BUG_ON to keep going and get the command working. It needs to be revisited and figured out why an end event comes before a begin event - might be a start up problem only. But as a start point did not seem to hurt to just ignore the sample.

+static bool verify_vcpu(int vcpu)
+ int nr_cpus;
+ if (vcpu != -1 && vcpu < 0) {
+ pr_err("Invalid vcpu:%d.\n", vcpu);
+ return false;
+ }
+ nr_cpus = sysconf(_SC_NPROCESSORS_ONLN);
+ if ((nr_cpus > 0) && (vcpu > nr_cpus - 1)) {
+ pr_err("Invalid vcpu:%d.\n", vcpu);
+ return false;
+ }

Hmm, kvm can use more vcpus than the cpus on host.

Good point. I'll fix.

+static int kvm_events_live(struct perf_kvm_stat *kvm,
+ int argc, const char **argv)
+ char errbuf[BUFSIZ];
+ int err;
+ const struct option live_options[] = {
+ OPT_STRING('p', "pid", &kvm->, "pid",
+ "record events on existing process id"),
+ OPT_STRING('t', "tid", &kvm->, "tid",
+ "record events on existing thread id"),
+ OPT_STRING('C', "cpu", &kvm->, "cpu",
+ "list of host cpus to monitor"),
+ OPT_UINTEGER('m', "mmap-pages", &kvm->opts.mmap_pages,
+ "number of mmap data pages"),
+ OPT_INCR('v', "verbose", &verbose,
+ "be more verbose (show counter open errors, etc)"),
+ OPT_BOOLEAN('a', "all-cpus", &kvm->,
+ "system-wide collection from all CPUs"),
+ OPT_UINTEGER('d', "display", &kvm->display_time,
+ "time in seconds between display updates"),
+ OPT_STRING(0, "event", &kvm->report_event, "report event",
+ "event for reporting: vmexit, mmio, ioport"),
+ OPT_INTEGER(0, "vcpu", &kvm->trace_vcpu,
+ "vcpu id to report"),
+ OPT_STRING('k', "key", &kvm->sort_key, "sort-key",
+ "key for sorting: sample(sort by samples number)"
+ " time (sort by avg time)"),

Why we have so many parameters used for tracking. For KVM, we only need to know
1) which guest is tracked and 2) which vcpu in the guest is tracked and 3) what
kind of events. no?

I should drop the cpu_list argument and tid is the same as vcpu so I can drop it. 'a' is implicit if pid is not given.

mmap-pages is definitely needed: a LOT of tracepoints get generated.

I'll update.

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at