Re: [Xen-devel] [PATCH v2 30/48] xen/sched: introduce unit_runnable_state()

[Juergen Gross <jgross@xxxxxxxx>]

On 11.09.19 12:30, Jan Beulich wrote:
> On 09.08.2019 16:58, Juergen Gross wrote:
> > --- a/xen/common/schedule.c
> > +++ b/xen/common/schedule.c
> > @@ -266,7 +266,7 @@ static inline void sched_unit_runstate_change(struct 
> > sched_unit *unit,
> >       struct vcpu *v = unit->vcpu_list;
> >   
> >       if ( running )
> > -        vcpu_runstate_change(v, RUNSTATE_running, new_entry_time);
> > +        vcpu_runstate_change(v, v->new_state, new_entry_time);
> Strictly speaking this is wrong when there's no actual state
> change, as the state entry time then shouldn't change. Quite
> possibly this would be merely a cosmetic issue though.
This will be changed in vcpu_runstate_change() with patch 31 when this
situation is actually possible. With only one vcpu in a unit the state
will always change here, while after the next patch
vcpu_runstate_change() will return early in case the state isn't
changing.

> > --- a/xen/include/xen/sched-if.h
> > +++ b/xen/include/xen/sched-if.h
> > @@ -75,6 +75,20 @@ static inline bool unit_runnable(const struct sched_unit 
> > *unit)
> >       return vcpu_runnable(unit->vcpu_list);
> >   }
> >   
> > +static inline bool unit_runnable_state(const struct sched_unit *unit)
> > +{
> > +    struct vcpu *v;
> > +    bool runnable;
> > +
> > +    v = unit->vcpu_list;
> > +    runnable = vcpu_runnable(v);
> > +
> > +    v->new_state = runnable ? RUNSTATE_running
> > +                            : (v->pause_flags & VPF_blocked)
> > +                              ? RUNSTATE_blocked : RUNSTATE_offline;
> > +    return runnable;
> > +}
> Especially for understanding the (correctness of the) credit1
> changes it would be rather helpful if once again this function
> actually iterated over all vCPU-s right away (even if there's
> only one per unit right now), to see how their varying states
> get combined.
Okay, will move it.

> > --- a/xen/include/xen/sched.h
> > +++ b/xen/include/xen/sched.h
> > @@ -174,6 +174,7 @@ struct vcpu
> >           XEN_GUEST_HANDLE(vcpu_runstate_info_compat_t) compat;
> >       } runstate_guest; /* guest address */
> >   #endif
> > +    int              new_state;
> I realize its counterpart (wrongly) is plain int in the public
> interface - I think it should be unsigned int here and uint32_t
> there. I'm pondering whether to do a swipe across all public
> headers to replace all uses of plain int (and alike) with
> fixed width types.
The list for cleanups is becoming longer...

So are you fine with me not changing anything in this regard right now?


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel