From mboxrd@z Thu Jan 1 00:00:00 1970 From: Juergen Gross Subject: Re: [PATCH 5/9] xen: sched: make implementing .alloc_pdata optional Date: Thu, 1 Oct 2015 07:28:13 +0200 Message-ID: <560CC46D.8050002@suse.com> References: <20150929164726.17589.96920.stgit@Solace.station> <20150929165604.17589.24387.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.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZhWPh-0000Uo-14 for xen-devel@lists.xenproject.org; Thu, 01 Oct 2015 05:28:17 +0000 In-Reply-To: <20150929165604.17589.24387.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 , Josh Whitehead , Robert VanVossen List-Id: xen-devel@lists.xenproject.org On 09/29/2015 06:56 PM, Dario Faggioli wrote: > The .alloc_pdata hook of the scheduler interface is > called, in schedule_cpu_switch(), unconditionally, > for all schedulers. > > This forces even schedulers that do not require any > actual allocation of per-pCPU data to: > - contain an implementation of the hook; > - return some artificial non-NULL value to make > such a caller happy. > > This changes allows schedulers that do not need per-pCPU > allocations to avoid implementing the hook. It also > kills such artificial implementation from the ARINC653 > scheduler (and, while there, it nukes .free_pdata from > there too, which is equally useless). > > Signed-off-by: Dario Faggioli > --- > Cc: George Dunlap > Cc: Robert VanVossen > Cc: Josh Whitehead > --- > xen/common/sched_arinc653.c | 31 ------------------------------- > xen/common/schedule.c | 6 +++--- > 2 files changed, 3 insertions(+), 34 deletions(-) > > diff --git a/xen/common/sched_arinc653.c b/xen/common/sched_arinc653.c > index cff5da9..ef192c3 100644 > --- a/xen/common/sched_arinc653.c > +++ b/xen/common/sched_arinc653.c > @@ -455,34 +455,6 @@ a653sched_free_vdata(const struct scheduler *ops, void *priv) > } > > /** > - * This function allocates scheduler-specific data for a physical CPU > - * > - * We do not actually make use of any per-CPU data but the hypervisor expects > - * a non-NULL return value > - * > - * @param ops Pointer to this instance of the scheduler structure > - * > - * @return Pointer to the allocated data > - */ > -static void * > -a653sched_alloc_pdata(const struct scheduler *ops, int cpu) > -{ > - /* return a non-NULL value to keep schedule.c happy */ > - return SCHED_PRIV(ops); > -} > - > -/** > - * This function frees scheduler-specific data for a physical CPU > - * > - * @param ops Pointer to this instance of the scheduler structure > - */ > -static void > -a653sched_free_pdata(const struct scheduler *ops, void *pcpu, int cpu) > -{ > - /* nop */ > -} > - > -/** > * This function allocates scheduler-specific data for a domain > * > * We do not actually make use of any per-domain data but the hypervisor > @@ -736,9 +708,6 @@ const struct scheduler sched_arinc653_def = { > .free_vdata = a653sched_free_vdata, > .alloc_vdata = a653sched_alloc_vdata, > > - .free_pdata = a653sched_free_pdata, > - .alloc_pdata = a653sched_alloc_pdata, > - > .free_domdata = a653sched_free_domdata, > .alloc_domdata = a653sched_alloc_domdata, > > diff --git a/xen/common/schedule.c b/xen/common/schedule.c > index 83244d7..0e02af2 100644 > --- a/xen/common/schedule.c > +++ b/xen/common/schedule.c > @@ -1493,7 +1493,7 @@ void __init scheduler_init(void) > int schedule_cpu_switch(unsigned int cpu, struct cpupool *c) > { > struct vcpu *idle; > - void *ppriv, *ppriv_old, *vpriv, *vpriv_old; > + void *ppriv = NULL, *ppriv_old, *vpriv, *vpriv_old; > struct scheduler *old_ops = per_cpu(scheduler, cpu); > struct scheduler *new_ops = (c == NULL) ? &ops : c->sched; > > @@ -1501,8 +1501,8 @@ int schedule_cpu_switch(unsigned int cpu, struct cpupool *c) > return 0; > > idle = idle_vcpu[cpu]; > - ppriv = SCHED_OP(new_ops, alloc_pdata, cpu); > - if ( ppriv == NULL ) > + if ( (new_ops->alloc_pdata != NULL) && > + ((ppriv = new_ops->alloc_pdata(new_ops, cpu)) == NULL) ) > return -ENOMEM; > vpriv = SCHED_OP(new_ops, alloc_vdata, idle, idle->domain->sched_priv); > if ( vpriv == NULL ) Just below this there are 2 SCHED_OP calls to free_pdata. You'll have to check whether the hook is present as you just have nuked it above. Juergen