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

On Fri, Feb 10, 2017 at 3:10 AM, John Ferlan <jferlan(a)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(a)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?