From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:58936) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QwcKG-0002Re-D1 for qemu-devel@nongnu.org; Thu, 25 Aug 2011 11:58:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QwcKF-0003Yn-5N for qemu-devel@nongnu.org; Thu, 25 Aug 2011 11:58:40 -0400 Received: from mail-gx0-f173.google.com ([209.85.161.173]:42208) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QwcKE-0003Yd-SF for qemu-devel@nongnu.org; Thu, 25 Aug 2011 11:58:39 -0400 Received: by gxk26 with SMTP id 26so2118433gxk.4 for ; Thu, 25 Aug 2011 08:58:38 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <1314284817-9034-2-git-send-email-kraxel@redhat.com> References: <1314284817-9034-1-git-send-email-kraxel@redhat.com> <1314284817-9034-2-git-send-email-kraxel@redhat.com> Date: Thu, 25 Aug 2011 16:58:37 +0100 Message-ID: From: Stefan Hajnoczi Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 1/7] usb-host: start tracing support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gerd Hoffmann Cc: qemu-devel@nongnu.org On Thu, Aug 25, 2011 at 4:06 PM, Gerd Hoffmann wrote: > Add a bunch of trace points to usb-linux.c =A0Drop a bunch of DPRINTK's i= n > favor of the trace points. =A0Also cleanup error reporting a bit while be= ing > at it. > > Signed-off-by: Gerd Hoffmann > --- > =A0trace-events | =A0 18 ++++++++++++++ > =A0usb-linux.c =A0| =A0 73 ++++++++++++++++++++++++++++++++++++++--------= ------------ > =A02 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 > =A0disable usb_clear_device_feature(int addr, int feature, int ret) "dev = %d, feature %d, ret %d" > =A0disable 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 len= gth, 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