Patchwork [24/37] KVM: PPC: booke: rework rescheduling checks

login
register
mail settings
Submitter Alexander Graf
Date Feb. 27, 2012, 6:23 p.m.
Message ID <4F4BCA1F.5040905@suse.de>
Download mbox | patch
Permalink /patch/143242/
State Not Applicable
Headers show

Comments

Alexander Graf - Feb. 27, 2012, 6:23 p.m.
On 02/27/2012 06:33 PM, Alexander Graf wrote:
> On 02/27/2012 05:34 PM, Bhushan Bharat-R65777 wrote:
>>
>>> +}
>>> +
>>> +/*
>>> + * Common checks before entering the guest world.  Call with 
>>> interrupts
>>> + * disabled.
>>> + *
>>> + * returns !0 if a signal is pending and check_signal is true  */
>>> +static int kvmppc_prepare_to_enter(struct kvm_vcpu *vcpu, bool
>>> +check_signal) {
>>> +    int r = 0;
>>> +
>>> +    WARN_ON_ONCE(!irqs_disabled());
>>> +    while (true) {
>>> +        if (need_resched()) {
>>> +            local_irq_enable();
>>> +            cond_resched();
>>> +            local_irq_disable();
>>> +            continue;
>>> +        }
>>> +
>>> +        if (kvmppc_core_prepare_to_enter(vcpu)) {
>> kvmppc_prepare_to_enter() is called even on heavyweight_exit. Should 
>> not this be called only on lightweight_exit?
>
> Yeah, we don't need to call it when exiting anyways. That's a 
> functional change though, which this patch is trying not to introduce. 
> So we should rather do that as a patch on top.

So how about this (warning! broken whitespace)?



@@ -483,7 +483,7 @@ static int kvmppc_prepare_to_enter(struct kvm_vcpu 
*vcpu, bool check_signal)
                         continue;
                 }

-               if (check_signal && signal_pending(current))
+               if (signal_pending(current))
                         r = 1;

                 break;
@@ -507,7 +507,7 @@ int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct 
kvm_vcpu *vcpu)
         }

         local_irq_disable();
-       if (kvmppc_prepare_to_enter(vcpu, true)) {
+       if (kvmppc_prepare_to_enter(vcpu)) {
                 kvm_run->exit_reason = KVM_EXIT_INTR;
                 ret = -EINTR;
                 goto out;
@@ -941,13 +941,16 @@ int kvmppc_handle_exit(struct kvm_run *run, struct 
kvm_vcpu *vcpu,
          * To avoid clobbering exit_reason, only check for signals if we
          * aren't already exiting to userspace for some other reason.
          */
-       local_irq_disable();
-       if (kvmppc_prepare_to_enter(vcpu, !(r & RESUME_HOST))) {
-               run->exit_reason = KVM_EXIT_INTR;
-               r = (-EINTR << 2) | RESUME_HOST | (r & RESUME_FLAG_NV);
-               kvmppc_account_exit(vcpu, SIGNAL_EXITS);
+       if (!(r & RESUME_HOST)) {
+               local_irq_disable();
+               if (kvmppc_prepare_to_enter(vcpu)) {
+                       run->exit_reason = KVM_EXIT_INTR;
+                       r = (-EINTR << 2) | RESUME_HOST | (r & 
RESUME_FLAG_NV);
+                       kvmppc_account_exit(vcpu, SIGNAL_EXITS);
+               }
         }

+out:
         return r;
  }
Bharat Bhushan - Feb. 27, 2012, 6:29 p.m.
> -----Original Message-----
> From: Alexander Graf [mailto:agraf@suse.de]
> Sent: Monday, February 27, 2012 11:53 PM
> To: Bhushan Bharat-R65777
> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; linuxppc-dev@lists.ozlabs.org;
> Wood Scott-B07421
> Subject: Re: [PATCH 24/37] KVM: PPC: booke: rework rescheduling checks
> 
> On 02/27/2012 06:33 PM, Alexander Graf wrote:
> > On 02/27/2012 05:34 PM, Bhushan Bharat-R65777 wrote:
> >>
> >>> +}
> >>> +
> >>> +/*
> >>> + * Common checks before entering the guest world.  Call with
> >>> interrupts
> >>> + * disabled.
> >>> + *
> >>> + * returns !0 if a signal is pending and check_signal is true  */
> >>> +static int kvmppc_prepare_to_enter(struct kvm_vcpu *vcpu, bool
> >>> +check_signal) {
> >>> +    int r = 0;
> >>> +
> >>> +    WARN_ON_ONCE(!irqs_disabled());
> >>> +    while (true) {
> >>> +        if (need_resched()) {
> >>> +            local_irq_enable();
> >>> +            cond_resched();
> >>> +            local_irq_disable();
> >>> +            continue;
> >>> +        }
> >>> +
> >>> +        if (kvmppc_core_prepare_to_enter(vcpu)) {
> >> kvmppc_prepare_to_enter() is called even on heavyweight_exit. Should
> >> not this be called only on lightweight_exit?
> >
> > Yeah, we don't need to call it when exiting anyways. That's a
> > functional change though, which this patch is trying not to introduce.
> > So we should rather do that as a patch on top.
> 
> So how about this (warning! broken whitespace)?
> 
> 
> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index
> 7a16b56..616aa2d 100644
> --- a/arch/powerpc/kvm/booke.c
> +++ b/arch/powerpc/kvm/booke.c
> @@ -464,7 +464,7 @@ int kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
>    *
>    * returns !0 if a signal is pending and check_signal is true
>    */
> -static int kvmppc_prepare_to_enter(struct kvm_vcpu *vcpu, bool
> check_signal)
> +static int kvmppc_prepare_to_enter(struct kvm_vcpu *vcpu)
>   {
>          int r = 0;
> 
> @@ -483,7 +483,7 @@ static int kvmppc_prepare_to_enter(struct kvm_vcpu *vcpu,
> bool check_signal)
>                          continue;
>                  }
> 
> -               if (check_signal && signal_pending(current))
> +               if (signal_pending(current))
>                          r = 1;
> 
>                  break;
> @@ -507,7 +507,7 @@ int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu
> *vcpu)
>          }
> 
>          local_irq_disable();
> -       if (kvmppc_prepare_to_enter(vcpu, true)) {
> +       if (kvmppc_prepare_to_enter(vcpu)) {
>                  kvm_run->exit_reason = KVM_EXIT_INTR;
>                  ret = -EINTR;
>                  goto out;
> @@ -941,13 +941,16 @@ int kvmppc_handle_exit(struct kvm_run *run, struct
> kvm_vcpu *vcpu,
>           * To avoid clobbering exit_reason, only check for signals if we
>           * aren't already exiting to userspace for some other reason.
>           */
> -       local_irq_disable();
> -       if (kvmppc_prepare_to_enter(vcpu, !(r & RESUME_HOST))) {
> -               run->exit_reason = KVM_EXIT_INTR;
> -               r = (-EINTR << 2) | RESUME_HOST | (r & RESUME_FLAG_NV);
> -               kvmppc_account_exit(vcpu, SIGNAL_EXITS);
> +       if (!(r & RESUME_HOST)) {
> +               local_irq_disable();
> +               if (kvmppc_prepare_to_enter(vcpu)) {
> +                       run->exit_reason = KVM_EXIT_INTR;
> +                       r = (-EINTR << 2) | RESUME_HOST | (r &
> RESUME_FLAG_NV);
> +                       kvmppc_account_exit(vcpu, SIGNAL_EXITS);
> +               }
>          }
> 
> +out:

Why?
Otherwise looks ok to me.

Thanks
-Bharat

>          return r;
>   }
> 
>

Patch

diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index 7a16b56..616aa2d 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -464,7 +464,7 @@  int kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
   *
   * returns !0 if a signal is pending and check_signal is true
   */
-static int kvmppc_prepare_to_enter(struct kvm_vcpu *vcpu, bool 
check_signal)
+static int kvmppc_prepare_to_enter(struct kvm_vcpu *vcpu)
  {
         int r = 0;