[Qemu-devel] [PATCH 0/3] trace: drop orphan events from ./trace-events

[Qemu-devel] [PATCH 0/3] trace: drop orphan events from ./trace-events

[Stefan Hajnoczi]

Over time a few unused trace events have been left behind in ./trace-events.
Either the code that called them was deleted or the event was never called in
the first place.

This is a clear violation of the solider's motto "no man left behind".  It's
time to bring these trace events home.

On a more serious note, unused trace events cause confusion to users who want
to enable existing instrumentation.  On the SystemTap backend we get errors
when attempting to enable them because the static probes that these events are
based on do not exist.

Stefan Hajnoczi (3):
  trace-events: drop orphan virtio_blk_data_plane_complete_request
  trace-events: drop orphan usb_mtp_data_out
  trace-events: drop orphan iscsi trace events

 block/iscsi.c | 1 -
 trace-events  | 8 --------
 2 files changed, 9 deletions(-)

-- 
1.9.3

[Qemu-devel] [PATCH 1/3] trace-events: drop orphan virtio_blk_data_plane_complete_request

[Stefan Hajnoczi]

This trace event has not been in use since commit
b002254dbd4c19a01f29790f840f983803e26893 ("virtio-blk: Unify
{non-,}dataplane's request handlings").

Cc: Fam Zheng <famz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 trace-events | 1 -
 1 file changed, 1 deletion(-)

diff --git a/trace-events b/trace-events
index fb58963..a86c99b 100644
--- a/trace-events
+++ b/trace-events
@@ -121,7 +121,6 @@ virtio_blk_handle_read(void *req, uint64_t sector, size_t nsectors) "req %p sect
 virtio_blk_data_plane_start(void *s) "dataplane %p"
 virtio_blk_data_plane_stop(void *s) "dataplane %p"
 virtio_blk_data_plane_process_request(void *s, unsigned int out_num, unsigned int in_num, unsigned int head) "dataplane %p out_num %u in_num %u head %u"
-virtio_blk_data_plane_complete_request(void *s, unsigned int head, int ret) "dataplane %p head %u ret %d"
 
 # hw/virtio/dataplane/vring.c
 vring_setup(uint64_t physical, void *desc, void *avail, void *used) "vring physical %#"PRIx64" desc %p avail %p used %p"
-- 
1.9.3

[Qemu-devel] [PATCH 2/3] trace-events: drop orphan usb_mtp_data_out

[Stefan Hajnoczi]

This trace event was added in commit
840a178c94dbd3f5b5550fb8621620cb761de72d ("usb: mtp filesharing") but
never used.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 trace-events | 1 -
 1 file changed, 1 deletion(-)

diff --git a/trace-events b/trace-events
index a86c99b..d69b503 100644
--- a/trace-events
+++ b/trace-events
@@ -446,7 +446,6 @@ usb_mtp_command(int dev, uint16_t code, uint32_t trans, uint32_t arg0, uint32_t
 usb_mtp_success(int dev, uint32_t trans, uint32_t arg0, uint32_t arg1) "dev %d, trans 0x%x, args 0x%x, 0x%x"
 usb_mtp_error(int dev, uint16_t code, uint32_t trans, uint32_t arg0, uint32_t arg1) "dev %d, code 0x%x, trans 0x%x, args 0x%x, 0x%x"
 usb_mtp_data_in(int dev, uint32_t trans, uint32_t len) "dev %d, trans 0x%x, len %d"
-usb_mtp_data_out(int dev, uint32_t trans, uint32_t len) "dev %d, trans 0x%x, len %d"
 usb_mtp_xfer(int dev, uint32_t ep, uint32_t dlen, uint32_t plen) "dev %d, ep %d, %d/%d"
 usb_mtp_nak(int dev, uint32_t ep) "dev %d, ep %d"
 usb_mtp_stall(int dev, const char *reason) "dev %d, reason: %s"
-- 
1.9.3

[Qemu-devel] [PATCH 3/3] trace-events: drop orphan iscsi trace events

[Stefan Hajnoczi]

iscsi_aio_write16_cb, iscsi_aio_writev, iscsi_aio_read16_cb, and
iscsi_aio_readv have not not been in use since commit
063c3378a9e3c25cc0afac3c72e4823d0621e352 ("block/iscsi: introduce
bdrv_co_{readv, writev, flush_to_disk}").

These were the only trace events in block/iscsi.c so drop the the
trace.h include.

Cc: Peter Lieven <pl@kamp.de>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/iscsi.c | 1 -
 trace-events  | 6 ------
 2 files changed, 7 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index 84bcae8..5fc3192 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -34,7 +34,6 @@
 #include "qemu/bitops.h"
 #include "qemu/bitmap.h"
 #include "block/block_int.h"
-#include "trace.h"
 #include "block/scsi.h"
 #include "qemu/iov.h"
 #include "sysemu/sysemu.h"
diff --git a/trace-events b/trace-events
index d69b503..467439b 100644
--- a/trace-events
+++ b/trace-events
@@ -876,12 +876,6 @@ escc_sunkbd_event_out(int ch) "Translated keycode 0x%2.2x"
 escc_kbd_command(int val) "Command %d"
 escc_sunmouse_event(int dx, int dy, int buttons_state) "dx=%d dy=%d buttons=%01x"
 
-# block/iscsi.c
-iscsi_aio_write16_cb(void *iscsi, int status, void *acb, int canceled) "iscsi %p status %d acb %p canceled %d"
-iscsi_aio_writev(void *iscsi, int64_t sector_num, int nb_sectors, void *opaque, void *acb) "iscsi %p sector_num %"PRId64" nb_sectors %d opaque %p acb %p"
-iscsi_aio_read16_cb(void *iscsi, int status, void *acb, int canceled) "iscsi %p status %d acb %p canceled %d"
-iscsi_aio_readv(void *iscsi, int64_t sector_num, int nb_sectors, void *opaque, void *acb) "iscsi %p sector_num %"PRId64" nb_sectors %d opaque %p acb %p"
-
 # hw/scsi/esp.c
 esp_error_fifo_overrun(void) "FIFO overrun"
 esp_error_unhandled_command(uint32_t val) "unhandled command (%2.2x)"
-- 
1.9.3

Re: [Qemu-devel] [PATCH 0/3] trace: drop orphan events from ./trace-events

[Markus Armbruster]

Stefan Hajnoczi <stefanha@redhat.com> writes:

> Over time a few unused trace events have been left behind in ./trace-events.
> Either the code that called them was deleted or the event was never called in
> the first place.
>
> This is a clear violation of the solider's motto "no man left behind".  It's
> time to bring these trace events home.
>
> On a more serious note, unused trace events cause confusion to users who want
> to enable existing instrumentation.  On the SystemTap backend we get errors
> when attempting to enable them because the static probes that these events are
> based on do not exist.

Reviewed-by: Markus Armbruster <armbru@redhat.com>

While you're at it, could you also clean up the pointers to source files
that have bit-rotted?


$ scripts/cleanup-trace-events.pl trace-events | diff -u trace-events -
--- trace-events	2014-09-22 17:40:04.228700732 +0200
+++ -	2014-09-22 17:41:03.107310697 +0200
@@ -836,7 +836,7 @@
 pvscsi_tx_rings_ppn(const char* label, uint64_t ppn) "%s page: %"PRIx64""
 pvscsi_tx_rings_num_pages(const char* label, uint32_t num) "Number of %s pages: %u"
 
-# xen-all.c
+# xen-hvm.c
 xen_ram_alloc(unsigned long ram_addr, unsigned long size) "requested: %#lx, size %#lx"
 xen_client_set_memory(uint64_t start_addr, unsigned long size, bool log_dirty) "%#"PRIx64" size %#lx, log_dirty %i"
 
@@ -845,7 +845,7 @@
 xen_remap_bucket(uint64_t index) "index %#"PRIx64
 xen_map_cache_return(void* ptr) "%p"
 
-# hw/xen/xen_platform.c
+# hw/i386/xen/xen_platform.c
 xen_platform_log(char *s) "xen platform: %s"
 
 # qemu-coroutine.c
@@ -1075,6 +1075,7 @@
 vmware_scratch_write(uint32_t index, uint32_t value) "index %d, value 0x%x"
 vmware_setmode(uint32_t w, uint32_t h, uint32_t bpp) "%dx%d @ %d bpp"
 
+# vmstate.c
 # savevm.c
 savevm_section_start(const char *id, unsigned int section_id) "%s, section_id %u"
 savevm_section_end(const char *id, unsigned int section_id) "%s, section_id %u"
@@ -1241,7 +1242,7 @@
 virtio_ccw_interpret_ccw(int cssid, int ssid, int schid, int cmd_code) "VIRTIO-CCW: %x.%x.%04x: interpret command %x"
 virtio_ccw_new_device(int cssid, int ssid, int schid, int devno, const char *devno_mode) "VIRTIO-CCW: add subchannel %x.%x.%04x, devno %04x (%s)"
 
-# hw/intc/s390_flic.c
+# hw/intc/s390_flic_kvm.c
 flic_create_device(int err) "flic: create device failed %d"
 flic_no_device_api(int err) "flic: no Device Contral API support %d"
 flic_reset_failed(int err) "flic: reset failed %d"
@@ -1254,6 +1255,7 @@
 migrate_pending(uint64_t size, uint64_t max) "pending size %" PRIu64 " max %" PRIu64
 migrate_transferred(uint64_t tranferred, uint64_t time_spent, double bandwidth, uint64_t size) "transferred %" PRIu64 " time_spent %" PRIu64 " bandwidth %g max_size %" PRId64
 
+# target-ppc/kvm.c
 # kvm-all.c
 kvm_ioctl(int type, void *arg) "type 0x%x, arg %p"
 kvm_vm_ioctl(int type, void *arg) "type 0x%x, arg %p"
@@ -1282,10 +1284,12 @@
 object_dynamic_cast_assert(const char *type, const char *target, const char *file, int line, const char *func) "%s->%s (%s:%d:%s)"
 object_class_dynamic_cast_assert(const char *type, const char *target, const char *file, int line, const char *func) "%s->%s (%s:%d:%s)"
 
-# hw/xen/xen_pvdevice.c
+# hw/i386/xen/xen_pvdevice.c
 xen_pv_mmio_read(uint64_t addr) "WARNING: read from Xen PV Device MMIO space (address %"PRIx64")"
 xen_pv_mmio_write(uint64_t addr) "WARNING: write to Xen PV Device MMIO space (address %"PRIx64")"
 
+# hw/i386/pc.c
+# hw/acpi/memory_hotplug.c
 # hw/pci/pci_host.c
 pci_cfg_read(const char *dev, unsigned devid, unsigned fnid, unsigned offs, unsigned val) "%s %02u:%u @0x%x -> 0x%x"
 pci_cfg_write(const char *dev, unsigned devid, unsigned fnid, unsigned offs, unsigned val) "%s %02u:%u @0x%x <- 0x%x"

Re: [Qemu-devel] [PATCH 0/3] trace: drop orphan events from ./trace-events

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 4136 bytes --]

On Mon, Sep 22, 2014 at 05:45:15PM +0200, Markus Armbruster wrote:
> Stefan Hajnoczi <stefanha@redhat.com> writes:
> 
> > Over time a few unused trace events have been left behind in ./trace-events.
> > Either the code that called them was deleted or the event was never called in
> > the first place.
> >
> > This is a clear violation of the solider's motto "no man left behind".  It's
> > time to bring these trace events home.
> >
> > On a more serious note, unused trace events cause confusion to users who want
> > to enable existing instrumentation.  On the SystemTap backend we get errors
> > when attempting to enable them because the static probes that these events are
> > based on do not exist.
> 
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> 
> While you're at it, could you also clean up the pointers to source files
> that have bit-rotted?

Sure, please post a separate patch with your Signed-off-by and I'll
merge it.

Stefan

> $ scripts/cleanup-trace-events.pl trace-events | diff -u trace-events -
> --- trace-events	2014-09-22 17:40:04.228700732 +0200
> +++ -	2014-09-22 17:41:03.107310697 +0200
> @@ -836,7 +836,7 @@
>  pvscsi_tx_rings_ppn(const char* label, uint64_t ppn) "%s page: %"PRIx64""
>  pvscsi_tx_rings_num_pages(const char* label, uint32_t num) "Number of %s pages: %u"
>  
> -# xen-all.c
> +# xen-hvm.c
>  xen_ram_alloc(unsigned long ram_addr, unsigned long size) "requested: %#lx, size %#lx"
>  xen_client_set_memory(uint64_t start_addr, unsigned long size, bool log_dirty) "%#"PRIx64" size %#lx, log_dirty %i"
>  
> @@ -845,7 +845,7 @@
>  xen_remap_bucket(uint64_t index) "index %#"PRIx64
>  xen_map_cache_return(void* ptr) "%p"
>  
> -# hw/xen/xen_platform.c
> +# hw/i386/xen/xen_platform.c
>  xen_platform_log(char *s) "xen platform: %s"
>  
>  # qemu-coroutine.c
> @@ -1075,6 +1075,7 @@
>  vmware_scratch_write(uint32_t index, uint32_t value) "index %d, value 0x%x"
>  vmware_setmode(uint32_t w, uint32_t h, uint32_t bpp) "%dx%d @ %d bpp"
>  
> +# vmstate.c
>  # savevm.c
>  savevm_section_start(const char *id, unsigned int section_id) "%s, section_id %u"
>  savevm_section_end(const char *id, unsigned int section_id) "%s, section_id %u"
> @@ -1241,7 +1242,7 @@
>  virtio_ccw_interpret_ccw(int cssid, int ssid, int schid, int cmd_code) "VIRTIO-CCW: %x.%x.%04x: interpret command %x"
>  virtio_ccw_new_device(int cssid, int ssid, int schid, int devno, const char *devno_mode) "VIRTIO-CCW: add subchannel %x.%x.%04x, devno %04x (%s)"
>  
> -# hw/intc/s390_flic.c
> +# hw/intc/s390_flic_kvm.c
>  flic_create_device(int err) "flic: create device failed %d"
>  flic_no_device_api(int err) "flic: no Device Contral API support %d"
>  flic_reset_failed(int err) "flic: reset failed %d"
> @@ -1254,6 +1255,7 @@
>  migrate_pending(uint64_t size, uint64_t max) "pending size %" PRIu64 " max %" PRIu64
>  migrate_transferred(uint64_t tranferred, uint64_t time_spent, double bandwidth, uint64_t size) "transferred %" PRIu64 " time_spent %" PRIu64 " bandwidth %g max_size %" PRId64
>  
> +# target-ppc/kvm.c
>  # kvm-all.c
>  kvm_ioctl(int type, void *arg) "type 0x%x, arg %p"
>  kvm_vm_ioctl(int type, void *arg) "type 0x%x, arg %p"
> @@ -1282,10 +1284,12 @@
>  object_dynamic_cast_assert(const char *type, const char *target, const char *file, int line, const char *func) "%s->%s (%s:%d:%s)"
>  object_class_dynamic_cast_assert(const char *type, const char *target, const char *file, int line, const char *func) "%s->%s (%s:%d:%s)"
>  
> -# hw/xen/xen_pvdevice.c
> +# hw/i386/xen/xen_pvdevice.c
>  xen_pv_mmio_read(uint64_t addr) "WARNING: read from Xen PV Device MMIO space (address %"PRIx64")"
>  xen_pv_mmio_write(uint64_t addr) "WARNING: write to Xen PV Device MMIO space (address %"PRIx64")"
>  
> +# hw/i386/pc.c
> +# hw/acpi/memory_hotplug.c
>  # hw/pci/pci_host.c
>  pci_cfg_read(const char *dev, unsigned devid, unsigned fnid, unsigned offs, unsigned val) "%s %02u:%u @0x%x -> 0x%x"
>  pci_cfg_write(const char *dev, unsigned devid, unsigned fnid, unsigned offs, unsigned val) "%s %02u:%u @0x%x <- 0x%x"

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH 0/3] trace: drop orphan events from ./trace-events

[Markus Armbruster]

Stefan Hajnoczi <stefanha@redhat.com> writes:

> On Mon, Sep 22, 2014 at 05:45:15PM +0200, Markus Armbruster wrote:
>> Stefan Hajnoczi <stefanha@redhat.com> writes:
>> 
>> > Over time a few unused trace events have been left behind in ./trace-events.
>> > Either the code that called them was deleted or the event was
>> > never called in
>> > the first place.
>> >
>> > This is a clear violation of the solider's motto "no man left behind".  It's
>> > time to bring these trace events home.
>> >
>> > On a more serious note, unused trace events cause confusion to
>> > users who want
>> > to enable existing instrumentation.  On the SystemTap backend we get errors
>> > when attempting to enable them because the static probes that
>> > these events are
>> > based on do not exist.
>> 
>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>> 
>> While you're at it, could you also clean up the pointers to source files
>> that have bit-rotted?
>
> Sure, please post a separate patch with your Signed-off-by and I'll
> merge it.

Done.  Turned into a mini-series.

Re: [Qemu-devel] [PATCH 0/3] trace: drop orphan events from ./trace-events

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1055 bytes --]

On Mon, Sep 22, 2014 at 03:03:12PM +0100, Stefan Hajnoczi wrote:
> Over time a few unused trace events have been left behind in ./trace-events.
> Either the code that called them was deleted or the event was never called in
> the first place.
> 
> This is a clear violation of the solider's motto "no man left behind".  It's
> time to bring these trace events home.
> 
> On a more serious note, unused trace events cause confusion to users who want
> to enable existing instrumentation.  On the SystemTap backend we get errors
> when attempting to enable them because the static probes that these events are
> based on do not exist.
> 
> Stefan Hajnoczi (3):
>   trace-events: drop orphan virtio_blk_data_plane_complete_request
>   trace-events: drop orphan usb_mtp_data_out
>   trace-events: drop orphan iscsi trace events
> 
>  block/iscsi.c | 1 -
>  trace-events  | 8 --------
>  2 files changed, 9 deletions(-)
> 
> -- 
> 1.9.3
> 
> 

Applied to my tracing tree:
https://github.com/stefanha/qemu/commits/tracing

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]