| Message ID | 20191217020600.10268-1-gshan@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] tools/kvm_stat: Fix kvm_exit filter name | expand |
On Tue, Dec 17, 2019 at 01:06:00PM +1100, Gavin Shan wrote: > The filter name is fixed to "exit_reason" for some kvm_exit events, no > matter what architect we have. Actually, the filter name ("exit_reason") > is only applicable to x86, meaning it's broken on other architects > including aarch64. > > This fixes the issue by providing various kvm_exit filter names, depending > on architect we're on. Afterwards, the variable filter name is picked and > applied by ioctl(fd, SET_FILTER). > > Reported-by: Andrew Jones <drjones@redhat.com> This wasn't reported by me - I was just the middleman. Credit should go to Jeff Bastian <jbastian@redhat.com> > Signed-off-by: Gavin Shan <gshan@redhat.com> > --- > v2: Rename exit_field to exit_reason_field > Fix the name to esr_ec for aarch64 > --- > tools/kvm/kvm_stat/kvm_stat | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat > index ad1b9e646c49..4cf93110c259 100755 > --- a/tools/kvm/kvm_stat/kvm_stat > +++ b/tools/kvm/kvm_stat/kvm_stat > @@ -270,6 +270,7 @@ class ArchX86(Arch): > def __init__(self, exit_reasons): > self.sc_perf_evt_open = 298 > self.ioctl_numbers = IOCTL_NUMBERS > + self.exit_reason_field = 'exit_reason' > self.exit_reasons = exit_reasons > > def debugfs_is_child(self, field): > @@ -289,6 +290,7 @@ class ArchPPC(Arch): > # numbers depend on the wordsize. > char_ptr_size = ctypes.sizeof(ctypes.c_char_p) > self.ioctl_numbers['SET_FILTER'] = 0x80002406 | char_ptr_size << 16 > + self.exit_reason_field = 'exit_nr' > self.exit_reasons = {} > > def debugfs_is_child(self, field): > @@ -300,6 +302,7 @@ class ArchA64(Arch): > def __init__(self): > self.sc_perf_evt_open = 241 > self.ioctl_numbers = IOCTL_NUMBERS > + self.exit_reason_field = 'esr_ec' > self.exit_reasons = AARCH64_EXIT_REASONS > > def debugfs_is_child(self, field): > @@ -311,6 +314,7 @@ class ArchS390(Arch): > def __init__(self): > self.sc_perf_evt_open = 331 > self.ioctl_numbers = IOCTL_NUMBERS > + self.exit_reason_field = None > self.exit_reasons = None > > def debugfs_is_child(self, field): > @@ -541,8 +545,8 @@ class TracepointProvider(Provider): > """ > filters = {} > filters['kvm_userspace_exit'] = ('reason', USERSPACE_EXIT_REASONS) > - if ARCH.exit_reasons: > - filters['kvm_exit'] = ('exit_reason', ARCH.exit_reasons) > + if ARCH.exit_reason_field and ARCH.exit_reasons: > + filters['kvm_exit'] = (ARCH.exit_reason_field, ARCH.exit_reasons) > return filters > > def _get_available_fields(self): > -- > 2.23.0 > Looks like a reasonable fix to me. Reviewed-by: Andrew Jones <drjones@redhat.com>
On 12/18/19 7:45 PM, Andrew Jones wrote: > On Tue, Dec 17, 2019 at 01:06:00PM +1100, Gavin Shan wrote: >> The filter name is fixed to "exit_reason" for some kvm_exit events, no >> matter what architect we have. Actually, the filter name ("exit_reason") >> is only applicable to x86, meaning it's broken on other architects >> including aarch64. >> >> This fixes the issue by providing various kvm_exit filter names, depending >> on architect we're on. Afterwards, the variable filter name is picked and >> applied by ioctl(fd, SET_FILTER). >> >> Reported-by: Andrew Jones <drjones@redhat.com> > > This wasn't reported by me - I was just the middleman. Credit should go > to Jeff Bastian <jbastian@redhat.com> > Sure. Paolo, please let me know if I need post a v3 to fix it up :) >> Signed-off-by: Gavin Shan <gshan@redhat.com> >> --- >> v2: Rename exit_field to exit_reason_field >> Fix the name to esr_ec for aarch64 >> --- >> tools/kvm/kvm_stat/kvm_stat | 8 ++++++-- >> 1 file changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat >> index ad1b9e646c49..4cf93110c259 100755 >> --- a/tools/kvm/kvm_stat/kvm_stat >> +++ b/tools/kvm/kvm_stat/kvm_stat >> @@ -270,6 +270,7 @@ class ArchX86(Arch): >> def __init__(self, exit_reasons): >> self.sc_perf_evt_open = 298 >> self.ioctl_numbers = IOCTL_NUMBERS >> + self.exit_reason_field = 'exit_reason' >> self.exit_reasons = exit_reasons >> >> def debugfs_is_child(self, field): >> @@ -289,6 +290,7 @@ class ArchPPC(Arch): >> # numbers depend on the wordsize. >> char_ptr_size = ctypes.sizeof(ctypes.c_char_p) >> self.ioctl_numbers['SET_FILTER'] = 0x80002406 | char_ptr_size << 16 >> + self.exit_reason_field = 'exit_nr' >> self.exit_reasons = {} >> >> def debugfs_is_child(self, field): >> @@ -300,6 +302,7 @@ class ArchA64(Arch): >> def __init__(self): >> self.sc_perf_evt_open = 241 >> self.ioctl_numbers = IOCTL_NUMBERS >> + self.exit_reason_field = 'esr_ec' >> self.exit_reasons = AARCH64_EXIT_REASONS >> >> def debugfs_is_child(self, field): >> @@ -311,6 +314,7 @@ class ArchS390(Arch): >> def __init__(self): >> self.sc_perf_evt_open = 331 >> self.ioctl_numbers = IOCTL_NUMBERS >> + self.exit_reason_field = None >> self.exit_reasons = None >> >> def debugfs_is_child(self, field): >> @@ -541,8 +545,8 @@ class TracepointProvider(Provider): >> """ >> filters = {} >> filters['kvm_userspace_exit'] = ('reason', USERSPACE_EXIT_REASONS) >> - if ARCH.exit_reasons: >> - filters['kvm_exit'] = ('exit_reason', ARCH.exit_reasons) >> + if ARCH.exit_reason_field and ARCH.exit_reasons: >> + filters['kvm_exit'] = (ARCH.exit_reason_field, ARCH.exit_reasons) >> return filters >> >> def _get_available_fields(self): >> -- >> 2.23.0 >> > > Looks like a reasonable fix to me. > > Reviewed-by: Andrew Jones <drjones@redhat.com> > Thanks for the review and comments. Regards, Gavin
On 12/19/19 2:33 PM, Gavin Shan wrote: > On 12/18/19 7:45 PM, Andrew Jones wrote: >> On Tue, Dec 17, 2019 at 01:06:00PM +1100, Gavin Shan wrote: >>> The filter name is fixed to "exit_reason" for some kvm_exit events, no >>> matter what architect we have. Actually, the filter name ("exit_reason") >>> is only applicable to x86, meaning it's broken on other architects >>> including aarch64. >>> >>> This fixes the issue by providing various kvm_exit filter names, depending >>> on architect we're on. Afterwards, the variable filter name is picked and >>> applied by ioctl(fd, SET_FILTER). >>> >>> Reported-by: Andrew Jones <drjones@redhat.com> >> >> This wasn't reported by me - I was just the middleman. Credit should go >> to Jeff Bastian <jbastian@redhat.com> >> > > Sure. Paolo, please let me know if I need post a v3 to fix it up :) > Ping, Paolo :) >>> Signed-off-by: Gavin Shan <gshan@redhat.com> >>> --- >>> v2: Rename exit_field to exit_reason_field >>> Fix the name to esr_ec for aarch64 >>> --- >>> tools/kvm/kvm_stat/kvm_stat | 8 ++++++-- >>> 1 file changed, 6 insertions(+), 2 deletions(-) >>> >>> diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat >>> index ad1b9e646c49..4cf93110c259 100755 >>> --- a/tools/kvm/kvm_stat/kvm_stat >>> +++ b/tools/kvm/kvm_stat/kvm_stat >>> @@ -270,6 +270,7 @@ class ArchX86(Arch): >>> def __init__(self, exit_reasons): >>> self.sc_perf_evt_open = 298 >>> self.ioctl_numbers = IOCTL_NUMBERS >>> + self.exit_reason_field = 'exit_reason' >>> self.exit_reasons = exit_reasons >>> def debugfs_is_child(self, field): >>> @@ -289,6 +290,7 @@ class ArchPPC(Arch): >>> # numbers depend on the wordsize. >>> char_ptr_size = ctypes.sizeof(ctypes.c_char_p) >>> self.ioctl_numbers['SET_FILTER'] = 0x80002406 | char_ptr_size << 16 >>> + self.exit_reason_field = 'exit_nr' >>> self.exit_reasons = {} >>> def debugfs_is_child(self, field): >>> @@ -300,6 +302,7 @@ class ArchA64(Arch): >>> def __init__(self): >>> self.sc_perf_evt_open = 241 >>> self.ioctl_numbers = IOCTL_NUMBERS >>> + self.exit_reason_field = 'esr_ec' >>> self.exit_reasons = AARCH64_EXIT_REASONS >>> def debugfs_is_child(self, field): >>> @@ -311,6 +314,7 @@ class ArchS390(Arch): >>> def __init__(self): >>> self.sc_perf_evt_open = 331 >>> self.ioctl_numbers = IOCTL_NUMBERS >>> + self.exit_reason_field = None >>> self.exit_reasons = None >>> def debugfs_is_child(self, field): >>> @@ -541,8 +545,8 @@ class TracepointProvider(Provider): >>> """ >>> filters = {} >>> filters['kvm_userspace_exit'] = ('reason', USERSPACE_EXIT_REASONS) >>> - if ARCH.exit_reasons: >>> - filters['kvm_exit'] = ('exit_reason', ARCH.exit_reasons) >>> + if ARCH.exit_reason_field and ARCH.exit_reasons: >>> + filters['kvm_exit'] = (ARCH.exit_reason_field, ARCH.exit_reasons) >>> return filters >>> def _get_available_fields(self): >>> -- >>> 2.23.0 >>> >> >> Looks like a reasonable fix to me. >> >> Reviewed-by: Andrew Jones <drjones@redhat.com> >> > > Thanks for the review and comments. > > Regards, > Gavin >
diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat index ad1b9e646c49..4cf93110c259 100755 --- a/tools/kvm/kvm_stat/kvm_stat +++ b/tools/kvm/kvm_stat/kvm_stat @@ -270,6 +270,7 @@ class ArchX86(Arch): def __init__(self, exit_reasons): self.sc_perf_evt_open = 298 self.ioctl_numbers = IOCTL_NUMBERS + self.exit_reason_field = 'exit_reason' self.exit_reasons = exit_reasons def debugfs_is_child(self, field): @@ -289,6 +290,7 @@ class ArchPPC(Arch): # numbers depend on the wordsize. char_ptr_size = ctypes.sizeof(ctypes.c_char_p) self.ioctl_numbers['SET_FILTER'] = 0x80002406 | char_ptr_size << 16 + self.exit_reason_field = 'exit_nr' self.exit_reasons = {} def debugfs_is_child(self, field): @@ -300,6 +302,7 @@ class ArchA64(Arch): def __init__(self): self.sc_perf_evt_open = 241 self.ioctl_numbers = IOCTL_NUMBERS + self.exit_reason_field = 'esr_ec' self.exit_reasons = AARCH64_EXIT_REASONS def debugfs_is_child(self, field): @@ -311,6 +314,7 @@ class ArchS390(Arch): def __init__(self): self.sc_perf_evt_open = 331 self.ioctl_numbers = IOCTL_NUMBERS + self.exit_reason_field = None self.exit_reasons = None def debugfs_is_child(self, field): @@ -541,8 +545,8 @@ class TracepointProvider(Provider): """ filters = {} filters['kvm_userspace_exit'] = ('reason', USERSPACE_EXIT_REASONS) - if ARCH.exit_reasons: - filters['kvm_exit'] = ('exit_reason', ARCH.exit_reasons) + if ARCH.exit_reason_field and ARCH.exit_reasons: + filters['kvm_exit'] = (ARCH.exit_reason_field, ARCH.exit_reasons) return filters def _get_available_fields(self):
The filter name is fixed to "exit_reason" for some kvm_exit events, no matter what architect we have. Actually, the filter name ("exit_reason") is only applicable to x86, meaning it's broken on other architects including aarch64. This fixes the issue by providing various kvm_exit filter names, depending on architect we're on. Afterwards, the variable filter name is picked and applied by ioctl(fd, SET_FILTER). Reported-by: Andrew Jones <drjones@redhat.com> Signed-off-by: Gavin Shan <gshan@redhat.com> --- v2: Rename exit_field to exit_reason_field Fix the name to esr_ec for aarch64 --- tools/kvm/kvm_stat/kvm_stat | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)