On Mon, 2016-03-21 at 09:07 -0600, Jan Beulich wrote:
> > > > On 21.03.16 at 15:48, <JGross@suse.com> wrote:
> > On 18/03/16 20:04, Dario Faggioli wrote:
> > > @@ -1491,9 +1491,9 @@ static int cpu_schedule_up(unsigned int
> > > cpu)
> > >      if ( idle_vcpu[cpu] == NULL )
> > >          return -ENOMEM;
> > >  
> > > -    if ( (ops.alloc_pdata != NULL) &&
> > > -         ((sd->sched_priv = ops.alloc_pdata(&ops, cpu)) == NULL)
> > > )
> > > -        return -ENOMEM;
> > > +    sd->sched_priv = SCHED_OP(&ops, alloc_pdata, cpu);
> > > +    if ( IS_ERR(sd->sched_priv) )
> > > +        return PTR_ERR(sd->sched_priv);
> > Calling xfree() with an IS_ERR() value might be a bad idea.
> > Either you need to set sd->sched_priv to NULL in error case or you
> > modify xfree() to return immediately not only in the NULL case, but
> > in the IS_ERR() case as well.
> The latter option is a no-go imo.
> 
Ok, I'll do:

    sd->sched_priv = SCHED_OP(&ops, alloc_pdata, cpu);
    if ( IS_ERR(sd->sched_priv) )
    {
        int err = PTR_ERR(sd->sched_priv);

        sd->sched_priv = NULL;
        return err;
    }

Is this ok?

And, just to be sure I got your point (Juergen), you're referring to
the situation where cpu_schedule_up() fails, we go to CPU_UP_CANCELLED,
which calls cpu_schedule_donw(), which calls free_pdata on
sd->sched_priv (inside which we may reach an xfree()), aren't you?

In fact, alloc_pdata is also called in schedule_cpu_switch(), but in
that case, I don't see anyone calling xfree() if alloc_pdata fails...
Am I missing it?

Thanks and Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)