Re: [Xen-devel] [PATCH 03/16] xen: sched: make implementing .alloc_pdata optional

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 01.04.16 at 19:01, <dario.faggioli@xxxxxxxxxx> wrote:
> On Mon, 2016-03-21 at 09:07 -0600, Jan Beulich wrote:
>> > > > On 21.03.16 at 15:48, <JGross@xxxxxxxx> 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?

Depends: Can ->sched_priv be looked at by another CPU between
the first and second assignments? If yes, you need to use an
intermediary local variable.

Jan


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