From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42586) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ag9IB-0005Wy-Lr for qemu-devel@nongnu.org; Wed, 16 Mar 2016 07:07:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ag9I8-00009Y-Td for qemu-devel@nongnu.org; Wed, 16 Mar 2016 07:07:07 -0400 Received: from mail-db5eur01on0116.outbound.protection.outlook.com ([104.47.2.116]:56096 helo=EUR01-DB5-obe.outbound.protection.outlook.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ag9I8-00005i-6B for qemu-devel@nongnu.org; Wed, 16 Mar 2016 07:07:04 -0400 References: <1457077710-17678-1-git-send-email-mst@redhat.com> <1457077710-17678-9-git-send-email-mst@redhat.com> From: "Denis V. Lunev" Message-ID: <56E9367B.8070305@openvz.org> Date: Wed, 16 Mar 2016 13:33:31 +0300 MIME-Version: 1.0 In-Reply-To: <1457077710-17678-9-git-send-email-mst@redhat.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PULL 08/16] virtio-balloon: export all balloon statistics List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" , qemu-devel@nongnu.org Cc: "Denis V . Lunev" , Eduardo Habkost , Igor Redko , Peter Maydell On 03/04/2016 10:49 AM, Michael S. Tsirkin 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 > Reviewed-by: Michael S. Tsirkin > Signed-off-by: Michael S. Tsirkin > --- > configure | 12 ++++++++++++ > include/hw/virtio/virtio-balloon.h | 3 ++- > hw/virtio/virtio-balloon.c | 32 ++++++++++++++++++++++++++------ > 3 files changed, 40 insertions(+), 7 deletions(-) > > diff --git a/configure b/configure > index 0c0472a..767d96e 100755 > --- a/configure > +++ b/configure > @@ -315,6 +315,7 @@ vhdx="" > numa="" > tcmalloc="no" > jemalloc="no" > +unknown_balloon_stats="no" > > # parse CC options first > for opt do > @@ -1142,6 +1143,10 @@ for opt do > ;; > --enable-jemalloc) jemalloc="yes" > ;; > + --enable-unknown-balloon-stats) unknown_balloon_stats="yes" > + ;; > + --disable-unknown-balloon-stats) unknown_balloon_stats="no" > + ;; > *) > echo "ERROR: unknown option $opt" > echo "Try '$0 --help' for more information" > @@ -1364,6 +1369,8 @@ disabled with --disable-FEATURE, default is enabled if available: > numa libnuma support > tcmalloc tcmalloc support > jemalloc jemalloc support > + unknown-balloon-stats report unknown balloon statistics counters > + ;; > > NOTE: The object files are built at the place where configure is launched > EOF > @@ -4790,6 +4797,7 @@ echo "bzip2 support $bzip2" > echo "NUMA host support $numa" > echo "tcmalloc support $tcmalloc" > echo "jemalloc support $jemalloc" > +echo "unknown balloon stat counters support $unknown_balloon_stats" > > if test "$sdl_too_old" = "yes"; then > echo "-> Your SDL version is too old - please upgrade to have SDL support" > @@ -5342,6 +5350,10 @@ if test "$rdma" = "yes" ; then > echo "CONFIG_RDMA=y" >> $config_host_mak > fi > > +if test "$unknown_balloon_stats" = "yes" ; then > + echo "CONFIG_UNKNOWN_BALLOON_STATS=y" >> $config_host_mak > +fi > + > # Hold two types of flag: > # CONFIG_THREAD_SETNAME_BYTHREAD - we've got a way of setting the name on > # a thread we have a handle to > 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; > diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c > index e97d403..64367ac 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,22 @@ 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 { > +#if defined(CONFIG_UNKNOWN_BALLOON_STATS) > + 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); > +#endif > + } > if (err) { > break; > } > } > + > error_propagate(errp, err); > err = NULL; > visit_end_struct(v, &err); > @@ -282,10 +291,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; > Michael, what has happened with this patch? I have seen pull request and this patch was coupled with 'available' modification. 'available' code was resent 11.03 and this one not. Den