[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PULL 08/16] virtio-balloon: export all balloon statist
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PULL 08/16] virtio-balloon: export all balloon statistics |
Date: |
Wed, 16 Mar 2016 12:37:50 +0200 |
On Wed, Mar 16, 2016 at 01:33:31PM +0300, Denis V. Lunev wrote:
> On 03/04/2016 10:49 AM, Michael S. Tsirkin wrote:
> >From: Igor Redko <address@hidden>
> >
> >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 <address@hidden>
> >Signed-off-by: Denis V. Lunev <address@hidden>
> >CC: Michael S. Tsirkin <address@hidden>
> >Reviewed-by: Michael S. Tsirkin <address@hidden>
> >Signed-off-by: Michael S. Tsirkin <address@hidden>
> >---
> > 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
Yes - I included it by mistake, and dropped in v2.
Let's discuss after 2.6.
--
MST
- [Qemu-devel] [PULL 00/16] vhost, virtio, pci, pc, acpi, Michael S. Tsirkin, 2016/03/04
- [Qemu-devel] [PULL 01/16] acpi: add aml_create_field(), Michael S. Tsirkin, 2016/03/04
- [Qemu-devel] [PULL 02/16] acpi: add aml_concatenate(), Michael S. Tsirkin, 2016/03/04
- [Qemu-devel] [PULL 03/16] acpi: allow using object as offset for OperationRegion, Michael S. Tsirkin, 2016/03/04
- [Qemu-devel] [PULL 04/16] acpi: add build_append_named_dword, returning an offset in buffer, Michael S. Tsirkin, 2016/03/04
- [Qemu-devel] [PULL 05/16] balloon: fix segfault and harden the stats queue, Michael S. Tsirkin, 2016/03/04
- [Qemu-devel] [PULL 06/16] hw/virtio: fix double use of a virtio flag, Michael S. Tsirkin, 2016/03/04
- [Qemu-devel] [PULL 07/16] hw/virtio: group virtio flags into an enum, Michael S. Tsirkin, 2016/03/04
- [Qemu-devel] [PULL 08/16] virtio-balloon: export all balloon statistics, Michael S. Tsirkin, 2016/03/04
- [Qemu-devel] [PULL 09/16] virtio-balloon: add 'available' counter, Michael S. Tsirkin, 2016/03/04
- [Qemu-devel] [PULL 10/16] vhost-user: verify that number of queues is less than MAX_QUEUE_NUM, Michael S. Tsirkin, 2016/03/04
- [Qemu-devel] [PULL 11/16] pc-dimm: fix error handling in pc_dimm_check_memdev_is_busy(), Michael S. Tsirkin, 2016/03/04
- [Qemu-devel] [PULL 12/16] i386/acpi: make floppy controller object dynamic, Michael S. Tsirkin, 2016/03/04
- [Qemu-devel] [PULL 13/16] i386: expose floppy drive CMOS type, Michael S. Tsirkin, 2016/03/04
- [Qemu-devel] [PULL 15/16] i386: populate floppy drive information in DSDT, Michael S. Tsirkin, 2016/03/04
- [Qemu-devel] [PULL 14/16] fdc: add function to determine drive chs limits, Michael S. Tsirkin, 2016/03/04
- [Qemu-devel] [PULL 16/16] i386: update expected DSDT, Michael S. Tsirkin, 2016/03/04
- Re: [Qemu-devel] [PULL 00/16] vhost, virtio, pci, pc, acpi, Fam Zheng, 2016/03/04