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

[Jürgen Groß <jgross@xxxxxxxx>]

On 19.08.26 12:43, Jan Beulich wrote:
> 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).
Why don't you just replace sched_free_unit() with sched_destroy_vcpu()?

rcu_read_lock() calls can be nested, even for the same lock. This works because
there is no rcu_write_lock() (see comment in rcupdate.h).


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature