| Message ID | 1274783056-14759-8-git-send-email-stefanha@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
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).
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
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.
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
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"
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(-)