2012-06-10 15:47:27

by Sedat Dilek

[permalink] [raw]
Subject: Re: WARNING: at arch/x86/kernel/kvmclock.c:127

Hi,

I see the same warning especially when resuming from a suspend (see
timestamps >=30-35sec) between Linux v3.5-rc1..v3.5-rc2.

$ sudo grep kvmclock.c /var/log/kern.log
Jun 4 21:11:32 fambox kernel: [ 784.037237] WARNING: at
arch/x86/kernel/kvmclock.c:127
kvm_check_and_clear_guest_paused+0x52/0x60()
Jun 5 20:35:57 fambox kernel: [ 1928.458060] WARNING: at
arch/x86/kernel/kvmclock.c:127
kvm_check_and_clear_guest_paused+0x52/0x60()
Jun 8 09:35:52 fambox kernel: [ 3290.134637] WARNING: at
arch/x86/kernel/kvmclock.c:127
kvm_check_and_clear_guest_paused+0x52/0x60()
Jun 8 09:35:52 fambox kernel: [ 3290.238592] WARNING: at
arch/x86/kernel/kvmclock.c:127
kvm_check_and_clear_guest_paused+0x52/0x60()
Jun 8 12:11:20 fambox kernel: [ 5777.023571] WARNING: at
arch/x86/kernel/kvmclock.c:127
kvm_check_and_clear_guest_paused+0x52/0x60()
Jun 9 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)
+{
+ bool ret = false;
+ struct pvclock_vcpu_time_info *src;
+
+ /*
+ * per_cpu() is safe here because this function is only called from
+ * timer functions where preemption is already disabled.
+ */
+ WARN_ON(!in_atomic());
+ src = &__get_cpu_var(hv_clock);
+ if ((src->flags & PVCLOCK_GUEST_STOPPED) != 0) {
+ __this_cpu_and(hv_clock.flags, ~PVCLOCK_GUEST_STOPPED);
+ ret = true;
+ }
+
+ 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?

Regards,
- Sedat -

[1] http://www.spinics.net/lists/kvm/msg73732.html
[2] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blobdiff;f=arch/x86/kernel/kvmclock.c;h=4ba090ca689db5322fb242b98b53158de5447dc9;hp=f8492da65bfcb03e2775638ad5ce9502099d62c8;hb=3b5d56b9317fa7b5407dff1aa7b115bf6cdbd494;hpb=eae3ee7d8a7c59cf63441dedf28674889f5fc477


2012-06-10 16:08:38

by Sedat Dilek

[permalink] [raw]
Subject: Re: WARNING: at arch/x86/kernel/kvmclock.c:127

On Sun, Jun 10, 2012 at 5:47 PM, Sedat Dilek <[email protected]> wrote:
> Hi,
>
> I see the same warning especially when resuming from a suspend (see
> timestamps >=30-35sec) between Linux v3.5-rc1..v3.5-rc2.
>
> $ sudo grep kvmclock.c /var/log/kern.log
> Jun  4 21:11:32 fambox kernel: [  784.037237] WARNING: at
> arch/x86/kernel/kvmclock.c:127
> kvm_check_and_clear_guest_paused+0x52/0x60()
> Jun  5 20:35:57 fambox kernel: [ 1928.458060] WARNING: at
> arch/x86/kernel/kvmclock.c:127
> kvm_check_and_clear_guest_paused+0x52/0x60()
> Jun  8 09:35:52 fambox kernel: [ 3290.134637] WARNING: at
> arch/x86/kernel/kvmclock.c:127
> kvm_check_and_clear_guest_paused+0x52/0x60()
> Jun  8 09:35:52 fambox kernel: [ 3290.238592] WARNING: at
> arch/x86/kernel/kvmclock.c:127
> kvm_check_and_clear_guest_paused+0x52/0x60()
> Jun  8 12:11:20 fambox kernel: [ 5777.023571] WARNING: at
> arch/x86/kernel/kvmclock.c:127
> kvm_check_and_clear_guest_paused+0x52/0x60()
> Jun  9 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)
> +{
> +       bool ret = false;
> +       struct pvclock_vcpu_time_info *src;
> +
> +       /*
> +        * per_cpu() is safe here because this function is only called from
> +        * timer functions where preemption is already disabled.
> +        */
> +       WARN_ON(!in_atomic());
> +       src = &__get_cpu_var(hv_clock);
> +       if ((src->flags & PVCLOCK_GUEST_STOPPED) != 0) {
> +               __this_cpu_and(hv_clock.flags, ~PVCLOCK_GUEST_STOPPED);
> +               ret = true;
> +       }
> +
> +       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?
>
> Regards,
> - Sedat -
>
> [1] http://www.spinics.net/lists/kvm/msg73732.html
> [2] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blobdiff;f=arch/x86/kernel/kvmclock.c;h=4ba090ca689db5322fb242b98b53158de5447dc9;hp=f8492da65bfcb03e2775638ad5ce9502099d62c8;hb=3b5d56b9317fa7b5407dff1aa7b115bf6cdbd494;hpb=eae3ee7d8a7c59cf63441dedf28674889f5fc477

[ Removed Glauber Costa - Email-address @ RH does not exist anymore ]
[ Added CC to Rafael J. Wysocki (S/R) ]

I added for the sake of correctness the last call-trace I have seen
with Linux v3.5-rc2.
And... I had loaded kvm and kvm_intel kernel-modules, but no KVM
instance actively running or active while doing suspend/resume (S/R).

- Sedat -


Attachments:
CALL-TRACE_kvmclock_kern-log.txt (9.50 kB)

2012-06-11 22:07:51

by Marcelo Tosatti

[permalink] [raw]
Subject: Re: WARNING: at arch/x86/kernel/kvmclock.c:127

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 >=30-35sec) between Linux v3.5-rc1..v3.5-rc2.
> >
> >$ sudo grep kvmclock.c /var/log/kern.log
> >Jun 4 21:11:32 fambox kernel: [ 784.037237] WARNING: at
> >arch/x86/kernel/kvmclock.c:127
> >kvm_check_and_clear_guest_paused+0x52/0x60()
> >Jun 5 20:35:57 fambox kernel: [ 1928.458060] WARNING: at
> >arch/x86/kernel/kvmclock.c:127
> >kvm_check_and_clear_guest_paused+0x52/0x60()
> >Jun 8 09:35:52 fambox kernel: [ 3290.134637] WARNING: at
> >arch/x86/kernel/kvmclock.c:127
> >kvm_check_and_clear_guest_paused+0x52/0x60()
> >Jun 8 09:35:52 fambox kernel: [ 3290.238592] WARNING: at
> >arch/x86/kernel/kvmclock.c:127
> >kvm_check_and_clear_guest_paused+0x52/0x60()
> >Jun 8 12:11:20 fambox kernel: [ 5777.023571] WARNING: at
> >arch/x86/kernel/kvmclock.c:127
> >kvm_check_and_clear_guest_paused+0x52/0x60()
> >Jun 9 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)
> >+{
> >+ bool ret = false;
> >+ struct pvclock_vcpu_time_info *src;
> >+
> >+ /*
> >+ * per_cpu() is safe here because this function is only
> >called from
> >+ * timer functions where preemption is already disabled.
> >+ */
> >+ WARN_ON(!in_atomic());
> >+ src = &__get_cpu_var(hv_clock);
> >+ if ((src->flags & PVCLOCK_GUEST_STOPPED) != 0) {
> >+ __this_cpu_and(hv_clock.flags,
> >~PVCLOCK_GUEST_STOPPED);
> >+ ret = true;
> >+ }
> >+
> >+ 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. Given that this is not the case, the warning can be
> removed. Though before that happens, I have a question: if this 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).

2012-06-11 22:09:21

by Eric B Munson

[permalink] [raw]
Subject: Re: WARNING: at arch/x86/kernel/kvmclock.c:127

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 >=30-35sec) between Linux v3.5-rc1..v3.5-rc2.
>
> $ sudo grep kvmclock.c /var/log/kern.log
> Jun 4 21:11:32 fambox kernel: [ 784.037237] WARNING: at
> arch/x86/kernel/kvmclock.c:127
> kvm_check_and_clear_guest_paused+0x52/0x60()
> Jun 5 20:35:57 fambox kernel: [ 1928.458060] WARNING: at
> arch/x86/kernel/kvmclock.c:127
> kvm_check_and_clear_guest_paused+0x52/0x60()
> Jun 8 09:35:52 fambox kernel: [ 3290.134637] WARNING: at
> arch/x86/kernel/kvmclock.c:127
> kvm_check_and_clear_guest_paused+0x52/0x60()
> Jun 8 09:35:52 fambox kernel: [ 3290.238592] WARNING: at
> arch/x86/kernel/kvmclock.c:127
> kvm_check_and_clear_guest_paused+0x52/0x60()
> Jun 8 12:11:20 fambox kernel: [ 5777.023571] WARNING: at
> arch/x86/kernel/kvmclock.c:127
> kvm_check_and_clear_guest_paused+0x52/0x60()
> Jun 9 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)
> +{
> + bool ret = false;
> + struct pvclock_vcpu_time_info *src;
> +
> + /*
> + * per_cpu() is safe here because this function is only
> called from
> + * timer functions where preemption is already disabled.
> + */
> + WARN_ON(!in_atomic());
> + src = &__get_cpu_var(hv_clock);
> + if ((src->flags & PVCLOCK_GUEST_STOPPED) != 0) {
> + __this_cpu_and(hv_clock.flags,
> ~PVCLOCK_GUEST_STOPPED);
> + ret = true;
> + }
> +
> + 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?

When I wrote the patch originally, I was under the (incorrect)
assumption that the watch dog was only ever reset in an atomic context.
Given that this is not the case, the warning can be removed. Though
before that happens, I have a question: if this is called outside of an
atomic context, is the use of __get_cpu_Var() and __this_cpu_and()
invalid?

Thanks,
Eric

2012-06-11 22:12:14

by Eric B Munson

[permalink] [raw]
Subject: Re: WARNING: at arch/x86/kernel/kvmclock.c:127

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 >=30-35sec) between Linux v3.5-rc1..v3.5-rc2.
>> >
>> >$ sudo grep kvmclock.c /var/log/kern.log
>> >Jun 4 21:11:32 fambox kernel: [ 784.037237] WARNING: at
>> >arch/x86/kernel/kvmclock.c:127
>> >kvm_check_and_clear_guest_paused+0x52/0x60()
>> >Jun 5 20:35:57 fambox kernel: [ 1928.458060] WARNING: at
>> >arch/x86/kernel/kvmclock.c:127
>> >kvm_check_and_clear_guest_paused+0x52/0x60()
>> >Jun 8 09:35:52 fambox kernel: [ 3290.134637] WARNING: at
>> >arch/x86/kernel/kvmclock.c:127
>> >kvm_check_and_clear_guest_paused+0x52/0x60()
>> >Jun 8 09:35:52 fambox kernel: [ 3290.238592] WARNING: at
>> >arch/x86/kernel/kvmclock.c:127
>> >kvm_check_and_clear_guest_paused+0x52/0x60()
>> >Jun 8 12:11:20 fambox kernel: [ 5777.023571] WARNING: at
>> >arch/x86/kernel/kvmclock.c:127
>> >kvm_check_and_clear_guest_paused+0x52/0x60()
>> >Jun 9 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)
>> >+{
>> >+ bool ret = false;
>> >+ struct pvclock_vcpu_time_info *src;
>> >+
>> >+ /*
>> >+ * per_cpu() is safe here because this function is only
>> >called from
>> >+ * timer functions where preemption is already disabled.
>> >+ */
>> >+ WARN_ON(!in_atomic());
>> >+ src = &__get_cpu_var(hv_clock);
>> >+ if ((src->flags & PVCLOCK_GUEST_STOPPED) != 0) {
>> >+ __this_cpu_and(hv_clock.flags,
>> >~PVCLOCK_GUEST_STOPPED);
>> >+ ret = true;
>> >+ }
>> >+
>> >+ 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. Given that this is not the case, the warning can be
>> removed. Though before that happens, I have a question: if this 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. I do not
have access to my working kernel tree and won't for 2 days. I can't get
to the patch until late Wednesday so if someone else wants to work that
up I'd appreciate it.

Thanks,
Eric

2012-06-12 02:14:23

by Marcelo Tosatti

[permalink] [raw]
Subject: Re: WARNING: at arch/x86/kernel/kvmclock.c:127

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 >=30-35sec) between Linux v3.5-rc1..v3.5-rc2.
> >>>
> >>>$ sudo grep kvmclock.c /var/log/kern.log
> >>>Jun 4 21:11:32 fambox kernel: [ 784.037237] WARNING: at
> >>>arch/x86/kernel/kvmclock.c:127
> >>>kvm_check_and_clear_guest_paused+0x52/0x60()
> >>>Jun 5 20:35:57 fambox kernel: [ 1928.458060] WARNING: at
> >>>arch/x86/kernel/kvmclock.c:127
> >>>kvm_check_and_clear_guest_paused+0x52/0x60()
> >>>Jun 8 09:35:52 fambox kernel: [ 3290.134637] WARNING: at
> >>>arch/x86/kernel/kvmclock.c:127
> >>>kvm_check_and_clear_guest_paused+0x52/0x60()
> >>>Jun 8 09:35:52 fambox kernel: [ 3290.238592] WARNING: at
> >>>arch/x86/kernel/kvmclock.c:127
> >>>kvm_check_and_clear_guest_paused+0x52/0x60()
> >>>Jun 8 12:11:20 fambox kernel: [ 5777.023571] WARNING: at
> >>>arch/x86/kernel/kvmclock.c:127
> >>>kvm_check_and_clear_guest_paused+0x52/0x60()
> >>>Jun 9 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)
> >>>+{
> >>>+ bool ret = false;
> >>>+ struct pvclock_vcpu_time_info *src;
> >>>+
> >>>+ /*
> >>>+ * per_cpu() is safe here because this function is only
> >>>called from
> >>>+ * timer functions where preemption is already disabled.
> >>>+ */
> >>>+ WARN_ON(!in_atomic());
> >>>+ src = &__get_cpu_var(hv_clock);
> >>>+ if ((src->flags & PVCLOCK_GUEST_STOPPED) != 0) {
> >>>+ __this_cpu_and(hv_clock.flags,
> >>>~PVCLOCK_GUEST_STOPPED);
> >>>+ ret = true;
> >>>+ }
> >>>+
> >>>+ 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. Given that this is not the case, the warning can be
> >>removed. Though before that happens, I have a question: if this 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. I do not
> have access to my working kernel tree and won't for 2 days. I can'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.

2012-06-12 08:48:35

by Sedat Dilek

[permalink] [raw]
Subject: Re: WARNING: at arch/x86/kernel/kvmclock.c:127

On Tue, Jun 12, 2012 at 4:13 AM, Marcelo Tosatti <[email protected]> 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 >=30-35sec) between Linux v3.5-rc1..v3.5-rc2.
>> >>>
>> >>>$ sudo grep kvmclock.c /var/log/kern.log
>> >>>Jun  4 21:11:32 fambox kernel: [  784.037237] WARNING: at
>> >>>arch/x86/kernel/kvmclock.c:127
>> >>>kvm_check_and_clear_guest_paused+0x52/0x60()
>> >>>Jun  5 20:35:57 fambox kernel: [ 1928.458060] WARNING: at
>> >>>arch/x86/kernel/kvmclock.c:127
>> >>>kvm_check_and_clear_guest_paused+0x52/0x60()
>> >>>Jun  8 09:35:52 fambox kernel: [ 3290.134637] WARNING: at
>> >>>arch/x86/kernel/kvmclock.c:127
>> >>>kvm_check_and_clear_guest_paused+0x52/0x60()
>> >>>Jun  8 09:35:52 fambox kernel: [ 3290.238592] WARNING: at
>> >>>arch/x86/kernel/kvmclock.c:127
>> >>>kvm_check_and_clear_guest_paused+0x52/0x60()
>> >>>Jun  8 12:11:20 fambox kernel: [ 5777.023571] WARNING: at
>> >>>arch/x86/kernel/kvmclock.c:127
>> >>>kvm_check_and_clear_guest_paused+0x52/0x60()
>> >>>Jun  9 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)
>> >>>+{
>> >>>+       bool ret = false;
>> >>>+       struct pvclock_vcpu_time_info *src;
>> >>>+
>> >>>+       /*
>> >>>+        * per_cpu() is safe here because this function is only
>> >>>called from
>> >>>+        * timer functions where preemption is already disabled.
>> >>>+        */
>> >>>+       WARN_ON(!in_atomic());
>> >>>+       src = &__get_cpu_var(hv_clock);
>> >>>+       if ((src->flags & PVCLOCK_GUEST_STOPPED) != 0) {
>> >>>+               __this_cpu_and(hv_clock.flags,
>> >>>~PVCLOCK_GUEST_STOPPED);
>> >>>+               ret = true;
>> >>>+       }
>> >>>+
>> >>>+       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.  Given that this is not the case, the warning can be
>> >>removed.  Though before that happens, I have a question: if this 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.  I do not
>> have access to my working kernel tree and won't for 2 days.  I can'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.
>

I am reading your email approx. 6hrs after you have sent it and I do
NOT see the patch in [1].
Can you please send me the patch as an attachment, Thanks!

- Sedat -

[1] http://git.kernel.org/?p=virt/kvm/kvm.git;a=shortlog;h=refs/heads/master

2012-06-15 09:41:43

by Sedat Dilek

[permalink] [raw]
Subject: Re: WARNING: at arch/x86/kernel/kvmclock.c:127

On Tue, Jun 12, 2012 at 4:13 AM, Marcelo Tosatti <[email protected]> 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 >=30-35sec) between Linux v3.5-rc1..v3.5-rc2.
>> >>>
>> >>>$ sudo grep kvmclock.c /var/log/kern.log
>> >>>Jun  4 21:11:32 fambox kernel: [  784.037237] WARNING: at
>> >>>arch/x86/kernel/kvmclock.c:127
>> >>>kvm_check_and_clear_guest_paused+0x52/0x60()
>> >>>Jun  5 20:35:57 fambox kernel: [ 1928.458060] WARNING: at
>> >>>arch/x86/kernel/kvmclock.c:127
>> >>>kvm_check_and_clear_guest_paused+0x52/0x60()
>> >>>Jun  8 09:35:52 fambox kernel: [ 3290.134637] WARNING: at
>> >>>arch/x86/kernel/kvmclock.c:127
>> >>>kvm_check_and_clear_guest_paused+0x52/0x60()
>> >>>Jun  8 09:35:52 fambox kernel: [ 3290.238592] WARNING: at
>> >>>arch/x86/kernel/kvmclock.c:127
>> >>>kvm_check_and_clear_guest_paused+0x52/0x60()
>> >>>Jun  8 12:11:20 fambox kernel: [ 5777.023571] WARNING: at
>> >>>arch/x86/kernel/kvmclock.c:127
>> >>>kvm_check_and_clear_guest_paused+0x52/0x60()
>> >>>Jun  9 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)
>> >>>+{
>> >>>+       bool ret = false;
>> >>>+       struct pvclock_vcpu_time_info *src;
>> >>>+
>> >>>+       /*
>> >>>+        * per_cpu() is safe here because this function is only
>> >>>called from
>> >>>+        * timer functions where preemption is already disabled.
>> >>>+        */
>> >>>+       WARN_ON(!in_atomic());
>> >>>+       src = &__get_cpu_var(hv_clock);
>> >>>+       if ((src->flags & PVCLOCK_GUEST_STOPPED) != 0) {
>> >>>+               __this_cpu_and(hv_clock.flags,
>> >>>~PVCLOCK_GUEST_STOPPED);
>> >>>+               ret = true;
>> >>>+       }
>> >>>+
>> >>>+       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.  Given that this is not the case, the warning can be
>> >>removed.  Though before that happens, I have a question: if this 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.  I do not
>> have access to my working kernel tree and won't for 2 days.  I can'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.
>

Unfortunately, I was busy this week with familiar affairs.
I have seen too late that Marcelo sent me the patch via PM.

Marcelo, could you please give credits to people next time, Thanks.

I am missing sth. like this in the commit now in Linus upstream GIT...

Reported-by: Frank Arnold <frank.arnold@...> (hidden - me reads KVM ML offline)
Reported-by: Sedat Dilek <[email protected]> (lazy guy - could
have reported earlier)
Acked-by or Reviewed-by: Eric B Munson <[email protected]>

- Sedat -

[1] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=e32025a56403df4386cd61a741c0a36afe79ae8a

2012-06-15 13:45:15

by Marcelo Tosatti

[permalink] [raw]
Subject: Re: WARNING: at arch/x86/kernel/kvmclock.c:127

On Fri, Jun 15, 2012 at 11:35:58AM +0200, Sedat Dilek wrote:
> On Tue, Jun 12, 2012 at 4:13 AM, Marcelo Tosatti <[email protected]> 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 >=30-35sec) between Linux v3.5-rc1..v3.5-rc2.
> >> >>>
> >> >>>$ sudo grep kvmclock.c /var/log/kern.log
> >> >>>Jun ?4 21:11:32 fambox kernel: [ ?784.037237] WARNING: at
> >> >>>arch/x86/kernel/kvmclock.c:127
> >> >>>kvm_check_and_clear_guest_paused+0x52/0x60()
> >> >>>Jun ?5 20:35:57 fambox kernel: [ 1928.458060] WARNING: at
> >> >>>arch/x86/kernel/kvmclock.c:127
> >> >>>kvm_check_and_clear_guest_paused+0x52/0x60()
> >> >>>Jun ?8 09:35:52 fambox kernel: [ 3290.134637] WARNING: at
> >> >>>arch/x86/kernel/kvmclock.c:127
> >> >>>kvm_check_and_clear_guest_paused+0x52/0x60()
> >> >>>Jun ?8 09:35:52 fambox kernel: [ 3290.238592] WARNING: at
> >> >>>arch/x86/kernel/kvmclock.c:127
> >> >>>kvm_check_and_clear_guest_paused+0x52/0x60()
> >> >>>Jun ?8 12:11:20 fambox kernel: [ 5777.023571] WARNING: at
> >> >>>arch/x86/kernel/kvmclock.c:127
> >> >>>kvm_check_and_clear_guest_paused+0x52/0x60()
> >> >>>Jun ?9 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)
> >> >>>+{
> >> >>>+ ? ? ? bool ret = false;
> >> >>>+ ? ? ? struct pvclock_vcpu_time_info *src;
> >> >>>+
> >> >>>+ ? ? ? /*
> >> >>>+ ? ? ? ?* per_cpu() is safe here because this function is only
> >> >>>called from
> >> >>>+ ? ? ? ?* timer functions where preemption is already disabled.
> >> >>>+ ? ? ? ?*/
> >> >>>+ ? ? ? WARN_ON(!in_atomic());
> >> >>>+ ? ? ? src = &__get_cpu_var(hv_clock);
> >> >>>+ ? ? ? if ((src->flags & PVCLOCK_GUEST_STOPPED) != 0) {
> >> >>>+ ? ? ? ? ? ? ? __this_cpu_and(hv_clock.flags,
> >> >>>~PVCLOCK_GUEST_STOPPED);
> >> >>>+ ? ? ? ? ? ? ? ret = true;
> >> >>>+ ? ? ? }
> >> >>>+
> >> >>>+ ? ? ? 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. ?Given that this is not the case, the warning can be
> >> >>removed. ?Though before that happens, I have a question: if this 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. ?I do not
> >> have access to my working kernel tree and won't for 2 days. ?I can'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.
> >
>
> Unfortunately, I was busy this week with familiar affairs.
> I have seen too late that Marcelo sent me the patch via PM.
>
> Marcelo, could you please give credits to people next time, Thanks.
>
> I am missing sth. like this in the commit now in Linus upstream GIT...
>
> Reported-by: Frank Arnold <frank.arnold@...> (hidden - me reads KVM ML offline)
> Reported-by: Sedat Dilek <[email protected]> (lazy guy - could
> have reported earlier)
> Acked-by or Reviewed-by: Eric B Munson <[email protected]>
>
> - Sedat -
>
> [1] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=e32025a56403df4386cd61a741c0a36afe79ae8a

Sedat,

You are right, sorry for not giving appropriate credit.

Unfortunately we cannot change the commit message now.

Thanks anyway for reporting.