Re: [PATCH v2 2/2] xen/sched: core: kill unarmed timers on sched_init_vcpu() failure

[Jan Beulich <jbeulich@xxxxxxxx>]

On 19.08.2026 12:39, Furkan Çalışkan wrote:
> On 8/19/26 10:22, Jan Beulich wrote:
>> On 19.08.2026 07:15, Furkan Caliskan wrote:
>>> --- a/xen/common/sched/core.c
>>> +++ b/xen/common/sched/core.c
>>> @@ -589,6 +589,9 @@ int sched_init_vcpu(struct vcpu *v)
>>>      unit->priv = sched_alloc_udata(dom_scheduler(d), unit, d->sched_priv);
>>>      if ( unit->priv == NULL )
>>>      {
>>> +        kill_timer(&v->periodic_timer);
>>> +        kill_timer(&v->singleshot_timer);
>>> +        kill_timer(&v->poll_timer);
>>>          sched_free_unit(unit, v);
>>>          rcu_read_unlock(&sched_res_rculock);
>>>          return 1;
>>
>> This almost, but not quite open-codes sched_destroy_vcpu(). Would be nice
>> if the cleanup logic was shared. The sched_free_unit() call there could be
>> leveraged here as well; what would need skipping are the sched_free_udata()
>> and sched_remove_unit(). And of course the RCU-locking would need sorting.
> 
> Would something like below be okay?

Maybe, but you need to ask the maintainers of this code, which I'm not a part
of. What I in particular can't easily judge is whether ...

> --- a/xen/common/sched/core.c
> +++ b/xen/common/sched/core.c
> @@ -589,8 +589,8 @@ int sched_init_vcpu(struct vcpu *v)
>      unit->priv = sched_alloc_udata(dom_scheduler(d), unit, d->sched_priv);
>      if ( unit->priv == NULL )
>      {
> -        sched_free_unit(unit, v);
>          rcu_read_unlock(&sched_res_rculock);
> +        sched_destroy_vcpu(v);
>          return 1;
>      }

... this intermediate dropping of the lock is entirely okay (it looks to be
at the first glance).

Jan

> @@ -869,8 +869,11 @@ void sched_destroy_vcpu(struct vcpu *v)
>      {
>          rcu_read_lock(&sched_res_rculock);
>  
> -        sched_remove_unit(vcpu_scheduler(v), unit);
> -        sched_free_udata(vcpu_scheduler(v), unit->priv);
> +        if ( unit->priv )
> +        {
> +            sched_remove_unit(vcpu_scheduler(v), unit);
> +            sched_free_udata(vcpu_scheduler(v), unit->priv);
> +        }
>          sched_free_unit(unit, v);
>  
>          rcu_read_unlock(&sched_res_rculock);
> 
> 
> Furkan