From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Tosatti Subject: Re: WARNING: at arch/x86/kernel/kvmclock.c:127 Date: Fri, 15 Jun 2012 08:08:37 -0300 Message-ID: <20120615110837.GA14316@amt.cnet> References: <0669740d6e2f3648f1c5c9a50e72f6eb@mgebm.net> <20120611220719.GA27415@amt.cnet> <20120612021352.GA2845@amt.cnet> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Eric B Munson , kvm@vger.kernel.org, LKML , Avi Kivity , Thomas Gleixner , Ingo Molnar , x86@kernel.org To: sedat.dilek@gmail.com Return-path: Content-Disposition: inline In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org List-Id: kvm.vger.kernel.org On Fri, Jun 15, 2012 at 11:35:58AM +0200, Sedat Dilek wrote: > On Tue, Jun 12, 2012 at 4:13 AM, Marcelo Tosatti wrote: > > On Mon, Jun 11, 2012 at 06:10:34PM -0400, Eric B Munson wrote: > >> On Mon, 11 Jun 2012 19:07:19 -0300, Marcelo Tosatti wrote: > >> >On Mon, Jun 11, 2012 at 05:47:00PM -0400, Eric B Munson wrote: > >> >>On Sun, 10 Jun 2012 17:47:24 +0200, Sedat Dilek wrote: > >> >>>Hi, > >> >>> > >> >>>I see the same warning especially when resuming from a suspend = (see > >> >>>timestamps >=3D30-35sec) between Linux v3.5-rc1..v3.5-rc2. > >> >>> > >> >>>$ sudo grep kvmclock.c /var/log/kern.log > >> >>>Jun =A04 21:11:32 fambox kernel: [ =A0784.037237] WARNING: at > >> >>>arch/x86/kernel/kvmclock.c:127 > >> >>>kvm_check_and_clear_guest_paused+0x52/0x60() > >> >>>Jun =A05 20:35:57 fambox kernel: [ 1928.458060] WARNING: at > >> >>>arch/x86/kernel/kvmclock.c:127 > >> >>>kvm_check_and_clear_guest_paused+0x52/0x60() > >> >>>Jun =A08 09:35:52 fambox kernel: [ 3290.134637] WARNING: at > >> >>>arch/x86/kernel/kvmclock.c:127 > >> >>>kvm_check_and_clear_guest_paused+0x52/0x60() > >> >>>Jun =A08 09:35:52 fambox kernel: [ 3290.238592] WARNING: at > >> >>>arch/x86/kernel/kvmclock.c:127 > >> >>>kvm_check_and_clear_guest_paused+0x52/0x60() > >> >>>Jun =A08 12:11:20 fambox kernel: [ 5777.023571] WARNING: at > >> >>>arch/x86/kernel/kvmclock.c:127 > >> >>>kvm_check_and_clear_guest_paused+0x52/0x60() > >> >>>Jun =A09 13:32:50 fambox kernel: [ 2778.842695] WARNING: at > >> >>>arch/x86/kernel/kvmclock.c:127 > >> >>>kvm_check_and_clear_guest_paused+0x52/0x60() > >> >>> > >> >>>From [1]: > >> >>> > >> >>>"...The warning itself is not required for the check_and_clear > >> >>>function and can be removed as far as I am concerned." > >> >>> > >> >>>From [2] commit 3b5d56b9317fa7b5407dff1aa7b115bf6cdbd494 > >> >>("kvmclock: > >> >>>Add functions to check if the host has stopped the vm") > >> >>>... > >> >>>+bool kvm_check_and_clear_guest_paused(void) > >> >>>+{ > >> >>>+ =A0 =A0 =A0 bool ret =3D false; > >> >>>+ =A0 =A0 =A0 struct pvclock_vcpu_time_info *src; > >> >>>+ > >> >>>+ =A0 =A0 =A0 /* > >> >>>+ =A0 =A0 =A0 =A0* per_cpu() is safe here because this function= is only > >> >>>called from > >> >>>+ =A0 =A0 =A0 =A0* timer functions where preemption is already = disabled. > >> >>>+ =A0 =A0 =A0 =A0*/ > >> >>>+ =A0 =A0 =A0 WARN_ON(!in_atomic()); > >> >>>+ =A0 =A0 =A0 src =3D &__get_cpu_var(hv_clock); > >> >>>+ =A0 =A0 =A0 if ((src->flags & PVCLOCK_GUEST_STOPPED) !=3D 0) = { > >> >>>+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 __this_cpu_and(hv_clock.flags, > >> >>>~PVCLOCK_GUEST_STOPPED); > >> >>>+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 ret =3D true; > >> >>>+ =A0 =A0 =A0 } > >> >>>+ > >> >>>+ =A0 =A0 =A0 return ret; > >> >>>+} > >> >>>+EXPORT_SYMBOL_GPL(kvm_check_and_clear_guest_paused); > >> >>>+ > >> >>>... > >> >>>( The export macro was dropped in a followup commit. ) > >> >>> > >> >>>So you mean "WARN_ON(!in_atomic());" can be deleted? > >> > > >> >Yes. > >> > > >> >>When I wrote the patch originally, I was under the (incorrect) > >> >>assumption that the watch dog was only ever reset in an atomic > >> >>context. =A0Given that this is not the case, the warning can be > >> >>removed. =A0Though before that happens, I have a question: if th= is is > >> >>called outside of an atomic context, is the use of __get_cpu_Var= () > >> >>and __this_cpu_and() invalid? > >> > > >> >It remains valid because its called with interrupts > >> >disabled (see migrate_hrtimers). > >> > >> Thank you, that was my assumption but I wanted to confirm. =A0I do= not > >> have access to my working kernel tree and won't for 2 days. =A0I c= an't > >> get to the patch until late Wednesday so if someone else wants to > >> work that up I'd appreciate it. > >> > >> Thanks, > >> Eric > > > > Removed the warning in kvm.git master. > > >=20 > Unfortunately, I was busy this week with familiar affairs. > I have seen too late that Marcelo sent me the patch via PM. >=20 > Marcelo, could you please give credits to people next time, Thanks. >=20 > I am missing sth. like this in the commit now in Linus upstream GIT..= =2E >=20 > Reported-by: Frank Arnold (hidden - me reads KVM M= L offline) > Reported-by: Sedat Dilek (lazy guy - could > have reported earlier) > Acked-by or Reviewed-by: Eric B Munson >=20 > - Sedat - >=20 > [1] http://git.kernel.org/?p=3Dlinux/kernel/git/torvalds/linux.git;a=3D= commitdiff;h=3De32025a56403df4386cd61a741c0a36afe79ae8a Sedat, You are right, sorry for not giving appropriate credit. Unfortunately we cannot change the commit message now. Thanks anyway for reporting.