<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Feb 13, 2017 at 8:14 PM, John Ferlan <span dir="ltr"><<a href="mailto:jferlan@redhat.com" target="_blank">jferlan@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="gmail-m_-211566420636597459gmail-"><br>
<br>
On 02/13/2017 01:49 AM, Nitesh Konkar wrote:<br>
><br>
><br>
><br>
> On Fri, Feb 10, 2017 at 3:22 AM, John Ferlan <<a href="mailto:jferlan@redhat.com" target="_blank">jferlan@redhat.com</a><br>
</span><span class="gmail-m_-211566420636597459gmail-">> <mailto:<a href="mailto:jferlan@redhat.com" target="_blank">jferlan@redhat.com</a>>> wrote:<br>
><br>
><br>
><br>
> On 01/27/2017 06:01 AM, Nitesh Konkar wrote:<br>
> > This patch adds support and documentation<br>
> > for the page_faults_maj perf event.<br>
> ><br>
</span>> > Signed-off-by: Nitesh Konkar <<a href="mailto:nitkon12@linux.vnet.ibm.com" target="_blank">nitkon12@linux.vnet.ibm.com</a> <mailto:<a href="mailto:nitkon12@linux.vnet.ibm.com" target="_blank">nitkon12@linux.vnet.ib<wbr>m.com</a>>><br>
> > ---<br>
> > docs/<a href="http://formatdomain.html.in" rel="noreferrer" target="_blank">formatdomain.html.in</a> <<a href="http://formatdomain.html.in" rel="noreferrer" target="_blank">http://formatdomain.html.in</a>><br>
<span class="gmail-m_-211566420636597459gmail-">> | 7 +++++++<br>
> > docs/news.xml | 4 ++--<br>
> > docs/schemas/domaincommon.rng | 1 +<br>
> > include/libvirt/libvirt-domain<wbr>.h | 10 ++++++++++<br>
> > src/libvirt-domain.c | 3 +++<br>
> > src/qemu/qemu_driver.c | 1 +<br>
> > src/util/virperf.c | 5 ++++-<br>
> > src/util/virperf.h | 1 +<br>
> > tests/genericxml2xmlindata/gen<wbr>eric-perf.xml | 1 +<br>
> > tools/virsh.pod | 5 +++++<br>
> > 10 files changed, 35 insertions(+), 3 deletions(-)<br>
> ><br>
><br>
> NB: Similar comments from the page_faults_min...<br>
><br>
</span>> > diff --git a/docs/<a href="http://formatdomain.html.in" rel="noreferrer" target="_blank">formatdomain.html.in</a> <<a href="http://formatdomain.html.in" rel="noreferrer" target="_blank">http://formatdomain.html.in</a>><br>
> b/docs/<a href="http://formatdomain.html.in" rel="noreferrer" target="_blank">formatdomain.html.in</a> <<a href="http://formatdomain.html.in" rel="noreferrer" target="_blank">http://formatdomain.html.in</a>><br>
> > index 1857168..50a6bdb 100644<br>
> > --- a/docs/<a href="http://formatdomain.html.in" rel="noreferrer" target="_blank">formatdomain.html.in</a> <<a href="http://formatdomain.html.in" rel="noreferrer" target="_blank">http://formatdomain.html.in</a>><br>
> > +++ b/docs/<a href="http://formatdomain.html.in" rel="noreferrer" target="_blank">formatdomain.html.in</a> <<a href="http://formatdomain.html.in" rel="noreferrer" target="_blank">http://formatdomain.html.in</a>><br>
<div><div class="gmail-m_-211566420636597459gmail-h5">> > @@ -1943,6 +1943,7 @@<br>
> > <event name='context_switches' enabled='no'/><br>
> > <event name='cpu_migrations' enabled='no'/><br>
> > <event name='page_faults_min' enabled='no'/><br>
> > + <event name='page_faults_maj' enabled='no'/><br>
> > </perf><br>
> > ...<br>
> > </pre><br>
> > @@ -2052,6 +2053,12 @@<br>
> > platform</td><br>
> > <td><code>perf.page_faults_min<wbr></code></td><br>
> > </tr><br>
> > + <tr><br>
> > + <td><code>page_faults_maj</cod<wbr>e></td><br>
> > + <td>the count of major page faults by applications running on the<br>
> > + platform</td><br>
> > + <td><code>perf.page_faults_maj<wbr></code></td><br>
> > + </tr><br>
><br>
> As already noted in patch 3... is maj+min the same as what patch 3<br>
> provides?<br>
><br>
><br>
> maj+min is not always exactly the same as page faults. Sometimes there<br>
> is a small offset<br>
> value.<br>
><br>
> Eg: perf record -a --event={page-faults,major-fau<wbr>lts,minor-faults}<br>
> 47<br>
> page-faults<br>
><br>
> 0<br>
> major-faults<br>
><br>
> 46 minor-faults<br>
> Offset by 1<br>
><br>
> Eg: virsh domstats --perf<br>
> Domain: 'Fedora'<br>
> perf.page_faults=890<br>
> perf.page_faults_min=890<br>
> perf.page_faults_maj=0<br>
> Here maj+min=page_faults<br>
><br>
> Thus are all necessary?<br>
><br>
> I am not sure on this part. Probably yes as we dont want<br>
> the user to add min+maj to get (approx)total page faults.<br>
><br>
><br>
<br>
</div></div>I don't mind all 3 being present... still if I'm going to ask the<br>
question, then someone getting the stats will ask the question... they<br>
may also wonder why maj+min != total.<br>
<br>
Perhaps something you could dig deeper on with the kernel code<br>
descriptions that are setting the value.<br>
<br>
My assumption is it's the "time" of the sample. That is a total page<br>
fault could have been counted even though it hadn't been counted as a<br>
maj or min page fault.<br></blockquote><div><br>I looked into the kernel code in /arch/x86/mm/fault.c and also confirmed from<br></div><div>the #perf IRC that maj+min != total is valid. This is because not all<br></div><div>page faults fall in maj/min category. Some maybe invalid page faults(invalid address generated)<br>whereas some pagefaults after occuring are not serviced due to lock contention <br>so as to avoid a deadlock at that instance, thus being counted in total but<br>not in min/maj faults. <br><br></div><div>Also, shd i follow the comment pattern as shown <br></div><div>in ur patch under review, in /virsh.pod ? <br><br></div><div>Thnx.<br></div><div><br><span class="gmail-m_-211566420636597459gmail-HOEnZb"></span><br><span class="gmail-m_-211566420636597459gmail-HOEnZb"></span></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="gmail-m_-211566420636597459gmail-HOEnZb"><font color="#888888">
John<br>
</font></span></blockquote></div><br></div></div>