tools/kvm_stat: separate drilldown and fields filtering

Drilldown (i.e. toggle display of child trace events) was implemented by
overriding the fields filter. This resulted in inconsistencies: E.g. when
drilldown was not active, adding a filter that also matches child trace
events would not only filter fields according to the filter, but also add
in the child trace events matching the filter. E.g. on x86, setting
'kvm_userspace_exit' as the fields filter after startup would result in
display of kvm_userspace_exit(DCR), although that wasn't previously
present - not exactly what one would expect from a filter.
This patch addresses the issue by keeping drilldown and fields filter
separate. While at it, we also fix a PEP8 issue by adding a blank line
at one place (since we're in the area...).
We implement this by adding a framework that also allows to define a
taxonomy among the debugfs events to identify child trace events. I.e.
drilldown using 'x' can now also work with debugfs. A respective parent-
child relationship is only known for S390 at the moment, but could be
added adjusting other platforms' ARCH.dbg_is_child() methods
accordingly.

Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This commit is contained in:
Stefan Raspl 2018-02-22 12:16:28 +01:00 committed by Paolo Bonzini
parent 516f1190a1
commit 18e8f4100e

View file

@ -228,6 +228,7 @@ IOCTL_NUMBERS = {
} }
ENCODING = locale.getpreferredencoding(False) ENCODING = locale.getpreferredencoding(False)
TRACE_FILTER = re.compile(r'^[^\(]*$')
class Arch(object): class Arch(object):
@ -260,6 +261,11 @@ class Arch(object):
return ArchX86(SVM_EXIT_REASONS) return ArchX86(SVM_EXIT_REASONS)
return return
def tracepoint_is_child(self, field):
if (TRACE_FILTER.match(field)):
return None
return field.split('(', 1)[0]
class ArchX86(Arch): class ArchX86(Arch):
def __init__(self, exit_reasons): def __init__(self, exit_reasons):
@ -267,6 +273,10 @@ class ArchX86(Arch):
self.ioctl_numbers = IOCTL_NUMBERS self.ioctl_numbers = IOCTL_NUMBERS
self.exit_reasons = exit_reasons self.exit_reasons = exit_reasons
def debugfs_is_child(self, field):
""" Returns name of parent if 'field' is a child, None otherwise """
return None
class ArchPPC(Arch): class ArchPPC(Arch):
def __init__(self): def __init__(self):
@ -282,6 +292,10 @@ class ArchPPC(Arch):
self.ioctl_numbers['SET_FILTER'] = 0x80002406 | char_ptr_size << 16 self.ioctl_numbers['SET_FILTER'] = 0x80002406 | char_ptr_size << 16
self.exit_reasons = {} self.exit_reasons = {}
def debugfs_is_child(self, field):
""" Returns name of parent if 'field' is a child, None otherwise """
return None
class ArchA64(Arch): class ArchA64(Arch):
def __init__(self): def __init__(self):
@ -289,6 +303,10 @@ class ArchA64(Arch):
self.ioctl_numbers = IOCTL_NUMBERS self.ioctl_numbers = IOCTL_NUMBERS
self.exit_reasons = AARCH64_EXIT_REASONS self.exit_reasons = AARCH64_EXIT_REASONS
def debugfs_is_child(self, field):
""" Returns name of parent if 'field' is a child, None otherwise """
return None
class ArchS390(Arch): class ArchS390(Arch):
def __init__(self): def __init__(self):
@ -296,6 +314,12 @@ class ArchS390(Arch):
self.ioctl_numbers = IOCTL_NUMBERS self.ioctl_numbers = IOCTL_NUMBERS
self.exit_reasons = None self.exit_reasons = None
def debugfs_is_child(self, field):
""" Returns name of parent if 'field' is a child, None otherwise """
if field.startswith('instruction_'):
return 'exit_instruction'
ARCH = Arch.get_arch() ARCH = Arch.get_arch()
@ -472,6 +496,10 @@ class Event(object):
class Provider(object): class Provider(object):
"""Encapsulates functionalities used by all providers.""" """Encapsulates functionalities used by all providers."""
def __init__(self, pid):
self.child_events = False
self.pid = pid
@staticmethod @staticmethod
def is_field_wanted(fields_filter, field): def is_field_wanted(fields_filter, field):
"""Indicate whether field is valid according to fields_filter.""" """Indicate whether field is valid according to fields_filter."""
@ -499,7 +527,7 @@ class TracepointProvider(Provider):
self.group_leaders = [] self.group_leaders = []
self.filters = self._get_filters() self.filters = self._get_filters()
self.update_fields(fields_filter) self.update_fields(fields_filter)
self.pid = pid super(TracepointProvider, self).__init__(pid)
@staticmethod @staticmethod
def _get_filters(): def _get_filters():
@ -519,7 +547,7 @@ class TracepointProvider(Provider):
return filters return filters
def _get_available_fields(self): def _get_available_fields(self):
"""Returns a list of available event's of format 'event name(filter """Returns a list of available events of format 'event name(filter
name)'. name)'.
All available events have directories under All available events have directories under
@ -547,7 +575,8 @@ class TracepointProvider(Provider):
def update_fields(self, fields_filter): def update_fields(self, fields_filter):
"""Refresh fields, applying fields_filter""" """Refresh fields, applying fields_filter"""
self.fields = [field for field in self._get_available_fields() self.fields = [field for field in self._get_available_fields()
if self.is_field_wanted(fields_filter, field)] if self.is_field_wanted(fields_filter, field) or
ARCH.tracepoint_is_child(field)]
@staticmethod @staticmethod
def _get_online_cpus(): def _get_online_cpus():
@ -668,8 +697,12 @@ class TracepointProvider(Provider):
ret = defaultdict(int) ret = defaultdict(int)
for group in self.group_leaders: for group in self.group_leaders:
for name, val in group.read().items(): for name, val in group.read().items():
if name in self._fields: if name not in self._fields:
ret[name] += val continue
parent = ARCH.tracepoint_is_child(name)
if parent:
name += ' ' + parent
ret[name] += val
return ret return ret
def reset(self): def reset(self):
@ -687,7 +720,7 @@ class DebugfsProvider(Provider):
self._baseline = {} self._baseline = {}
self.do_read = True self.do_read = True
self.paths = [] self.paths = []
self.pid = pid super(DebugfsProvider, self).__init__(pid)
if include_past: if include_past:
self._restore() self._restore()
@ -702,7 +735,8 @@ class DebugfsProvider(Provider):
def update_fields(self, fields_filter): def update_fields(self, fields_filter):
"""Refresh fields, applying fields_filter""" """Refresh fields, applying fields_filter"""
self._fields = [field for field in self._get_available_fields() self._fields = [field for field in self._get_available_fields()
if self.is_field_wanted(fields_filter, field)] if self.is_field_wanted(fields_filter, field) or
ARCH.debugfs_is_child(field)]
@property @property
def fields(self): def fields(self):
@ -763,14 +797,15 @@ class DebugfsProvider(Provider):
self._baseline[key] = 0 self._baseline[key] = 0
if self._baseline.get(key, -1) == -1: if self._baseline.get(key, -1) == -1:
self._baseline[key] = value self._baseline[key] = value
increment = (results.get(field, 0) + value - parent = ARCH.debugfs_is_child(field)
self._baseline.get(key, 0)) if parent:
if by_guest: field = field + ' ' + parent
pid = key.split('-')[0] else:
if pid in results: if by_guest:
results[pid] += increment field = key.split('-')[0] # set 'field' to 'pid'
else: increment = value - self._baseline.get(key, 0)
results[pid] = increment if field in results:
results[field] += increment
else: else:
results[field] = increment results[field] = increment
@ -812,6 +847,7 @@ class Stats(object):
self._pid_filter = options.pid self._pid_filter = options.pid
self._fields_filter = options.fields self._fields_filter = options.fields
self.values = {} self.values = {}
self._child_events = False
def _get_providers(self, options): def _get_providers(self, options):
"""Returns a list of data providers depending on the passed options.""" """Returns a list of data providers depending on the passed options."""
@ -860,12 +896,29 @@ class Stats(object):
for provider in self.providers: for provider in self.providers:
provider.pid = self._pid_filter provider.pid = self._pid_filter
@property
def child_events(self):
return self._child_events
@child_events.setter
def child_events(self, val):
self._child_events = val
for provider in self.providers:
provider.child_events = val
def get(self, by_guest=0): def get(self, by_guest=0):
"""Returns a dict with field -> (value, delta to last value) of all """Returns a dict with field -> (value, delta to last value) of all
provider data.""" provider data.
Key formats:
* plain: 'key' is event name
* child-parent: 'key' is in format '<child> <parent>'
* pid: 'key' is the pid of the guest, and the record contains the
aggregated event data
These formats are generated by the providers, and handled in class TUI.
"""
for provider in self.providers: for provider in self.providers:
new = provider.read(by_guest=by_guest) new = provider.read(by_guest=by_guest)
for key in new if by_guest else provider.fields: for key in new:
oldval = self.values.get(key, EventStat(0, 0)).value oldval = self.values.get(key, EventStat(0, 0)).value
newval = new.get(key, 0) newval = new.get(key, 0)
newdelta = newval - oldval newdelta = newval - oldval
@ -898,10 +951,10 @@ class Stats(object):
self.get(to_pid) self.get(to_pid)
return 0 return 0
DELAY_DEFAULT = 3.0 DELAY_DEFAULT = 3.0
MAX_GUEST_NAME_LEN = 48 MAX_GUEST_NAME_LEN = 48
MAX_REGEX_LEN = 44 MAX_REGEX_LEN = 44
DEFAULT_REGEX = r'^[^\(]*$'
SORT_DEFAULT = 0 SORT_DEFAULT = 0
@ -1031,14 +1084,6 @@ class Tui(object):
return name return name
def _update_drilldown(self):
"""Sets or removes a filter that only allows fields without braces."""
if not self.stats.fields_filter:
self.stats.fields_filter = DEFAULT_REGEX
elif self.stats.fields_filter == DEFAULT_REGEX:
self.stats.fields_filter = None
def _update_pid(self, pid): def _update_pid(self, pid):
"""Propagates pid selection to stats object.""" """Propagates pid selection to stats object."""
self.screen.addstr(4, 1, 'Updating pid filter...') self.screen.addstr(4, 1, 'Updating pid filter...')
@ -1060,8 +1105,7 @@ class Tui(object):
.format(pid, gname), curses.A_BOLD) .format(pid, gname), curses.A_BOLD)
else: else:
self.screen.addstr(0, 0, 'kvm statistics - summary', curses.A_BOLD) self.screen.addstr(0, 0, 'kvm statistics - summary', curses.A_BOLD)
if self.stats.fields_filter and self.stats.fields_filter \ if self.stats.fields_filter:
!= DEFAULT_REGEX:
regex = self.stats.fields_filter regex = self.stats.fields_filter
if len(regex) > MAX_REGEX_LEN: if len(regex) > MAX_REGEX_LEN:
regex = regex[:MAX_REGEX_LEN] + '...' regex = regex[:MAX_REGEX_LEN] + '...'
@ -1077,6 +1121,9 @@ class Tui(object):
self.screen.refresh() self.screen.refresh()
def _refresh_body(self, sleeptime): def _refresh_body(self, sleeptime):
def is_child_field(field):
return field.find('(') != -1
row = 3 row = 3
self.screen.move(row, 0) self.screen.move(row, 0)
self.screen.clrtobot() self.screen.clrtobot()
@ -1084,7 +1131,11 @@ class Tui(object):
total = 0. total = 0.
ctotal = 0. ctotal = 0.
for key, values in stats.items(): for key, values in stats.items():
if key.find('(') == -1: if self._display_guests:
if self.get_gname_from_pid(key):
total += values.value
continue
if not key.find(' ') != -1:
total += values.value total += values.value
else: else:
ctotal += values.value ctotal += values.value
@ -1101,19 +1152,26 @@ class Tui(object):
# sort by overall value # sort by overall value
return v.value return v.value
sorted_items = sorted(stats.items(), key=sortkey, reverse=True)
# print events
tavg = 0 tavg = 0
for key, values in sorted(stats.items(), key=sortkey, reverse=True): for key, values in sorted_items:
if row >= self.screen.getmaxyx()[0] - 1: if row >= self.screen.getmaxyx()[0] - 1:
break break
if not values.value and not values.delta: if values == (0, 0):
break continue
if not self.stats.child_events and key.find(' ') != -1:
continue
if values.value is not None: if values.value is not None:
cur = int(round(values.delta / sleeptime)) if values.delta else '' cur = int(round(values.delta / sleeptime)) if values.delta else ''
if self._display_guests: if self._display_guests:
key = self.get_gname_from_pid(key) key = self.get_gname_from_pid(key)
self.screen.addstr(row, 1, '%-40s %10d%7.1f %8s' % if not key:
(key, values.value, values.value * 100 / total, continue
cur)) self.screen.addstr(row, 1, '%-40s %10d%7.1f %8s' % (key
.split(' ')[0], values.value,
values.value * 100 / total, cur))
if cur != '' and key.find('(') == -1: if cur != '' and key.find('(') == -1:
tavg += cur tavg += cur
row += 1 row += 1
@ -1189,7 +1247,7 @@ class Tui(object):
regex = self.screen.getstr().decode(ENCODING) regex = self.screen.getstr().decode(ENCODING)
curses.noecho() curses.noecho()
if len(regex) == 0: if len(regex) == 0:
self.stats.fields_filter = DEFAULT_REGEX self.stats.fields_filter = ''
self._refresh_header() self._refresh_header()
return return
try: try:
@ -1307,7 +1365,7 @@ class Tui(object):
self._display_guests = not self._display_guests self._display_guests = not self._display_guests
self._refresh_header() self._refresh_header()
if char == 'c': if char == 'c':
self.stats.fields_filter = DEFAULT_REGEX self.stats.fields_filter = ''
self._refresh_header(0) self._refresh_header(0)
self._update_pid(0) self._update_pid(0)
if char == 'f': if char == 'f':
@ -1332,9 +1390,7 @@ class Tui(object):
curses.curs_set(0) curses.curs_set(0)
sleeptime = self._delay_initial sleeptime = self._delay_initial
if char == 'x': if char == 'x':
self._update_drilldown() self.stats.child_events = not self.stats.child_events
# prevents display of current values on next refresh
self.stats.get(self._display_guests)
except KeyboardInterrupt: except KeyboardInterrupt:
break break
except curses.error: except curses.error:
@ -1348,7 +1404,8 @@ def batch(stats):
time.sleep(1) time.sleep(1)
s = stats.get() s = stats.get()
for key, values in sorted(s.items()): for key, values in sorted(s.items()):
print('%-42s%10d%10d' % (key, values.value, values.delta)) print('%-42s%10d%10d' % (key.split(' ')[0], values.value,
values.delta))
except KeyboardInterrupt: except KeyboardInterrupt:
pass pass
@ -1359,7 +1416,7 @@ def log(stats):
def banner(): def banner():
for key in keys: for key in keys:
print(key, end=' ') print(key.split(' ')[0], end=' ')
print() print()
def statline(): def statline():
@ -1470,7 +1527,7 @@ Press any other key to refresh statistics immediately.
) )
optparser.add_option('-f', '--fields', optparser.add_option('-f', '--fields',
action='store', action='store',
default=DEFAULT_REGEX, default='',
dest='fields', dest='fields',
help='''fields to display (regex) help='''fields to display (regex)
"-f help" for a list of available events''', "-f help" for a list of available events''',