From mboxrd@z Thu Jan 1 00:00:00 1970 From: Cornelia Huck Subject: Re: [PATCH 1/1] KVM: halt_polling: provide a way to qualify wakeups during poll Date: Wed, 4 May 2016 08:22:23 +0200 Message-ID: <20160504082223.2a0a1ee0.cornelia.huck@de.ibm.com> References: <1462279041-17028-1-git-send-email-borntraeger@de.ibm.com> <1462279041-17028-2-git-send-email-borntraeger@de.ibm.com> <20160503145612.58d8da82.cornelia.huck@de.ibm.com> <20160503150356.GE30059@potion> <5728E9F0.1090705@de.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Radim =?UTF-8?B?S3LEjW3DocWZ?= , Paolo Bonzini , KVM , linux-s390 , Jens Freimann , David Hildenbrand , Wanpeng Li , David Matlack To: Christian Borntraeger Return-path: Received: from e06smtp07.uk.ibm.com ([195.75.94.103]:50870 "EHLO e06smtp07.uk.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757119AbcEDGWc convert rfc822-to-8bit (ORCPT ); Wed, 4 May 2016 02:22:32 -0400 Received: from localhost by e06smtp07.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 4 May 2016 07:22:30 +0100 In-Reply-To: <5728E9F0.1090705@de.ibm.com> Sender: kvm-owner@vger.kernel.org List-ID: On Tue, 3 May 2016 20:12:00 +0200 Christian Borntraeger wrote: > On 05/03/2016 05:03 PM, Radim Kr=C4=8Dm=C3=A1=C5=99 wrote: > > 2016-05-03 14:56+0200, Cornelia Huck: > >> On Tue, 3 May 2016 14:37:21 +0200 > >> Christian Borntraeger wrote: > >>> diff --git a/include/trace/events/kvm.h b/include/trace/events/kv= m.h > >>> index aa69253..92e6fd6 100644 > >>> --- a/include/trace/events/kvm.h > >>> +++ b/include/trace/events/kvm.h > >>> @@ -38,22 +38,25 @@ TRACE_EVENT(kvm_userspace_exit, > >>> ); > >>> > >>> TRACE_EVENT(kvm_vcpu_wakeup, > >>> - TP_PROTO(__u64 ns, bool waited), > >>> - TP_ARGS(ns, waited), > >>> + TP_PROTO(__u64 ns, bool waited, bool tuned), > >>> + TP_ARGS(ns, waited, tuned), > >>> > >>> TP_STRUCT__entry( > >>> __field( __u64, ns ) > >>> __field( bool, waited ) > >>> + __field( bool, tuned ) > >>> ), > >>> > >>> TP_fast_assign( > >>> __entry->ns =3D ns; > >>> __entry->waited =3D waited; > >>> + __entry->tuned =3D tuned; > >>> ), > >>> > >>> - TP_printk("%s time %lld ns", > >>> + TP_printk("%s time %lld ns, polling %s", > >>> __entry->waited ? "wait" : "poll", > >>> - __entry->ns) > >>> + __entry->ns, > >>> + __entry->tuned ? "changed" : "unchanged") > >> > >> I think "changed"/"unchanged" is a bit misleading here, as we do a= djust > >> the intervall if we had an invalid poll... but it's hard to find a > >> suitable text here. > >> > >> Just print "poll interval tuned" if we were (a) polling to begin w= ith, > >> (b) the poll was valid and (c) the interval was actually changed a= nd > >> print "invalid poll" if that's what happened? Or is that overkill? > >=20 > > Just renaming to valid/invalid is fine, IMO, the state of polling i= s > > static and interval change can be read from other traces. > >=20 > > I think that having "no_tuning" counter, "unchanged" trace and "inv= alid" > > in source names obscures the logical connection; doesn't "invalid"= fit > > them all? > >=20 >=20 > Yes, will change tracing into=20 > __entry->valid ? "valid" : "invalid") > and halt_poll_no_tuning --> halt_poll_invalid >=20 > That seems to be in line with the remaining parts of the patch. It seems we agree on the colour of the bikeshed :)