From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33436) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b1X9T-0002oP-4V for qemu-devel@nongnu.org; Sat, 14 May 2016 06:50:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b1X9P-0002p2-G8 for qemu-devel@nongnu.org; Sat, 14 May 2016 06:50:30 -0400 Received: from mx1.parallels.com ([199.115.104.18]:37821) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b1X9P-0002Q3-7T for qemu-devel@nongnu.org; Sat, 14 May 2016 06:50:27 -0400 References: <1457077710-17678-1-git-send-email-mst@redhat.com> <1457077710-17678-9-git-send-email-mst@redhat.com> <56E9367B.8070305@openvz.org> <20160316123547-mutt-send-email-mst@redhat.com> From: "Denis V. Lunev" Message-ID: <573702B0.7090201@virtuozzo.com> Date: Sat, 14 May 2016 13:49:20 +0300 MIME-Version: 1.0 In-Reply-To: <20160316123547-mutt-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" , "Denis V. Lunev" Cc: Peter Maydell , qemu-devel@nongnu.org, Igor Redko , Eduardo Habkost On 03/16/2016 01:37 PM, Michael S. Tsirkin wrote: > 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 >>> >>> 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 > Yes - I included it by mistake, and dropped in v2. > Let's discuss after 2.6. > how could we start the discussion?