diff mbox

[16/17] tools/kvm_stat: add interactive command 'r'

Message ID 20170220154211.11882-17-raspl@linux.vnet.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Stefan Raspl Feb. 20, 2017, 3:42 p.m. UTC
Provide an interactive command to reset the tracepoint statistics.
Requires some extra work for debugfs, as the counters cannot be reset.
On the up side, offers us the opportunity to have debugfs values reset
on startup and whenever a filter is modified, becoming consistent with
the tracepoint provider. As a bonus, 'kvmstat -dt' will now provide
useful output, instead of mixing values in totally different orders of
magnitude.
Furthermore, we avoid unnecessary resets when any of the filters is
"changed" interactively to the previous value.

Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
---
 tools/kvm/kvm_stat/kvm_stat     | 66 +++++++++++++++++++++++++++++++----------
 tools/kvm/kvm_stat/kvm_stat.txt |  2 ++
 2 files changed, 53 insertions(+), 15 deletions(-)

Comments

Janosch Frank March 10, 2017, 11:37 a.m. UTC | #1
On 20.02.2017 16:42, Stefan Raspl wrote:
> Provide an interactive command to reset the tracepoint statistics.
> Requires some extra work for debugfs, as the counters cannot be reset.
> On the up side, offers us the opportunity to have debugfs values reset
                  ^
                  add "this"

> on startup and whenever a filter is modified, becoming consistent with
> the tracepoint provider. As a bonus, 'kvmstat -dt' will now provide
> useful output, instead of mixing values in totally different orders of
> magnitude.
> Furthermore, we avoid unnecessary resets when any of the filters is
> "changed" interactively to the previous value.

Acked-by: Janosch Frank <frankja@linux.vnet.ibm.com>

> 
> Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
> ---
>  tools/kvm/kvm_stat/kvm_stat     | 66 +++++++++++++++++++++++++++++++----------
>  tools/kvm/kvm_stat/kvm_stat.txt |  2 ++
>  2 files changed, 53 insertions(+), 15 deletions(-)
> 
> diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
> index e0f28e1..f9f653a 100755
> --- a/tools/kvm/kvm_stat/kvm_stat
> +++ b/tools/kvm/kvm_stat/kvm_stat
> @@ -715,15 +715,23 @@ class TracepointProvider(object):
>                      ret[name] += val
>          return ret
> 
> +    def reset(self):
> +        """Reset all field counters"""
> +        for group in self.group_leaders:
> +            for event in group.events:
> +                event.reset()
> +
> 
>  class DebugfsProvider(object):
>      """Provides data from the files that KVM creates in the kvm debugfs
>      folder."""
>      def __init__(self):
>          self._fields = self.get_available_fields()
> +        self._baseline = {}
>          self._pid = 0
>          self.do_read = True
>          self.paths = []
> +        self.reset()
> 
>      def get_available_fields(self):
>          """"Returns a list of available fields.
> @@ -740,6 +748,7 @@ class DebugfsProvider(object):
>      @fields.setter
>      def fields(self, fields):
>          self._fields = fields
> +        self.reset()
> 
>      @property
>      def pid(self):
> @@ -757,10 +766,11 @@ class DebugfsProvider(object):
>              self.paths = filter(lambda x: "{}-".format(pid) in x, vms)
> 
>          else:
> -            self.paths = ['']
> +            self.paths = []

This is more a cleanup than new code, isn't it?

[...]
> @@ -1188,7 +1224,7 @@ Interactive Commands:
>     g     filter by guest name
>     p     filter by PID
>     q     quit
> -   x     toggle reporting of stats for individual child trace events
> +   r     reset stats
>  Press any other key to refresh statistics immediately.
>  """

This is inconsistent to the part below.

> diff --git a/tools/kvm/kvm_stat/kvm_stat.txt b/tools/kvm/kvm_stat/kvm_stat.txt
> index c3ab6a2..109431b 100644
> --- a/tools/kvm/kvm_stat/kvm_stat.txt
> +++ b/tools/kvm/kvm_stat/kvm_stat.txt
> @@ -39,6 +39,8 @@ INTERACTIVE COMMANDS
> 
>  *q*::	quit
> 
> +*r*::	reset stats
> +
>  *x*::	toggle reporting of stats for child trace events
> 
>  Press any other key to refresh statistics immediately.
>
Stefan Raspl March 10, 2017, 12:32 p.m. UTC | #2
On 10.03.2017 12:37, Janosch Frank wrote:
>> @@ -757,10 +766,11 @@ class DebugfsProvider(object):
>>              self.paths = filter(lambda x: "{}-".format(pid) in x, vms)
>>
>>          else:
>> -            self.paths = ['']
>> +            self.paths = []
> 
> This is more a cleanup than new code, isn't it?

Not quite: The former holds an element, while the latter doesn't.
Therefore a subsequent 'for path in self.paths:' would execute with
the former version, but won't with the latter.

> [...]
>> @@ -1188,7 +1224,7 @@ Interactive Commands:
>>     g     filter by guest name
>>     p     filter by PID
>>     q     quit
>> -   x     toggle reporting of stats for individual child trace events
>> +   r     reset stats
>>  Press any other key to refresh statistics immediately.
>>  """
> 
> This is inconsistent to the part below.

Yup, accidental deletion - good catch, thanks!

Ciao,
Stefan
diff mbox

Patch

diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
index e0f28e1..f9f653a 100755
--- a/tools/kvm/kvm_stat/kvm_stat
+++ b/tools/kvm/kvm_stat/kvm_stat
@@ -715,15 +715,23 @@  class TracepointProvider(object):
                     ret[name] += val
         return ret
 
+    def reset(self):
+        """Reset all field counters"""
+        for group in self.group_leaders:
+            for event in group.events:
+                event.reset()
+
 
 class DebugfsProvider(object):
     """Provides data from the files that KVM creates in the kvm debugfs
     folder."""
     def __init__(self):
         self._fields = self.get_available_fields()
+        self._baseline = {}
         self._pid = 0
         self.do_read = True
         self.paths = []
+        self.reset()
 
     def get_available_fields(self):
         """"Returns a list of available fields.
@@ -740,6 +748,7 @@  class DebugfsProvider(object):
     @fields.setter
     def fields(self, fields):
         self._fields = fields
+        self.reset()
 
     @property
     def pid(self):
@@ -757,10 +766,11 @@  class DebugfsProvider(object):
             self.paths = filter(lambda x: "{}-".format(pid) in x, vms)
 
         else:
-            self.paths = ['']
+            self.paths = []
             self.do_read = True
+        self.reset()
 
-    def read(self):
+    def read(self, reset=0):
         """Returns a dict with format:'file name / field -> current value'."""
         results = {}
 
@@ -768,10 +778,22 @@  class DebugfsProvider(object):
         if not self.do_read:
             return results
 
-        for path in self.paths:
+        paths = self.paths
+        if self._pid == 0:
+            paths = []
+            for entry in os.walk(PATH_DEBUGFS_KVM):
+                for dir in entry[1]:
+                    paths.append(dir)
+        for path in paths:
             for field in self._fields:
-                results[field] = results.get(field, 0) \
-                                 + self.read_field(field, path)
+                value = self.read_field(field, path)
+                key = path + field
+                if reset:
+                    self._baseline[key] = value
+                if self._baseline.get(key, -1) == -1:
+                    self._baseline[key] = value
+                results[field] = (results.get(field, 0) + value -
+                                  self._baseline.get(key, 0))
 
         return results
 
@@ -785,6 +807,11 @@  class DebugfsProvider(object):
         except IOError:
             return 0
 
+    def reset(self):
+        """Reset field counters"""
+        self._baseline = {}
+        self.read(1)
+
 
 class Stats(object):
     """Manages the data providers and the data they provide.
@@ -821,14 +848,20 @@  class Stats(object):
         for provider in self.providers:
             provider.pid = self._pid_filter
 
+    def reset(self):
+        self.values = {}
+        for provider in self.providers:
+            provider.reset()
+
     @property
     def fields_filter(self):
         return self._fields_filter
 
     @fields_filter.setter
     def fields_filter(self, fields_filter):
-        self._fields_filter = fields_filter
-        self.update_provider_filters()
+        if fields_filter != self._fields_filter:
+            self._fields_filter = fields_filter
+            self.update_provider_filters()
 
     @property
     def pid_filter(self):
@@ -836,9 +869,10 @@  class Stats(object):
 
     @pid_filter.setter
     def pid_filter(self, pid):
-        self._pid_filter = pid
-        self.values = {}
-        self.update_provider_pid()
+        if pid != self._pid_filter:
+            self._pid_filter = pid
+            self.values = {}
+            self.update_provider_pid()
 
     def get(self):
         """Returns a dict with field -> (value, delta to last value) of all
@@ -846,11 +880,9 @@  class Stats(object):
         for provider in self.providers:
             new = provider.read()
             for key in provider.fields:
-                oldval = self.values.get(key, (0, 0))
+                oldval = self.values.get(key, (0, 0))[0]
                 newval = new.get(key, 0)
-                newdelta = None
-                if oldval is not None:
-                    newdelta = newval - oldval[0]
+                newdelta = newval - oldval
                 self.values[key] = (newval, newdelta)
         return self.values
 
@@ -1116,6 +1148,10 @@  class Tui(object):
                 if char == 'p':
                     self.show_vm_selection_by_pid()
                     sleeptime = DELAY_INITIAL
+                if char == 'r':
+                    self.refresh_header()
+                    self.stats.reset()
+                    sleeptime = DELAY_INITIAL
             except KeyboardInterrupt:
                 break
             except curses.error:
@@ -1188,7 +1224,7 @@  Interactive Commands:
    g     filter by guest name
    p     filter by PID
    q     quit
-   x     toggle reporting of stats for individual child trace events
+   r     reset stats
 Press any other key to refresh statistics immediately.
 """
 
diff --git a/tools/kvm/kvm_stat/kvm_stat.txt b/tools/kvm/kvm_stat/kvm_stat.txt
index c3ab6a2..109431b 100644
--- a/tools/kvm/kvm_stat/kvm_stat.txt
+++ b/tools/kvm/kvm_stat/kvm_stat.txt
@@ -39,6 +39,8 @@  INTERACTIVE COMMANDS
 
 *q*::	quit
 
+*r*::	reset stats
+
 *x*::	toggle reporting of stats for child trace events
 
 Press any other key to refresh statistics immediately.