lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Mon, 05 Aug 2013 13:53:30 +0800
From:	Xiao Guangrong <xiaoguangrong@...ux.vnet.ibm.com>
To:	David Ahern <dsahern@...il.com>
CC:	acme@...stprotocols.net, linux-kernel@...r.kernel.org,
	Ingo Molnar <mingo@...nel.org>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Jiri Olsa <jolsa@...hat.com>,
	Namhyung Kim <namhyung@...nel.org>,
	Runzhen Wang <runzhen@...ux.vnet.ibm.com>
Subject: Re: [PATCH 5/9] perf kvm: add live mode - v3

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

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

> +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->opts.target.pid, "pid",
> +			"record events on existing process id"),
> +		OPT_STRING('t', "tid", &kvm->opts.target.tid, "tid",
> +			"record events on existing thread id"),
> +		OPT_STRING('C', "cpu", &kvm->opts.target.cpu_list, "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->opts.target.system_wide,
> +			"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?

Others look good to me. :)

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists