On 02/13/2017 01:49 AM, Nitesh Konkar wrote:
>
>
>
> On Fri, Feb 10, 2017 at 3:22 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 page_faults_maj 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>
>      |  7 +++++++
>     >  docs/news.xml                               |  4 ++--
>     >  docs/schemas/domaincommon.rng               |  1 +
>     >  include/libvirt/libvirt-domain.h            | 10 ++++++++++
>     >  src/libvirt-domain.c                        |  3 +++
>     >  src/qemu/qemu_driver.c                      |  1 +
>     >  src/util/virperf.c                          |  5 ++++-
>     >  src/util/virperf.h                          |  1 +
>     >  tests/genericxml2xmlindata/generic-perf.xml |  1 +
>     >  tools/virsh.pod                             |  5 +++++
>     >  10 files changed, 35 insertions(+), 3 deletions(-)
>     >
>
>     NB: Similar comments from the page_faults_min...
>
>     > diff --git a/docs/formatdomain.html.in <http://formatdomain.html.in>
>     b/docs/formatdomain.html.in <http://formatdomain.html.in>
>     > index 1857168..50a6bdb 100644
>     > --- a/docs/formatdomain.html.in <http://formatdomain.html.in>
>     > +++ b/docs/formatdomain.html.in <http://formatdomain.html.in>

>     > @@ -1943,6 +1943,7 @@
>     >    &lt;event name='context_switches' enabled='no'/&gt;
>     >    &lt;event name='cpu_migrations' enabled='no'/&gt;
>     >    &lt;event name='page_faults_min' enabled='no'/&gt;
>     > +  &lt;event name='page_faults_maj' enabled='no'/&gt;
>     >  &lt;/perf&gt;
>     >  ...
>     >  </pre>
>     > @@ -2052,6 +2053,12 @@
>     >            platform</td>
>     >        <td><code>perf.page_faults_min</code></td>
>     >      </tr>
>     > +    <tr>
>     > +      <td><code>page_faults_maj</code></td>
>     > +      <td>the count of major page faults by applications running on the
>     > +          platform</td>
>     > +      <td><code>perf.page_faults_maj</code></td>
>     > +    </tr>
>
>     As already noted in patch 3... is maj+min the same as what patch 3
>     provides?
>
>
> maj+min is not always exactly the same as page faults. Sometimes there
> is a small offset
> value.
>
> Eg: perf record -a --event={page-faults,major-faults,minor-faults}
> 47
> page-faults
>
> 0
> major-faults
>
> 46 minor-faults
> Offset by 1
>
> Eg:  virsh domstats --perf
> Domain: 'Fedora'
>   perf.page_faults=890
>   perf.page_faults_min=890
>   perf.page_faults_maj=0
> Here maj+min=page_faults
>
>     Thus are all necessary?
>
> I am not sure on this part. Probably yes as we dont want
> the user to add min+maj to get (approx)total page faults.
>
>

I don't mind all 3 being present... still if I'm going to ask the
question, then someone getting the stats will ask the question... they
may also wonder why maj+min != total.

Perhaps something you could dig deeper on with the kernel code
descriptions that are setting the value.

My assumption is it's the "time" of the sample. That is a total page
fault could have been counted even though it hadn't been counted as a
maj or min page fault.

John