[PATCH v2 0/2] xen/arm: vcpu_yield on WFE

[PATCH v2 0/2] xen/arm: vcpu_yield on WFE

[Stefano Stabellini]

Hi all,
the patches should be self-explanatory.

Changes in v2:
- do not change parameters and return value of do_yield.


Stefano Stabellini (2):
      xen: export do_yield as vcpu_yield
      xen/arm: call vcpu_yield on WFE trap

 xen/arch/arm/traps.c    |    2 +-
 xen/common/schedule.c   |    6 +++---
 xen/include/xen/sched.h |    1 +
 3 files changed, 5 insertions(+), 4 deletions(-)

[PATCH v2 1/2] xen: export do_yield as vcpu_yield

[Stefano Stabellini]

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
CC: george.dunlap@eu.citrix.com
CC: JBeulich@suse.com

Changes in v2:
- do not change paramters and return value of do_yield.
---
 xen/common/schedule.c   |    6 +++---
 xen/include/xen/sched.h |    1 +
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index e9eb0bc..9a49769 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -795,7 +795,7 @@ static long do_poll(struct sched_poll *sched_poll)
 }
 
 /* Voluntarily yield the processor for this allocation. */
-static long do_yield(void)
+long vcpu_yield(void)
 {
     struct vcpu * v=current;
     spinlock_t *lock = vcpu_schedule_lock_irq(v);
@@ -888,7 +888,7 @@ long do_sched_op_compat(int cmd, unsigned long arg)
     {
     case SCHEDOP_yield:
     {
-        ret = do_yield();
+        ret = vcpu_yield();
         break;
     }
 
@@ -925,7 +925,7 @@ ret_t do_sched_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
     {
     case SCHEDOP_yield:
     {
-        ret = do_yield();
+        ret = vcpu_yield();
         break;
     }
 
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 2f876f5..be932cf 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -634,6 +634,7 @@ int  sched_id(void);
 void sched_tick_suspend(void);
 void sched_tick_resume(void);
 void vcpu_wake(struct vcpu *v);
+long vcpu_yield(void);
 void vcpu_sleep_nosync(struct vcpu *v);
 void vcpu_sleep_sync(struct vcpu *v);
 
-- 
1.7.10.4

Re: [PATCH v2 1/2] xen: export do_yield as vcpu_yield

[George Dunlap]

On 07/23/2014 02:37 PM, Stefano Stabellini wrote:
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> CC: george.dunlap@eu.citrix.com
> CC: JBeulich@suse.com

Acked-by: George Dunlap <george.dunlap@eu.citrix.com>

>
> Changes in v2:
> - do not change paramters and return value of do_yield.
> ---
>   xen/common/schedule.c   |    6 +++---
>   xen/include/xen/sched.h |    1 +
>   2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/xen/common/schedule.c b/xen/common/schedule.c
> index e9eb0bc..9a49769 100644
> --- a/xen/common/schedule.c
> +++ b/xen/common/schedule.c
> @@ -795,7 +795,7 @@ static long do_poll(struct sched_poll *sched_poll)
>   }
>   
>   /* Voluntarily yield the processor for this allocation. */
> -static long do_yield(void)
> +long vcpu_yield(void)
>   {
>       struct vcpu * v=current;
>       spinlock_t *lock = vcpu_schedule_lock_irq(v);
> @@ -888,7 +888,7 @@ long do_sched_op_compat(int cmd, unsigned long arg)
>       {
>       case SCHEDOP_yield:
>       {
> -        ret = do_yield();
> +        ret = vcpu_yield();
>           break;
>       }
>   
> @@ -925,7 +925,7 @@ ret_t do_sched_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>       {
>       case SCHEDOP_yield:
>       {
> -        ret = do_yield();
> +        ret = vcpu_yield();
>           break;
>       }
>   
> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> index 2f876f5..be932cf 100644
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -634,6 +634,7 @@ int  sched_id(void);
>   void sched_tick_suspend(void);
>   void sched_tick_resume(void);
>   void vcpu_wake(struct vcpu *v);
> +long vcpu_yield(void);
>   void vcpu_sleep_nosync(struct vcpu *v);
>   void vcpu_sleep_sync(struct vcpu *v);
>   

[PATCH v2 2/2] xen/arm: call vcpu_yield on WFE trap

[Stefano Stabellini]

No need to call vcpu_force_reschedule, is too expensive.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen/arch/arm/traps.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 3dfabd0..76a9586 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1805,7 +1805,7 @@ asmlinkage void do_trap_hypervisor(struct cpu_user_regs *regs)
         }
         if ( hsr.wfi_wfe.ti ) {
             /* Yield the VCPU for WFE */
-            vcpu_force_reschedule(current);
+            vcpu_yield();
         } else {
             /* Block the VCPU for WFI */
             vcpu_block_unless_event_pending(current);
-- 
1.7.10.4

Re: [PATCH v2 0/2] xen/arm: vcpu_yield on WFE

[Julien Grall]

On 07/23/2014 02:35 PM, Stefano Stabellini wrote:
> Hi all,

Hi Stefano,

> the patches should be self-explanatory.

I gave a try of this series on midway and hackbench get stuck after few
minutes.

FYI, I'm not able to reproduce on our local midway node.

Regards,

> Changes in v2:
> - do not change parameters and return value of do_yield.
> 
> 
> Stefano Stabellini (2):
>       xen: export do_yield as vcpu_yield
>       xen/arm: call vcpu_yield on WFE trap
> 
>  xen/arch/arm/traps.c    |    2 +-
>  xen/common/schedule.c   |    6 +++---
>  xen/include/xen/sched.h |    1 +
>  3 files changed, 5 insertions(+), 4 deletions(-)
> 


-- 
Julien Grall

Re: [PATCH v2 0/2] xen/arm: vcpu_yield on WFE

[Stefano Stabellini]

On Wed, 23 Jul 2014, Julien Grall wrote:
> On 07/23/2014 02:35 PM, Stefano Stabellini wrote:
> > Hi all,
> 
> Hi Stefano,
> 
> > the patches should be self-explanatory.
> 
> I gave a try of this series on midway and hackbench get stuck after few
> minutes.

Just as a confirmation, you did run hackbench before on midway and it
didn't get stuck, right?


> FYI, I'm not able to reproduce on our local midway node.

Without a reproducing environment is going to be very hard to fix it.
It might be worth living it running in a loop for a few hours...


> Regards,
> 
> > Changes in v2:
> > - do not change parameters and return value of do_yield.
> > 
> > 
> > Stefano Stabellini (2):
> >       xen: export do_yield as vcpu_yield
> >       xen/arm: call vcpu_yield on WFE trap
> > 
> >  xen/arch/arm/traps.c    |    2 +-
> >  xen/common/schedule.c   |    6 +++---
> >  xen/include/xen/sched.h |    1 +
> >  3 files changed, 5 insertions(+), 4 deletions(-)
> > 
> 
> 
> -- 
> Julien Grall
> 

Re: [PATCH v2 0/2] xen/arm: vcpu_yield on WFE

[Julien Grall]

On 07/23/2014 05:21 PM, Stefano Stabellini wrote:
> On Wed, 23 Jul 2014, Julien Grall wrote:
>> On 07/23/2014 02:35 PM, Stefano Stabellini wrote:
>>> Hi all,
>>
>> Hi Stefano,
>>
>>> the patches should be self-explanatory.
>>
>> I gave a try of this series on midway and hackbench get stuck after few
>> minutes.
> 
> Just as a confirmation, you did run hackbench before on midway and it
> didn't get stuck, right?

Yes. The latest working version is c047211.

I got 2 different kind of errors with a guest setup with 2 VCPUs:
	1) Hackbench gets stuck and use 100% of the CPU.
	2) Hackbench heap gets corrupted

For the former, I was thinking about a possible missing SGI.

>> FYI, I'm not able to reproduce on our local midway node.
> 
> Without a reproducing environment is going to be very hard to fix it.
> It might be worth living it running in a loop for a few hours...

Sorry I meant I'm able. I spent my afternoon to set up the same
environment as the CI loop.

It looks like the bug is likely the same on the Arndale, even if WFE is
not trap. I will try to bisect on this board.

Regards,

-- 
Julien Grall

Re: [PATCH v2 0/2] xen/arm: vcpu_yield on WFE

[Julien Grall]

On 07/23/2014 05:33 PM, Julien Grall wrote:
> On 07/23/2014 05:21 PM, Stefano Stabellini wrote:
>> On Wed, 23 Jul 2014, Julien Grall wrote:
>>> On 07/23/2014 02:35 PM, Stefano Stabellini wrote:
>>>> Hi all,
>>>
>>> Hi Stefano,
>>>
>>>> the patches should be self-explanatory.
>>>
>>> I gave a try of this series on midway and hackbench get stuck after few
>>> minutes.
>>
>> Just as a confirmation, you did run hackbench before on midway and it
>> didn't get stuck, right?
> 
> Yes. The latest working version is c047211.
> 
> I got 2 different kind of errors with a guest setup with 2 VCPUs:
> 	1) Hackbench gets stuck and use 100% of the CPU.
> 	2) Hackbench heap gets corrupted
> 
> For the former, I was thinking about a possible missing SGI.
> 
>>> FYI, I'm not able to reproduce on our local midway node.
>>
>> Without a reproducing environment is going to be very hard to fix it.
>> It might be worth living it running in a loop for a few hours...
> 
> Sorry I meant I'm able. I spent my afternoon to set up the same
> environment as the CI loop.
> 
> It looks like the bug is likely the same on the Arndale, even if WFE is
> not trap. I will try to bisect on this board.

Hrmmmm, I misread the CI log [1]. The Arndale is working fine without
this patch.

I'm able to confirm to confirm this behavior locally on my board. So
both Midway and the Arndale is impacted with this bug. Even though it
took a bit longer to reproduce the issue on the Arndale.

With your series, the benchmark is broken on midway and working on the
Arndale. So there is a hidden bug at least on ARM32.

[1]
https://validation.linaro.org/dashboard/streams/private/team/linaro/virtualization/bundles/067c7274dfb08eeed970cbc51b9acdd256a83aa9/

-- 
Julien Grall

Re: [PATCH v2 0/2] xen/arm: vcpu_yield on WFE

[Ian Campbell]

On Wed, 2014-07-23 at 14:35 +0100, Stefano Stabellini wrote:
> Stefano Stabellini (2):
>       xen: export do_yield as vcpu_yield
>       xen/arm: call vcpu_yield on WFE trap

Applied.

Ian.