From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=38966 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OFZQn-00051t-NP for qemu-devel@nongnu.org; Fri, 21 May 2010 17:07:00 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OFZQZ-0007e2-Ru for qemu-devel@nongnu.org; Fri, 21 May 2010 17:06:46 -0400 Received: from mail-vw0-f45.google.com ([209.85.212.45]:36470) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OFZQZ-0007do-Kr for qemu-devel@nongnu.org; Fri, 21 May 2010 17:06:43 -0400 Received: by vws1 with SMTP id 1so1213990vws.4 for ; Fri, 21 May 2010 14:06:43 -0700 (PDT) Message-ID: <4BF6F5DF.2060703@codemonkey.ws> Date: Fri, 21 May 2010 16:06:39 -0500 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 1/2] trace: Add simple tracing support References: <1274434947-2863-1-git-send-email-stefanha@linux.vnet.ibm.com> <1274434947-2863-2-git-send-email-stefanha@linux.vnet.ibm.com> <4BF67E72.5040908@codemonkey.ws> <4BF68E9B.1020405@siemens.com> <4BF692A5.7080501@codemonkey.ws> <4BF6BA3D.9000200@siemens.com> In-Reply-To: <4BF6BA3D.9000200@siemens.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jan Kiszka Cc: "qemu-devel@nongnu.org" , "kvm@vger.kernel.org" , Stefan Hajnoczi , Prerna Saxena On 05/21/2010 11:52 AM, Jan Kiszka wrote: > Anthony Liguori wrote: > >> On 05/21/2010 08:46 AM, Jan Kiszka wrote: >> >>> Anthony Liguori wrote: >>> >>> >>>> On 05/21/2010 04:42 AM, Stefan Hajnoczi wrote: >>>> >>>> >>>>> Trace events should be defined in trace.h. Events are written to >>>>> /tmp/trace.log and can be formatted using trace.py. Remember to add >>>>> events to trace.py for pretty-printing. >>>>> >>>>> Signed-off-by: Stefan Hajnoczi >>>>> --- >>>>> Makefile.objs | 2 +- >>>>> trace.c | 64 >>>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >>>>> trace.h | 9 ++++++++ >>>>> trace.py | 30 ++++++++++++++++++++++++++ >>>>> 4 files changed, 104 insertions(+), 1 deletions(-) >>>>> create mode 100644 trace.c >>>>> create mode 100644 trace.h >>>>> create mode 100755 trace.py >>>>> >>>>> diff --git a/Makefile.objs b/Makefile.objs >>>>> index acbaf22..307e989 100644 >>>>> --- a/Makefile.objs >>>>> +++ b/Makefile.objs >>>>> @@ -8,7 +8,7 @@ qobject-obj-y += qerror.o >>>>> # block-obj-y is code used by both qemu system emulation and qemu-img >>>>> >>>>> block-obj-y = cutils.o cache-utils.o qemu-malloc.o qemu-option.o >>>>> module.o >>>>> -block-obj-y += nbd.o block.o aio.o aes.o osdep.o qemu-config.o >>>>> +block-obj-y += nbd.o block.o aio.o aes.o osdep.o qemu-config.o trace.o >>>>> block-obj-$(CONFIG_POSIX) += posix-aio-compat.o >>>>> block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o >>>>> >>>>> diff --git a/trace.c b/trace.c >>>>> new file mode 100644 >>>>> index 0000000..2fec4d3 >>>>> --- /dev/null >>>>> +++ b/trace.c >>>>> @@ -0,0 +1,64 @@ >>>>> +#include >>>>> +#include >>>>> +#include "trace.h" >>>>> + >>>>> +typedef struct { >>>>> + unsigned long event; >>>>> + unsigned long x1; >>>>> + unsigned long x2; >>>>> + unsigned long x3; >>>>> + unsigned long x4; >>>>> + unsigned long x5; >>>>> +} TraceRecord; >>>>> + >>>>> +enum { >>>>> + TRACE_BUF_LEN = 64 * 1024 / sizeof(TraceRecord), >>>>> +}; >>>>> + >>>>> +static TraceRecord trace_buf[TRACE_BUF_LEN]; >>>>> +static unsigned int trace_idx; >>>>> +static FILE *trace_fp; >>>>> + >>>>> +static void trace(TraceEvent event, unsigned long x1, >>>>> + unsigned long x2, unsigned long x3, >>>>> + unsigned long x4, unsigned long x5) { >>>>> + TraceRecord *rec =&trace_buf[trace_idx]; >>>>> + rec->event = event; >>>>> + rec->x1 = x1; >>>>> + rec->x2 = x2; >>>>> + rec->x3 = x3; >>>>> + rec->x4 = x4; >>>>> + rec->x5 = x5; >>>>> + >>>>> + if (++trace_idx == TRACE_BUF_LEN) { >>>>> + trace_idx = 0; >>>>> + >>>>> + if (!trace_fp) { >>>>> + trace_fp = fopen("/tmp/trace.log", "w"); >>>>> + } >>>>> + if (trace_fp) { >>>>> + size_t result = fwrite(trace_buf, sizeof trace_buf, 1, >>>>> trace_fp); >>>>> + result = result; >>>>> + } >>>>> + } >>>>> +} >>>>> >>>>> >>>>> >>>> It is probably worth while to read trace points via the monitor or >>>> through some other mechanism. My concern would be that writing even 64k >>>> out to disk would introduce enough performance overhead mainly because >>>> it runs lock-step with the guest's VCPU. >>>> >>>> Maybe it's worth adding a thread that syncs the ring to disk if we want >>>> to write to disk? >>>> >>>> >>> That's not what QEMU should worry about. If somehow possible, let's push >>> this into the hands of a (user space) tracing framework, ideally one >>> that is already designed for such requirements. E.g. there exists quite >>> useful work in the context of LTTng (user space RCU for application >>> tracing). >>> >>> >> From what I understand, none of the current kernel approaches to >> userspace tracing have much momentum at the moment. >> >> >>> We may need simple stubs for the case that no such framework is (yet) >>> available. But effort should focus on a QEMU infrastructure to add >>> useful tracepoints to the code. Specifically when tracing over KVM, you >>> usually need information about kernel states as well, so you depend on >>> an integrated approach, not Yet Another Log File. >>> >>> >> I think the simple code that Stefan pasted gives us 95% of what we need. >> > IMHO not 95%, but it is a start. > I'm not opposed to using a framework, but I'd rather have an equivalent to kvm_stat tomorrow than wait 3 years for LTTng to not get merged. So let's have a dirt-simple tracing mechanism and focus on adding useful trace points. Then when we have a framework we can use, we can just convert the tracepoints to the new framework. Regards, Anthony Liguori > I would just like to avoid that too much efforts are spent on > re-inventing smart trace buffers, trace daemons, or trace visualization > tools. Then better pick up some semi-perfect approach (e.g. [1], it > unfortunately still seems to lack kernel integration) and drive it > according to our needs. > > Jan > > [1] http://lttng.org/ust > >