From mboxrd@z Thu Jan 1 00:00:00 1970 From: Juergen Gross Subject: Re: [PATCH 4/9] xen: sched: add .init_pdata hook to the scheduler interface Date: Thu, 1 Oct 2015 07:21:48 +0200 Message-ID: <560CC2EC.6030801@suse.com> References: <20150929164726.17589.96920.stgit@Solace.station> <20150929165556.17589.62924.stgit@Solace.station> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZhWJT-0007a0-UJ for xen-devel@lists.xenproject.org; Thu, 01 Oct 2015 05:21:52 +0000 In-Reply-To: <20150929165556.17589.62924.stgit@Solace.station> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Dario Faggioli , xen-devel@lists.xenproject.org Cc: George Dunlap List-Id: xen-devel@lists.xenproject.org On 09/29/2015 06:55 PM, Dario Faggioli wrote: > with the purpose of decoupling the allocation phase and > the initialization one, for per-pCPU data of the schedulers. > > This makes it possible to perform the initialization later > in the pCPU bringup/assignement process, when more information > (for instance, the host CPU topology) are available. This, > for now, is important only for Credit2, but it can well be > useful to other schedulers. > > This also fixes a latent bug. In fact, when moving a pCPU > from a Credit2 cpupool to another, whose scheduler also > remaps runqueue spin locks (e.g., RTDS), we experience the > following Oops: > > (XEN) Initializing RTDS scheduler > (XEN) WARNING: This is experimental software in development. > (XEN) Use at your own risk. > (XEN) Removing cpu 6 from runqueue 0 > (XEN) Assertion 'sd->schedule_lock == &rqd->lock' failed at sched_credit2.c:2102 > (XEN) ----[ Xen-4.7-unstable x86_64 debug=y Not tainted ]---- > ... ... ... > (XEN) Xen call trace: > (XEN) [] csched2_free_pdata+0x135/0x180 > (XEN) [] schedule_cpu_switch+0x1ee/0x217 > (XEN) [] cpupool_assign_cpu_locked+0x4d/0x152 > (XEN) [] cpupool_do_sysctl+0x20b/0x69d > (XEN) [] do_sysctl+0x665/0x10f8 > (XEN) [] lstar_enter+0x106/0x160 > > This happens because, right now, the scheduler of the > target pool remaps the runqueue lock during (rt_)alloc_pdata, > which is called at the very beginning of schedule_cpu_switch(). > Credit2's csched2_free_pdata(), however, wants to find the spin > lock the same way it was put by csched2_alloc_pdata(), and > explodes, it not being the case. > > This patch also fixes this as, now, spin lock remapping > happens in the init_pdata hook of the target scheduler for > the pCPU, which we can easily make sure it is ivoked *after* > the free_pdata hook of the old scheduler (which is exactly > what is done in this patch), without needing to restructure > schedule_cpu_switch(). > > Signed-off-by: Dario Faggioli > --- > Cc: George Dunlap > Cc: Juergen Gross > --- > xen/common/schedule.c | 16 +++++++++++++--- > xen/include/xen/sched-if.h | 1 + > 2 files changed, 14 insertions(+), 3 deletions(-) > > diff --git a/xen/common/schedule.c b/xen/common/schedule.c > index 4a89222..83244d7 100644 > --- a/xen/common/schedule.c > +++ b/xen/common/schedule.c > @@ -1407,6 +1407,9 @@ static int cpu_schedule_callback( > > switch ( action ) > { > + case CPU_STARTING: > + SCHED_OP(&ops, init_pdata, cpu); > + break; > case CPU_UP_PREPARE: > rc = cpu_schedule_up(cpu); > break; > @@ -1484,6 +1487,7 @@ void __init scheduler_init(void) > if ( ops.alloc_pdata && > !(this_cpu(schedule_data).sched_priv = ops.alloc_pdata(&ops, 0)) ) > BUG(); > + SCHED_OP(&ops, init_pdata, 0); You can't call this without having it set in all schedulers. I guess using the init_pdata hook has to be introduced after or in patch 5. Juergen > } > > int schedule_cpu_switch(unsigned int cpu, struct cpupool *c) > @@ -1513,12 +1517,18 @@ int schedule_cpu_switch(unsigned int cpu, struct cpupool *c) > per_cpu(scheduler, cpu) = new_ops; > ppriv_old = per_cpu(schedule_data, cpu).sched_priv; > per_cpu(schedule_data, cpu).sched_priv = ppriv; > - SCHED_OP(new_ops, tick_resume, cpu); > - SCHED_OP(new_ops, insert_vcpu, idle); > - > SCHED_OP(old_ops, free_vdata, vpriv_old); > SCHED_OP(old_ops, free_pdata, ppriv_old, cpu); > > + /* > + * Now that the new pcpu data have been allocated and all pointers > + * correctly redirected to them, we can perform all the necessary > + * initializations. > + */ > + SCHED_OP(new_ops, init_pdata, cpu); > + SCHED_OP(new_ops, tick_resume, cpu); > + SCHED_OP(new_ops, insert_vcpu, idle); > + > return 0; > } > > diff --git a/xen/include/xen/sched-if.h b/xen/include/xen/sched-if.h > index dbe7cab..14392f3 100644 > --- a/xen/include/xen/sched-if.h > +++ b/xen/include/xen/sched-if.h > @@ -133,6 +133,7 @@ struct scheduler { > void *); > void (*free_pdata) (const struct scheduler *, void *, int); > void * (*alloc_pdata) (const struct scheduler *, int); > + void (*init_pdata) (const struct scheduler *, int); > void (*free_domdata) (const struct scheduler *, void *); > void * (*alloc_domdata) (const struct scheduler *, struct domain *); > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel >