On 02/07/2012 11:14 PM, Xiao Guangrong wrote: > On 02/08/2012 01:55 AM, David Ahern wrote: > >> On 02/07/2012 06:08 AM, Xiao Guangrong wrote: >>> Add 'perf kvm-events' support to analyze kvm vmexit/mmio/ioport smartly >> >> example output? >> > > > You can find a example output at this website: > http://lwn.net/Articles/479069/ > > [ Sorry, i did not put it into changlog in the v3 ] Right, I meant add an example output to the changelog. > >>> +-------- >>> +[verse] >>> +'perf kvm-events' {record|report} >> >> command name is very generic -- kvm-events; but command focus is rather >> narrow -- I/O events. >> > > > It is not only analysing IO events but also for other events(like vmexit, > and other events will be added). I prefer to kvm-events :) Same. kvm-events should be the name of the command. But from the admittedly not thorough review of the code it seemed to be expecting I/O (and entry/exit tracepoints as a part of that) and not easily amendable to add new tracepoints. ---8<--- >>> + >>> + vcpu_event_record = zalloc(sizeof(*vcpu_event_record) * kvm_max_vcpus); >>> + if (!vcpu_event_record) >>> + die("zalloc.\n"); >>> + >>> + for (i = 0; i < (int)EVENTS_CACHE_SIZE; i++) >>> + INIT_LIST_HEAD(&kvm_events_cache[i]); >>> +} >> >> Really, the caches could be tied to thread structs, and then you don't >> need a max vcpu style allocation. more below. >> > > > Hmm, this cache is used to cache "event struct", this is the common resources > for all vcpus. But consecutive events that are analyzed together happen in the context of a thread -- the vcpu task. ---8<--- >>> +static int kvm_events_report(int vcpu) >>> +{ >>> + init_kvm_event_record(); >>> + init_kvm_tid_to_pid(); >>> + verify_vcpu(vcpu); >>> + select_key(); >>> + register_kvm_events_ops(); >>> + setup_pager(); >> >> I believe setup_pager is handled by perf.c >> > > > Hmm, i did not find it, could you please tell me where is it? > And, setup_pager is also used in other tools such 'perf sched', > 'perf lock'... > run_builtin() --> commit_pager_choice() --> setup_pager() It could be that the other commands need to be updated. ---8<--- >>> + if (!strncmp(argv[0], "rec", 3)) >>> + return kvm_events_record(argc, argv); >>> + >>> + if (!strncmp(argv[0], "report", 6)) { >> >> exact match for 'report'? >> > > > It is the style in other tools, what is your suggestion? Right, I see that. builtin-lock should be fixed. rec is ok for record; is report123 ok for report? if not that string should be an exact match. David -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html