From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
To: Paul Mackerras <paulus@samba.org>
Cc: agraf@suse.de, benh@kernel.crashing.org,
linuxppc-dev@lists.ozlabs.org, kvm-ppc@vger.kernel.org,
kvm@vger.kernel.org
Subject: Re: [PATCH] KVM: PPC: BOOK3S: PR: Fix WARN_ON with debug options on
Date: Wed, 07 May 2014 07:58:38 +0000 [thread overview]
Message-ID: <87iopiuijg.fsf@linux.vnet.ibm.com> (raw)
In-Reply-To: <20140507055626.GA26650@iris.ozlabs.ibm.com>
Paul Mackerras <paulus@samba.org> writes:
> On Sun, May 04, 2014 at 10:56:08PM +0530, Aneesh Kumar K.V wrote:
>> With debug option "sleep inside atomic section checking" enabled we get
>> the below WARN_ON during a PR KVM boot. This is because upstream now
>> have PREEMPT_COUNT enabled even if we have preempt disabled. Fix the
>> warning by adding preempt_disable/enable around floating point and altivec
>> enable.
>
> This worries me a bit. In this code:
>
>> if (msr & MSR_FP) {
>> + preempt_disable();
>> enable_kernel_fp();
>> load_fp_state(&vcpu->arch.fp);
>> t->fp_save_area = &vcpu->arch.fp;
>> + preempt_enable();
>
> What would happen if we actually did get preempted at this point?
> Wouldn't we lose the FP state we just loaded?
I was not sure we have got CONFIG_PREEMPT working with kvm. So i was
not looking at preempted case. But yes, if we have PREEMPT enabled it
would break as per the current code.
>
> In other words, how come we're not already preempt-disabled at this
> point?
I don't see us disabling preempt in the code path. Are you suggesting
that we should be preempt disabled for the whole duration of
kvmppc_handle_ext ?
-aneesh
WARNING: multiple messages have this Message-ID (diff)
From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
To: Paul Mackerras <paulus@samba.org>
Cc: linuxppc-dev@lists.ozlabs.org, agraf@suse.de,
kvm-ppc@vger.kernel.org, kvm@vger.kernel.org
Subject: Re: [PATCH] KVM: PPC: BOOK3S: PR: Fix WARN_ON with debug options on
Date: Wed, 07 May 2014 13:28:27 +0530 [thread overview]
Message-ID: <87iopiuijg.fsf@linux.vnet.ibm.com> (raw)
In-Reply-To: <20140507055626.GA26650@iris.ozlabs.ibm.com>
Paul Mackerras <paulus@samba.org> writes:
> On Sun, May 04, 2014 at 10:56:08PM +0530, Aneesh Kumar K.V wrote:
>> With debug option "sleep inside atomic section checking" enabled we get
>> the below WARN_ON during a PR KVM boot. This is because upstream now
>> have PREEMPT_COUNT enabled even if we have preempt disabled. Fix the
>> warning by adding preempt_disable/enable around floating point and altivec
>> enable.
>
> This worries me a bit. In this code:
>
>> if (msr & MSR_FP) {
>> + preempt_disable();
>> enable_kernel_fp();
>> load_fp_state(&vcpu->arch.fp);
>> t->fp_save_area = &vcpu->arch.fp;
>> + preempt_enable();
>
> What would happen if we actually did get preempted at this point?
> Wouldn't we lose the FP state we just loaded?
I was not sure we have got CONFIG_PREEMPT working with kvm. So i was
not looking at preempted case. But yes, if we have PREEMPT enabled it
would break as per the current code.
>
> In other words, how come we're not already preempt-disabled at this
> point?
I don't see us disabling preempt in the code path. Are you suggesting
that we should be preempt disabled for the whole duration of
kvmppc_handle_ext ?
-aneesh
WARNING: multiple messages have this Message-ID (diff)
From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
To: Paul Mackerras <paulus@samba.org>
Cc: agraf@suse.de, benh@kernel.crashing.org,
linuxppc-dev@lists.ozlabs.org, kvm-ppc@vger.kernel.org,
kvm@vger.kernel.org
Subject: Re: [PATCH] KVM: PPC: BOOK3S: PR: Fix WARN_ON with debug options on
Date: Wed, 07 May 2014 13:28:27 +0530 [thread overview]
Message-ID: <87iopiuijg.fsf@linux.vnet.ibm.com> (raw)
In-Reply-To: <20140507055626.GA26650@iris.ozlabs.ibm.com>
Paul Mackerras <paulus@samba.org> writes:
> On Sun, May 04, 2014 at 10:56:08PM +0530, Aneesh Kumar K.V wrote:
>> With debug option "sleep inside atomic section checking" enabled we get
>> the below WARN_ON during a PR KVM boot. This is because upstream now
>> have PREEMPT_COUNT enabled even if we have preempt disabled. Fix the
>> warning by adding preempt_disable/enable around floating point and altivec
>> enable.
>
> This worries me a bit. In this code:
>
>> if (msr & MSR_FP) {
>> + preempt_disable();
>> enable_kernel_fp();
>> load_fp_state(&vcpu->arch.fp);
>> t->fp_save_area = &vcpu->arch.fp;
>> + preempt_enable();
>
> What would happen if we actually did get preempted at this point?
> Wouldn't we lose the FP state we just loaded?
I was not sure we have got CONFIG_PREEMPT working with kvm. So i was
not looking at preempted case. But yes, if we have PREEMPT enabled it
would break as per the current code.
>
> In other words, how come we're not already preempt-disabled at this
> point?
I don't see us disabling preempt in the code path. Are you suggesting
that we should be preempt disabled for the whole duration of
kvmppc_handle_ext ?
-aneesh
next prev parent reply other threads:[~2014-05-07 7:58 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-05-04 17:26 [PATCH] KVM: PPC: BOOK3S: PR: Fix WARN_ON with debug options on Aneesh Kumar K.V
2014-05-04 17:38 ` Aneesh Kumar K.V
2014-05-04 17:26 ` Aneesh Kumar K.V
2014-05-05 11:29 ` Alexander Graf
2014-05-05 11:29 ` Alexander Graf
2014-05-05 11:29 ` Alexander Graf
2014-05-07 5:56 ` Paul Mackerras
2014-05-07 5:56 ` Paul Mackerras
2014-05-07 5:56 ` Paul Mackerras
2014-05-07 7:58 ` Aneesh Kumar K.V [this message]
2014-05-07 7:58 ` Aneesh Kumar K.V
2014-05-07 7:58 ` Aneesh Kumar K.V
2014-05-07 11:37 ` Alexander Graf
2014-05-07 11:37 ` Alexander Graf
2014-05-07 11:37 ` Alexander Graf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87iopiuijg.fsf@linux.vnet.ibm.com \
--to=aneesh.kumar@linux.vnet.ibm.com \
--cc=agraf@suse.de \
--cc=benh@kernel.crashing.org \
--cc=kvm-ppc@vger.kernel.org \
--cc=kvm@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=paulus@samba.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.