Re: [Qemu-devel] [PATCH 1/7] usb-host: start tracing support

[Stefan Hajnoczi]

On Thu, Aug 25, 2011 at 4:06 PM, Gerd Hoffmann <address@hidden> wrote:
> Add a bunch of trace points to usb-linux.c  Drop a bunch of DPRINTK's in
> favor of the trace points.  Also cleanup error reporting a bit while being
> at it.
>
> Signed-off-by: Gerd Hoffmann <address@hidden>
> ---
>  trace-events |   18 ++++++++++++++
>  usb-linux.c  |   73 
> ++++++++++++++++++++++++++++++++++++++--------------------
>  2 files changed, 66 insertions(+), 25 deletions(-)
>
> diff --git a/trace-events b/trace-events
> index 14e6f8b..1c7a624 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -243,6 +243,24 @@ disable usb_set_config(int addr, int config, int ret) 
> "dev %d, config %d, ret %d
>  disable usb_clear_device_feature(int addr, int feature, int ret) "dev %d, 
> feature %d, ret %d"
>  disable usb_set_device_feature(int addr, int feature, int ret) "dev %d, 
> feature %d, ret %d"
>
> +# usb-linux.c
> +usb_host_open(int bus, int addr, const char *state) "dev %d:%d, %s"

Please do not use strings.  It works for stderr but may not be
supported by other backends.  The simple backend only logs 64-bit
arguments and does not dereference into strings/arrays.  I can't
remember the status of dtrace or ust, but I'd ask you to avoid using a
string, if possible.

> +usb_host_disconnect(int bus, int addr) "dev %d:%d"
> +usb_host_close(int bus, int addr) "dev %d:%d"
> +usb_host_set_address(int bus, int addr, int config) "dev %d:%d, address %d"
> +usb_host_set_config(int bus, int addr, int config) "dev %d:%d, config %d"
> +usb_host_set_interface(int bus, int addr, int interface, int alt) "dev 
> %d:%d, interface %d, alt %d"
> +usb_host_claim_interfaces(int bus, int addr, int config, int nif) "dev 
> %d:%d, config %d, nif %d"
> +usb_host_release_interfaces(int bus, int addr) "dev %d:%d"
> +usb_host_req_control(int bus, int addr, int req, int value, int index) "dev 
> %d:%d, req 0x%x, value %d, index %d"
> +usb_host_req_data(int bus, int addr, const char *dir, int ep, int size) "dev 
> %d:%d, %s, ep %d, size %d"

String

> +usb_host_req_complete(int bus, int addr, int status) "dev %d:%d, status %d"
> +usb_host_urb_submit(int bus, int addr, void *aurb, int length, int more) 
> "dev %d:%d, aurb %p, length %d, more %d"
> +usb_host_urb_complete(int bus, int addr, void *aurb, int status, int length, 
> int more) "dev %d:%d, aurb %p, status %d, length %d, more %d"
> +usb_host_ep_op(int bus, int addr, int ep, const char *op) "dev %d:%d, ep %d, 
> %s"

String

> +usb_host_reset(int bus, int addr) "dev %d:%d"
> +usb_host_auto_scan(const char *state) "%s"

String

The rest looks fine.  In some cases you can simply split the trace
event into multiple independent events which contains the
status/operation/etc value that you're trying to pretty-print.

Stefan