Re: [Xen-devel] [PATCH for Xen 4.5 v3 2/2] xen: serialize vcpu data in sched_rt.c

[George Dunlap <George.Dunlap@xxxxxxxxxxxxx>]

On Mon, Nov 10, 2014 at 9:11 PM, Meng Xu <mengxu@xxxxxxxxxxxxx> wrote:
> Fix the following two issues in rtds scheduler:
> 1) The runq queue lock is not grabbed when rt_update_deadline is
> called in rt_alloc_vdata function, which may cause race condition;
> Solution: Move call to rt_update_deadline from _alloc to _insert;
> Note: rt_alloc_vdata does not need grab the runq lock, because only one
> cpu will allocate the rt_vcpu; before the rt_vcpu is inserted into the
> runq, no more than one cpu operates on the rt_vcpu.
>
> 2) rt_vcpu_remove should grab the runq lock before remove the vcpu
> from runq; otherwise, race condition may happen.
> Solution: Add lock in rt_vcpu_remove().
>
> Signed-off-by: Meng Xu <mengxu@xxxxxxxxxxxxx>
> Reviewed-by: Dario Faggioli <dario.faggioli@xxxxxxxxxx>

Great, thanks Meng!

Reviewed-by: George Dunlap <george.dunlap@xxxxxxxxxxxxx>



> ---
>  xen/common/sched_rt.c |   12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/xen/common/sched_rt.c b/xen/common/sched_rt.c
> index 8251e41..e70d6c7 100644
> --- a/xen/common/sched_rt.c
> +++ b/xen/common/sched_rt.c
> @@ -508,7 +508,6 @@ static void *
>  rt_alloc_vdata(const struct scheduler *ops, struct vcpu *vc, void *dd)
>  {
>      struct rt_vcpu *svc;
> -    s_time_t now = NOW();
>
>      /* Allocate per-VCPU info */
>      svc = xzalloc(struct rt_vcpu);
> @@ -526,10 +525,6 @@ rt_alloc_vdata(const struct scheduler *ops, struct vcpu 
> *vc, void *dd)
>      if ( !is_idle_vcpu(vc) )
>          svc->budget = RTDS_DEFAULT_BUDGET;
>
> -    ASSERT( now >= svc->cur_deadline );
> -
> -    rt_update_deadline(now, svc);
> -
>      return svc;
>  }
>
> @@ -552,11 +547,15 @@ static void
>  rt_vcpu_insert(const struct scheduler *ops, struct vcpu *vc)
>  {
>      struct rt_vcpu *svc = rt_vcpu(vc);
> +    s_time_t now = NOW();
>
>      /* not addlocate idle vcpu to dom vcpu list */
>      if ( is_idle_vcpu(vc) )
>          return;
>
> +    if ( now >= svc->cur_deadline )
> +        rt_update_deadline(now, svc);
> +
>      if ( !__vcpu_on_q(svc) && vcpu_runnable(vc) && !vc->is_running )
>          __runq_insert(ops, svc);
>
> @@ -573,11 +572,14 @@ rt_vcpu_remove(const struct scheduler *ops, struct vcpu 
> *vc)
>  {
>      struct rt_vcpu * const svc = rt_vcpu(vc);
>      struct rt_dom * const sdom = svc->sdom;
> +    spinlock_t *lock;
>
>      BUG_ON( sdom == NULL );
>
> +    lock = vcpu_schedule_lock_irq(vc);
>      if ( __vcpu_on_q(svc) )
>          __q_remove(svc);
> +    vcpu_schedule_unlock_irq(lock, vc);
>
>      if ( !is_idle_vcpu(vc) )
>          list_del_init(&svc->sdom_elem);
> --
> 1.7.9.5
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel