Patchwork [7/7] trace: Trace virtqueue operations

login
register
mail settings
Submitter Stefan Hajnoczi
Date May 25, 2010, 10:24 a.m.
Message ID <1274783056-14759-8-git-send-email-stefanha@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/53527/
State New
Headers show

Comments

Stefan Hajnoczi - May 25, 2010, 10:24 a.m.
This patch adds trace events for virtqueue operations including
adding/removing buffers, notifying the guest, and receiving a notify
from the guest.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
v2:
 * This patch is new in v2

 hw/virtio.c  |    8 ++++++++
 trace-events |    8 ++++++++
 2 files changed, 16 insertions(+), 0 deletions(-)
Avi Kivity - May 25, 2010, 12:04 p.m.
On 05/25/2010 01:24 PM, Stefan Hajnoczi wrote:
> This patch adds trace events for virtqueue operations including
> adding/removing buffers, notifying the guest, and receiving a notify
> from the guest.
>
> diff --git a/trace-events b/trace-events
> index 48415f8..a533414 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -35,6 +35,14 @@ qemu_memalign(size_t alignment, size_t size, void *ptr) "alignment %zu size %zu
>   qemu_valloc(size_t size, void *ptr) "size %zu ptr %p"
>   qemu_vfree(void *ptr) "ptr %p"
>
> +# hw/virtio.c
> +virtqueue_fill(void *vq, const void *elem, unsigned int len, unsigned int idx) "vq %p elem %p len %u idx %u"
> +virtqueue_flush(void *vq, unsigned int count) "vq %p count %u"
> +virtqueue_pop(void *vq, void *elem, unsigned int in_num, unsigned int out_num) "vq %p elem %p in_num %u out_num %u"
> +virtio_queue_notify(void *vdev, int n, void *vq) "vdev %p n %d vq %p"
> +virtio_irq(void *vq) "vq %p"
> +virtio_notify(void *vdev, void *vq) "vdev %p vq %p"
> +
>    


Those %ps are more or less useless.  We need better ways of identifying 
them.

Linux uses %pTYPE to pretty print arbitrary types.  We could do 
something similar (not the same since we don't want our own printf 
implementation).
Stefan Hajnoczi - May 25, 2010, 1:27 p.m.
On Tue, May 25, 2010 at 1:04 PM, Avi Kivity <avi@redhat.com> wrote:
> Those %ps are more or less useless.  We need better ways of identifying
> them.

You're right, the vq pointer is useless in isolation.  We don't know
which virtio device or which virtqueue number.

With the full context of a trace it would be possible to correlate the
vq pointer if we had trace events for vdev and vq setup.

Adding custom formatters is could be tricky since the format string is
passed only to tracing backends that use it, like UST.  And UST uses
its own sprintf implementation which we don't have direct control
over.

I think we just need to guarantee that any pointer can be correlated
with previous trace entries that give context for that pointer.

Stefan
Avi Kivity - May 25, 2010, 1:52 p.m.
On 05/25/2010 04:27 PM, Stefan Hajnoczi wrote:
> On Tue, May 25, 2010 at 1:04 PM, Avi Kivity<avi@redhat.com>  wrote:
>    
>> Those %ps are more or less useless.  We need better ways of identifying
>> them.
>>      
> You're right, the vq pointer is useless in isolation.  We don't know
> which virtio device or which virtqueue number.
>
> With the full context of a trace it would be possible to correlate the
> vq pointer if we had trace events for vdev and vq setup.
>
> Adding custom formatters is could be tricky since the format string is
> passed only to tracing backends that use it, like UST.  And UST uses
> its own sprintf implementation which we don't have direct control
> over.
>    

Hm.  Perhaps we can convert %{type} to %p for backends which don't 
support it, and to whatever format they do support for those that do.
Stefan Hajnoczi - May 25, 2010, 2 p.m.
On Tue, May 25, 2010 at 2:52 PM, Avi Kivity <avi@redhat.com> wrote:
> Hm.  Perhaps we can convert %{type} to %p for backends which don't support
> it, and to whatever format they do support for those that do.

True.

Stefan

Patch

diff --git a/hw/virtio.c b/hw/virtio.c
index 4475bb3..a5741ae 100644
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -13,6 +13,7 @@ 
 
 #include <inttypes.h>
 
+#include "trace.h"
 #include "virtio.h"
 #include "sysemu.h"
 
@@ -205,6 +206,8 @@  void virtqueue_fill(VirtQueue *vq, const VirtQueueElement *elem,
     unsigned int offset;
     int i;
 
+    trace_virtqueue_fill(vq, elem, len, idx);
+
     offset = 0;
     for (i = 0; i < elem->in_num; i++) {
         size_t size = MIN(len - offset, elem->in_sg[i].iov_len);
@@ -232,6 +235,7 @@  void virtqueue_flush(VirtQueue *vq, unsigned int count)
 {
     /* Make sure buffer is written before we update index. */
     wmb();
+    trace_virtqueue_flush(vq, count);
     vring_used_idx_increment(vq, count);
     vq->inuse -= count;
 }
@@ -422,6 +426,7 @@  int virtqueue_pop(VirtQueue *vq, VirtQueueElement *elem)
 
     vq->inuse++;
 
+    trace_virtqueue_pop(vq, elem, elem->in_num, elem->out_num);
     return elem->in_num + elem->out_num;
 }
 
@@ -560,6 +565,7 @@  int virtio_queue_get_num(VirtIODevice *vdev, int n)
 void virtio_queue_notify(VirtIODevice *vdev, int n)
 {
     if (n < VIRTIO_PCI_QUEUE_MAX && vdev->vq[n].vring.desc) {
+        trace_virtio_queue_notify(vdev, n, &vdev->vq[n]);
         vdev->vq[n].handle_output(vdev, &vdev->vq[n]);
     }
 }
@@ -597,6 +603,7 @@  VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size,
 
 void virtio_irq(VirtQueue *vq)
 {
+    trace_virtio_irq(vq);
     vq->vdev->isr |= 0x01;
     virtio_notify_vector(vq->vdev, vq->vector);
 }
@@ -609,6 +616,7 @@  void virtio_notify(VirtIODevice *vdev, VirtQueue *vq)
          (vq->inuse || vring_avail_idx(vq) != vq->last_avail_idx)))
         return;
 
+    trace_virtio_notify(vdev, vq);
     vdev->isr |= 0x01;
     virtio_notify_vector(vdev, vq->vector);
 }
diff --git a/trace-events b/trace-events
index 48415f8..a533414 100644
--- a/trace-events
+++ b/trace-events
@@ -35,6 +35,14 @@  qemu_memalign(size_t alignment, size_t size, void *ptr) "alignment %zu size %zu
 qemu_valloc(size_t size, void *ptr) "size %zu ptr %p"
 qemu_vfree(void *ptr) "ptr %p"
 
+# hw/virtio.c
+virtqueue_fill(void *vq, const void *elem, unsigned int len, unsigned int idx) "vq %p elem %p len %u idx %u"
+virtqueue_flush(void *vq, unsigned int count) "vq %p count %u"
+virtqueue_pop(void *vq, void *elem, unsigned int in_num, unsigned int out_num) "vq %p elem %p in_num %u out_num %u"
+virtio_queue_notify(void *vdev, int n, void *vq) "vdev %p n %d vq %p"
+virtio_irq(void *vq) "vq %p"
+virtio_notify(void *vdev, void *vq) "vdev %p vq %p"
+
 # block.c
 multiwrite_cb(void *mcb, int ret) "mcb %p ret %d"
 bdrv_aio_multiwrite(void *mcb, int num_callbacks, int num_reqs) "mcb %p num_callbacks %d num_reqs %d"