[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/9] perf: add cpu_clock software perf event support




On 02/13/2017 03:01 AM, Nitesh Konkar wrote:
> 
> 
> On Fri, Feb 10, 2017 at 3:10 AM, John Ferlan <jferlan redhat com
> <mailto:jferlan redhat com>> wrote:
> 
> 
> 
>     On 01/27/2017 06:01 AM, Nitesh Konkar wrote:
>     > This patch adds support and documentation for
>     > the cpu_clock perf event.
>     >
>     > Signed-off-by: Nitesh Konkar <nitkon12 linux vnet ibm com
>     <mailto:nitkon12 linux vnet ibm com>>
>     > ---
>     >  docs/formatdomain.html.in <http://formatdomain.html.in>         
>              |  6 ++++++
>     >  docs/news.xml                               |  4 ++--
>     >  docs/schemas/domaincommon.rng               |  1 +
>     >  include/libvirt/libvirt-domain.h            | 10 ++++++++++
>     >  src/libvirt-domain.c                        |  2 ++
>     >  src/qemu/qemu_driver.c                      |  1 +
>     >  src/util/virperf.c                          |  6 +++++-
>     >  src/util/virperf.h                          |  1 +
>     >  tests/genericxml2xmlindata/generic-perf.xml |  1 +
>     >  tools/virsh.pod                             |  3 +++
>     >  10 files changed, 32 insertions(+), 3 deletions(-)
>     >
>     > diff --git a/docs/formatdomain.html.in
>     <http://formatdomain.html.in> b/docs/formatdomain.html.in
>     <http://formatdomain.html.in>
>     > index 3f7f875..e44e758 100644
>     > --- a/docs/formatdomain.html.in <http://formatdomain.html.in>
>     > +++ b/docs/formatdomain.html.in <http://formatdomain.html.in>
>     > @@ -1937,6 +1937,7 @@
>     >    &lt;event name='stalled_cycles_frontend' enabled='no'/&gt;
>     >    &lt;event name='stalled_cycles_backend' enabled='no'/&gt;
>     >    &lt;event name='ref_cpu_cycles' enabled='no'/&gt;
>     > +  &lt;event name='cpu_clock' enabled='no'/&gt;
>     >  &lt;/perf&gt;
>     >  ...
>     >  </pre>
>     > @@ -2015,6 +2016,11 @@
>     >           by applications running on the platform</td>
>     >        <td><code>perf.ref_cpu_cycles</code></td>
>     >      </tr>
>     > +    <tr>
>     > +      <td><code>cpu_clock</code></td>
>     > +      <td>the count of cpu clock by applications running on the
>     platform</td>
>     > +      <td><code>perf.cpu_clock</code></td>
>     > +    </tr>
>     >    </table>
> 
>     "the count of cpu clock by applications..."
> 
>     doesn't convey enough meaning. Is that cycles? Time?  something else?
> 
>     The virsh.pod description seems to give the best hint...
> 
>     Some subsequent patches had longer descriptions in the virsh.pod, which
>     got me thinking - where is the best place to have the longer
>     description. I would think either in formatdomain or the
>     libvirt-domain.{c|} description.  At least that way you don't need to
>     mess with the 80 column formatting that is "nice" to have in virsh.pod
>     output
> 
> Lets keep the virsh.pod and libvirt-domain.c explanantion terse and let
> formatdomain have detailed explanation of the events. Do you agree?
> 
> 

I'm fine with that as long as the terse description can provide some
amount of context.

John


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]