1. Patchew
  2. Libvirt
  3. [libvirt] [PATCH v2 0/2] Simplify perf events handling a little
  4. View patch

[libvirt] [PATCH v2 1/2] perf: get rid of pointless virPerfGetEventAttr() method

Daniel P. Berrange posted 2 patches 8 years, 10 months ago
Only 1 patches received!
[libvirt] [PATCH v2 1/2] perf: get rid of pointless virPerfGetEventAttr() method
Posted by Daniel P. Berrange 8 years, 10 months ago 
The virPerfGetEventAttr method contains a totally pointless
loop. Remove it, verify the array size statically, and then
just use an array index to access the perf event.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 src/util/virperf.c | 143 +++++++++++++++++++++++++----------------------------
 1 file changed, 66 insertions(+), 77 deletions(-)

diff --git a/src/util/virperf.c b/src/util/virperf.c
index e39cebb..7366efc 100644
--- a/src/util/virperf.c
+++ b/src/util/virperf.c
@@ -71,102 +71,91 @@ struct virPerf {
 # include <linux/perf_event.h>
 
 struct virPerfEventAttr {
-    int type;
     unsigned int attrType;
     unsigned long long attrConfig;
 };
 
 static struct virPerfEventAttr attrs[] = {
-    {.type = VIR_PERF_EVENT_CMT, .attrType = 0, .attrConfig = 1},
-    {.type = VIR_PERF_EVENT_MBMT, .attrType = 0, .attrConfig = 2},
-    {.type = VIR_PERF_EVENT_MBML, .attrType = 0, .attrConfig = 3},
-    {.type = VIR_PERF_EVENT_CPU_CYCLES,
-     .attrType = PERF_TYPE_HARDWARE,
-     .attrConfig = PERF_COUNT_HW_CPU_CYCLES},
-    {.type = VIR_PERF_EVENT_INSTRUCTIONS,
-     .attrType = PERF_TYPE_HARDWARE,
-     .attrConfig = PERF_COUNT_HW_INSTRUCTIONS},
-    {.type = VIR_PERF_EVENT_CACHE_REFERENCES,
-     .attrType = PERF_TYPE_HARDWARE,
-     .attrConfig = PERF_COUNT_HW_CACHE_REFERENCES},
-    {.type = VIR_PERF_EVENT_CACHE_MISSES,
-     .attrType = PERF_TYPE_HARDWARE,
-     .attrConfig = PERF_COUNT_HW_CACHE_MISSES},
-    {.type = VIR_PERF_EVENT_BRANCH_INSTRUCTIONS,
-     .attrType = PERF_TYPE_HARDWARE,
-     .attrConfig = PERF_COUNT_HW_BRANCH_INSTRUCTIONS},
-    {.type = VIR_PERF_EVENT_BRANCH_MISSES,
-     .attrType = PERF_TYPE_HARDWARE,
-     .attrConfig = PERF_COUNT_HW_BRANCH_MISSES},
-    {.type = VIR_PERF_EVENT_BUS_CYCLES,
-     .attrType = PERF_TYPE_HARDWARE,
-     .attrConfig = PERF_COUNT_HW_BUS_CYCLES},
-    {.type = VIR_PERF_EVENT_STALLED_CYCLES_FRONTEND,
-     .attrType = PERF_TYPE_HARDWARE,
-     .attrConfig = PERF_COUNT_HW_STALLED_CYCLES_FRONTEND},
-    {.type = VIR_PERF_EVENT_STALLED_CYCLES_BACKEND,
-     .attrType = PERF_TYPE_HARDWARE,
-     .attrConfig = PERF_COUNT_HW_STALLED_CYCLES_BACKEND},
-    {.type = VIR_PERF_EVENT_REF_CPU_CYCLES,
+    [VIR_PERF_EVENT_CMT] = {
+        .attrType = 0,
+        .attrConfig = 1
+    },
+    [VIR_PERF_EVENT_MBMT] = {
+        .attrType = 0,
+        .attrConfig = 2
+    },
+    [VIR_PERF_EVENT_MBML] {
+        .attrType = 0,
+        .attrConfig = 3
+    },
+    [VIR_PERF_EVENT_CPU_CYCLES] = {
+        .attrType = PERF_TYPE_HARDWARE,
+        .attrConfig = PERF_COUNT_HW_CPU_CYCLES
+    },
+    [VIR_PERF_EVENT_INSTRUCTIONS] = {
+        .attrType = PERF_TYPE_HARDWARE,
+        .attrConfig = PERF_COUNT_HW_INSTRUCTIONS
+    },
+    [VIR_PERF_EVENT_CACHE_REFERENCES] = {
+        .attrType = PERF_TYPE_HARDWARE,
+        .attrConfig = PERF_COUNT_HW_CACHE_REFERENCES
+    },
+    [VIR_PERF_EVENT_CACHE_MISSES] = {
+        .attrType = PERF_TYPE_HARDWARE,
+        .attrConfig = PERF_COUNT_HW_CACHE_MISSES
+    },
+    [VIR_PERF_EVENT_BRANCH_INSTRUCTIONS] = {
+        .attrType = PERF_TYPE_HARDWARE,
+        .attrConfig = PERF_COUNT_HW_BRANCH_INSTRUCTIONS
+    },
+    [VIR_PERF_EVENT_BRANCH_MISSES] = {
+        .attrType = PERF_TYPE_HARDWARE,
+        .attrConfig = PERF_COUNT_HW_BRANCH_MISSES
+    },
+    [VIR_PERF_EVENT_BUS_CYCLES] = {
+        .attrType = PERF_TYPE_HARDWARE,
+        .attrConfig = PERF_COUNT_HW_BUS_CYCLES
+    },
+    [VIR_PERF_EVENT_STALLED_CYCLES_FRONTEND] = {
+        .attrType = PERF_TYPE_HARDWARE,
+        .attrConfig = PERF_COUNT_HW_STALLED_CYCLES_FRONTEND
+    },
+    [VIR_PERF_EVENT_STALLED_CYCLES_BACKEND] = {
+        .attrType = PERF_TYPE_HARDWARE,
+        .attrConfig = PERF_COUNT_HW_STALLED_CYCLES_BACKEND
+    },
+    [VIR_PERF_EVENT_REF_CPU_CYCLES] = {
 # ifdef PERF_COUNT_HW_REF_CPU_CYCLES
-     .attrType = PERF_TYPE_HARDWARE,
-     .attrConfig = PERF_COUNT_HW_REF_CPU_CYCLES
+        .attrType = PERF_TYPE_HARDWARE,
+        .attrConfig = PERF_COUNT_HW_REF_CPU_CYCLES
 # else
-     .attrType = 0,
-     .attrConfig = 0,
+        .attrType = 0,
+        .attrConfig = 0,
 # endif
     },
-    {.type = VIR_PERF_EVENT_CPU_CLOCK,
-     .attrType = PERF_TYPE_SOFTWARE,
+    {.attrType = PERF_TYPE_SOFTWARE,
      .attrConfig = PERF_COUNT_SW_CPU_CLOCK},
-    {.type = VIR_PERF_EVENT_TASK_CLOCK,
-     .attrType = PERF_TYPE_SOFTWARE,
+    {.attrType = PERF_TYPE_SOFTWARE,
      .attrConfig = PERF_COUNT_SW_TASK_CLOCK},
-    {.type = VIR_PERF_EVENT_PAGE_FAULTS,
-     .attrType = PERF_TYPE_SOFTWARE,
+    {.attrType = PERF_TYPE_SOFTWARE,
      .attrConfig = PERF_COUNT_SW_PAGE_FAULTS},
-    {.type = VIR_PERF_EVENT_CONTEXT_SWITCHES,
-     .attrType = PERF_TYPE_SOFTWARE,
+    {.attrType = PERF_TYPE_SOFTWARE,
      .attrConfig = PERF_COUNT_SW_CONTEXT_SWITCHES},
-    {.type = VIR_PERF_EVENT_CPU_MIGRATIONS,
-     .attrType = PERF_TYPE_SOFTWARE,
+    {.attrType = PERF_TYPE_SOFTWARE,
      .attrConfig = PERF_COUNT_SW_CPU_MIGRATIONS},
-    {.type = VIR_PERF_EVENT_PAGE_FAULTS_MIN,
-     .attrType = PERF_TYPE_SOFTWARE,
+    {.attrType = PERF_TYPE_SOFTWARE,
      .attrConfig = PERF_COUNT_SW_PAGE_FAULTS_MIN},
-    {.type = VIR_PERF_EVENT_PAGE_FAULTS_MAJ,
-     .attrType = PERF_TYPE_SOFTWARE,
+    {.attrType = PERF_TYPE_SOFTWARE,
      .attrConfig = PERF_COUNT_SW_PAGE_FAULTS_MAJ},
-    {.type = VIR_PERF_EVENT_ALIGNMENT_FAULTS,
-     .attrType = PERF_TYPE_SOFTWARE,
+    {.attrType = PERF_TYPE_SOFTWARE,
      .attrConfig = PERF_COUNT_SW_ALIGNMENT_FAULTS},
-    {.type = VIR_PERF_EVENT_EMULATION_FAULTS,
-     .attrType = PERF_TYPE_SOFTWARE,
+    {.attrType = PERF_TYPE_SOFTWARE,
      .attrConfig = PERF_COUNT_SW_EMULATION_FAULTS},
 };
+verify(ARRAY_CARDINALITY(attrs) == VIR_PERF_EVENT_LAST);
 typedef struct virPerfEventAttr *virPerfEventAttrPtr;
 
 
-static virPerfEventAttrPtr
-virPerfGetEventAttr(virPerfEventType type)
-{
-    size_t i;
-    if (type >= VIR_PERF_EVENT_LAST) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("Event '%d' is not supported"),
-                       type);
-        return NULL;
-    }
-
-    for (i = 0; i < VIR_PERF_EVENT_LAST; i++) {
-        if (i == type)
-            return attrs + i;
-    }
-
-    return NULL;
-}
-
-
 static int
 virPerfRdtAttrInit(void)
 {
@@ -224,9 +213,9 @@ virPerfEventEnable(virPerfPtr perf,
     char *buf = NULL;
     struct perf_event_attr attr;
     virPerfEventPtr event = virPerfGetEvent(perf, type);
-    virPerfEventAttrPtr event_attr = virPerfGetEventAttr(type);
+    virPerfEventAttrPtr event_attr = &attrs[type];
 
-    if (!event || !event_attr)
+    if (!event)
         return -1;
 
     if (event->enabled)
@@ -237,7 +226,7 @@ virPerfEventEnable(virPerfPtr perf,
                                        type == VIR_PERF_EVENT_MBML)) {
         virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
                        _("unable to enable host cpu perf event for %s"),
-                       virPerfEventTypeToString(event->type));
+                       virPerfEventTypeToString(type));
         return -1;
     }
 
-- 
2.9.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/2] perf: get rid of pointless virPerfGetEventAttr() method
Posted by Ján Tomko 8 years, 10 months ago 
On Wed, Apr 12, 2017 at 12:27:11PM +0100, Daniel P. Berrange wrote:
>The virPerfGetEventAttr method contains a totally pointless
>loop. Remove it, verify the array size statically, and then
>just use an array index to access the perf event.
>
>Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
>---
> src/util/virperf.c | 143 +++++++++++++++++++++++++----------------------------
> 1 file changed, 66 insertions(+), 77 deletions(-)
>
>diff --git a/src/util/virperf.c b/src/util/virperf.c
>index e39cebb..7366efc 100644
>--- a/src/util/virperf.c
>+++ b/src/util/virperf.c

[...]

>+    [VIR_PERF_EVENT_STALLED_CYCLES_BACKEND] = {
>+        .attrType = PERF_TYPE_HARDWARE,
>+        .attrConfig = PERF_COUNT_HW_STALLED_CYCLES_BACKEND
>+    },
>+    [VIR_PERF_EVENT_REF_CPU_CYCLES] = {

Up to VIR_PERF_EVENT_REF_CPU_CYCLES, the initialization is indexed
by the enum value...

> # ifdef PERF_COUNT_HW_REF_CPU_CYCLES
>-     .attrType = PERF_TYPE_HARDWARE,
>-     .attrConfig = PERF_COUNT_HW_REF_CPU_CYCLES
>+        .attrType = PERF_TYPE_HARDWARE,
>+        .attrConfig = PERF_COUNT_HW_REF_CPU_CYCLES
> # else
>-     .attrType = 0,
>-     .attrConfig = 0,
>+        .attrType = 0,
>+        .attrConfig = 0,
> # endif
>     },
>-    {.type = VIR_PERF_EVENT_CPU_CLOCK,
>-     .attrType = PERF_TYPE_SOFTWARE,
>+    {.attrType = PERF_TYPE_SOFTWARE,
>      .attrConfig = PERF_COUNT_SW_CPU_CLOCK},

... but the rest does not.

ACK with the inconsitency fixed.

Jan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/2] perf: get rid of pointless virPerfGetEventAttr() method
Posted by Daniel P. Berrange 8 years, 10 months ago 
On Wed, Apr 12, 2017 at 03:26:36PM +0200, Ján Tomko wrote:
> On Wed, Apr 12, 2017 at 12:27:11PM +0100, Daniel P. Berrange wrote:
> > The virPerfGetEventAttr method contains a totally pointless
> > loop. Remove it, verify the array size statically, and then
> > just use an array index to access the perf event.
> > 
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
> > src/util/virperf.c | 143 +++++++++++++++++++++++++----------------------------
> > 1 file changed, 66 insertions(+), 77 deletions(-)
> > 
> > diff --git a/src/util/virperf.c b/src/util/virperf.c
> > index e39cebb..7366efc 100644
> > --- a/src/util/virperf.c
> > +++ b/src/util/virperf.c
> 
> [...]
> 
> > +    [VIR_PERF_EVENT_STALLED_CYCLES_BACKEND] = {
> > +        .attrType = PERF_TYPE_HARDWARE,
> > +        .attrConfig = PERF_COUNT_HW_STALLED_CYCLES_BACKEND
> > +    },
> > +    [VIR_PERF_EVENT_REF_CPU_CYCLES] = {
> 
> Up to VIR_PERF_EVENT_REF_CPU_CYCLES, the initialization is indexed
> by the enum value...
> 
> > # ifdef PERF_COUNT_HW_REF_CPU_CYCLES
> > -     .attrType = PERF_TYPE_HARDWARE,
> > -     .attrConfig = PERF_COUNT_HW_REF_CPU_CYCLES
> > +        .attrType = PERF_TYPE_HARDWARE,
> > +        .attrConfig = PERF_COUNT_HW_REF_CPU_CYCLES
> > # else
> > -     .attrType = 0,
> > -     .attrConfig = 0,
> > +        .attrType = 0,
> > +        .attrConfig = 0,
> > # endif
> >     },
> > -    {.type = VIR_PERF_EVENT_CPU_CLOCK,
> > -     .attrType = PERF_TYPE_SOFTWARE,
> > +    {.attrType = PERF_TYPE_SOFTWARE,
> >      .attrConfig = PERF_COUNT_SW_CPU_CLOCK},
> 
> ... but the rest does not.
> 
> ACK with the inconsitency fixed.

Opps, yes, my mistake when resolving the conflict


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
  • [libvirt] [PATCH v2 1/2] perf: get rid of pointless virPerfGetEventAttr() method

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.