Re: [PATCH 2/2] xen: sched: we never get into context_switch() with prev==next

From: Julien Grall <julien.grall@arm.com>
To: Dario Faggioli <dfaggioli@suse.com>, xen-devel@lists.xenproject.org
Cc: "Juergen Gross" <jgross@suse.com>,
	"Stefano Stabellini" <sstabellini@kernel.org>,
	"Wei Liu" <wei.liu2@citrix.com>,
	"Andrew Cooper" <andrew.cooper3@citrix.com>,
	"George Dunlap" <george.dunlap@citrix.com>,
	"Jan Beulich" <jbeulich@suse.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>
Subject: Re: [PATCH 2/2] xen: sched: we never get into context_switch() with prev==next
Date: Sun, 21 Apr 2019 18:08:34 +0100	[thread overview]
Message-ID: <082f8826-1a75-f11a-05ef-2bed4ee8023d@arm.com> (raw)
In-Reply-To: <155577388740.25746.3780283868034526234.stgit@wayrath>

Hi Dario,

On 4/20/19 4:24 PM, Dario Faggioli wrote:
> In schedule(), if we pick, as the next vcpu to run (next) the same one
> that is running already (prev), we never get to call context_switch().
> 
> We can, therefore, get rid of all the `if`-s testing prev and next being
> different, trading them with an ASSERT() (on ARM, the ASSERT() was even
> already there!)
> 
> Signed-off-by: Dario Faggioli <dfaggioli@suse.com>
> Suggested-by: Juergen Gross <jgross@suse.com>

For Arm:

Acked-by: Julien Grall <julien.grall@arm.com>

Cheers,

> ---
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Julien Grall <julien.grall@arm.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: "Roger Pau Monné" <roger.pau@citrix.com>
> Cc: George Dunlap <george.dunlap@citrix.com>
> Cc: Juergen Gross <jgross@suse.com>
> ---
>   xen/arch/arm/domain.c |    3 +--
>   xen/arch/x86/domain.c |   22 ++++++++--------------
>   2 files changed, 9 insertions(+), 16 deletions(-)
> 
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 6dc633ed50..915ae0b4c6 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -343,8 +343,7 @@ void context_switch(struct vcpu *prev, struct vcpu *next)
>       ASSERT(prev != next);
>       ASSERT(!vcpu_cpu_dirty(next));
>   
> -    if ( prev != next )
> -        update_runstate_area(prev);
> +    update_runstate_area(prev);
>   
>       local_irq_disable();
>   
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index 9eaa978ce5..d2d9f2fc3c 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -1721,6 +1721,7 @@ void context_switch(struct vcpu *prev, struct vcpu *next)
>       const struct domain *prevd = prev->domain, *nextd = next->domain;
>       unsigned int dirty_cpu = next->dirty_cpu;
>   
> +    ASSERT(prev != next);
>       ASSERT(local_irq_is_enabled());
>   
>       get_cpu_info()->use_pv_cr3 = false;
> @@ -1732,12 +1733,9 @@ void context_switch(struct vcpu *prev, struct vcpu *next)
>           flush_mask(cpumask_of(dirty_cpu), FLUSH_VCPU_STATE);
>       }
>   
> -    if ( prev != next )
> -    {
> -        _update_runstate_area(prev);
> -        vpmu_switch_from(prev);
> -        np2m_schedule(NP2M_SCHEDLE_OUT);
> -    }
> +    _update_runstate_area(prev);
> +    vpmu_switch_from(prev);
> +    np2m_schedule(NP2M_SCHEDLE_OUT);
>   
>       if ( is_hvm_domain(prevd) && !list_empty(&prev->arch.hvm.tm_list) )
>           pt_save_timer(prev);
> @@ -1794,14 +1792,10 @@ void context_switch(struct vcpu *prev, struct vcpu *next)
>   
>       context_saved(prev);
>   
> -    if ( prev != next )
> -    {
> -        _update_runstate_area(next);
> -
> -        /* Must be done with interrupts enabled */
> -        vpmu_switch_to(next);
> -        np2m_schedule(NP2M_SCHEDLE_IN);
> -    }
> +    _update_runstate_area(next);
> +    /* Must be done with interrupts enabled */
> +    vpmu_switch_to(next);
> +    np2m_schedule(NP2M_SCHEDLE_IN);
>   
>       /* Ensure that the vcpu has an up-to-date time base. */
>       update_vcpu_system_time(next);
> 

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel