lkml.org 
[lkml]   [2012]   [Feb]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v3 3/3] KVM: perf: kvm events analysis tool


    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


    \
     
     \ /
      Last update: 2012-02-08 15:29    [W:0.029 / U:88.440 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site