From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49685) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aYEoy-0006v4-Uf for qemu-devel@nongnu.org; Tue, 23 Feb 2016 10:24:18 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aYEov-0000P5-M4 for qemu-devel@nongnu.org; Tue, 23 Feb 2016 10:24:16 -0500 Received: from mx1.redhat.com ([209.132.183.28]:48490) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aYEov-0000P0-E5 for qemu-devel@nongnu.org; Tue, 23 Feb 2016 10:24:13 -0500 Date: Tue, 23 Feb 2016 17:24:10 +0200 From: "Michael S. Tsirkin" Message-ID: <20160223172048-mutt-send-email-mst@redhat.com> References: <1456239585-13324-1-git-send-email-den@openvz.org> <1456239585-13324-2-git-send-email-den@openvz.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1456239585-13324-2-git-send-email-den@openvz.org> Subject: Re: [Qemu-devel] [PATCH 1/2] virtio-balloon: export all balloon statistics List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Denis V. Lunev" Cc: Igor Redko , qemu-devel@nongnu.org On Tue, Feb 23, 2016 at 05:59:44PM +0300, Denis V. Lunev wrote: > From: Igor Redko > > We are making experiments with different autoballooning strategies > based on the guest behavior. Thus we need to experiment with different > guest statistics. For now every counter change requires QEMU recompilation > and dances with Libvirt. > > This patch introduces transport for unrecognized counters in virtio-balloon. > This transport can be used for measuring benefits from using new > balloon counters, before submitting any patches. Current alternative > is 'guest-exec' transport which isn't made for such delicate matters > and can influence test results. > > Originally all counters with tag >= VIRTIO_BALLOON_S_NR were ignored. > Instead of this we keep first (VIRTIO_BALLOON_S_NR + 32) counters from the > queue and pass unrecognized ones with the following names: 'x-stat-XXXX', > where XXXX is a tag number in hex. Defined counters are reported with their > regular names. > > Signed-off-by: Igor Redko > Signed-off-by: Denis V. Lunev > CC: Michael S. Tsirkin This seems to open the ABI to abuse. Seems like a reasonable way to experiment though. How about adding this within #if 0 statements? You can uncomment them for debugging ... > --- > hw/virtio/virtio-balloon.c | 30 ++++++++++++++++++++++++------ > include/hw/virtio/virtio-balloon.h | 3 ++- > 2 files changed, 26 insertions(+), 7 deletions(-) > > diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c > index a382f43..1740293 100644 > --- a/hw/virtio/virtio-balloon.c > +++ b/hw/virtio/virtio-balloon.c > @@ -66,8 +66,7 @@ static const char *balloon_stat_names[] = { > */ > static inline void reset_stats(VirtIOBalloon *dev) > { > - int i; > - for (i = 0; i < VIRTIO_BALLOON_S_NR; dev->stats[i++] = -1); > + dev->stats_cnt = 0; > } > > static bool balloon_stats_supported(const VirtIOBalloon *s) > @@ -133,12 +132,20 @@ static void balloon_stats_get_all(Object *obj, Visitor *v, const char *name, > if (err) { > goto out_end; > } > - for (i = 0; i < VIRTIO_BALLOON_S_NR; i++) { > - visit_type_uint64(v, balloon_stat_names[i], &s->stats[i], &err); > + for (i = 0; i < s->stats_cnt; i++) { > + if (s->stats[i].tag < VIRTIO_BALLOON_S_NR) { > + visit_type_uint64(v, balloon_stat_names[s->stats[i].tag], > + &s->stats[i].val, &err); > + } else { > + gchar *str = g_strdup_printf("x-stat-%04x", s->stats[i].tag); > + visit_type_uint64(v, str, &s->stats[i].val, &err); > + g_free(str); > + } > if (err) { > break; > } > } > + > error_propagate(errp, err); > err = NULL; > visit_end_struct(v, &err); > @@ -273,10 +280,21 @@ static void virtio_balloon_receive_stats(VirtIODevice *vdev, VirtQueue *vq) > == sizeof(stat)) { > uint16_t tag = virtio_tswap16(vdev, stat.tag); > uint64_t val = virtio_tswap64(vdev, stat.val); > + int i; > > offset += sizeof(stat); > - if (tag < VIRTIO_BALLOON_S_NR) > - s->stats[tag] = val; > + for (i = 0; i < s->stats_cnt; i++) { > + if (s->stats[i].tag == tag) { > + break; > + } > + } > + if (i < ARRAY_SIZE(s->stats)) { > + s->stats[i].tag = tag; > + s->stats[i].val = val; > + if (s->stats_cnt <= i) { > + s->stats_cnt = i + 1; > + } > + } > } > s->stats_vq_offset = offset; > > diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h > index 35f62ac..5c8730e 100644 > --- a/include/hw/virtio/virtio-balloon.h > +++ b/include/hw/virtio/virtio-balloon.h > @@ -36,7 +36,8 @@ typedef struct VirtIOBalloon { > VirtQueue *ivq, *dvq, *svq; > uint32_t num_pages; > uint32_t actual; > - uint64_t stats[VIRTIO_BALLOON_S_NR]; > + VirtIOBalloonStatModern stats[VIRTIO_BALLOON_S_NR + 32]; > + uint16_t stats_cnt; > VirtQueueElement *stats_vq_elem; > size_t stats_vq_offset; > QEMUTimer *stats_timer; > -- > 2.1.4