Re: [PATCH 5/9] xen: sched: make implementing .alloc_pdata optional

[Juergen Gross <jgross@suse.com>]

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 <dario.faggioli@citrix.com>
> ---
> Cc: George Dunlap <george.dunlap@eu.citrix.com>
> Cc: Robert VanVossen <robert.vanvossen@dornerworks.com>
> Cc: Josh Whitehead <josh.whitehead@dornerworks.com>
> ---
>   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