On Mon, Feb 13, 2017 at 8:14 PM, John Ferlan <jferlan@xxxxxxxxxx> wrote:
I looked into the kernel code in /arch/x86/mm/fault.c and also confirmed from
On 02/13/2017 01:49 AM, Nitesh Konkar wrote:
>
>
>
> On Fri, Feb 10, 2017 at 3:22 AM, John Ferlan <jferlan@xxxxxxxxxx
> <mailto:jferlan@xxxxxxxxxx>> 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@xxxxxxxxxxxxxxxxxx <mailto:nitkon12@xxxxxxxxxxxxxm.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 ++++++++++ > > diff --git a/docs/formatdomain.html.in <http://formatdomain.html.in>
> > 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...
>
> 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>
I don't mind all 3 being present... still if I'm going to ask the> > @@ -1943,6 +1943,7 @@
> > <event name='context_switches' enabled='no'/>
> > <event name='cpu_migrations' enabled='no'/>
> > <event name='page_faults_min' enabled='no'/>
> > + <event name='page_faults_maj' enabled='no'/>
> > </perf>
> > ...
> > </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.
>
>
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.
I looked into the kernel code in /arch/x86/mm/fault.c and also confirmed from
the #perf IRC that maj+min != total is valid. This is because not all
page faults fall in maj/min category. Some maybe invalid page faults(invalid address generated)
whereas some pagefaults after occuring are not serviced due to lock contention
so as to avoid a deadlock at that instance, thus being counted in total but
not in min/maj faults.
whereas some pagefaults after occuring are not serviced due to lock contention
so as to avoid a deadlock at that instance, thus being counted in total but
not in min/maj faults.
Also, shd i follow the comment pattern as shown
in ur patch under review, in /virsh.pod ?
Thnx.
John
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
