Re: [Xen-devel] [PATCH v3] schedule: move last_run_time to the credit scheduler privates

[Dario Faggioli <dfaggioli@xxxxxxxx>]

On Mon, 2019-06-10 at 18:16 +0300, Andrii Anisov wrote:
> On 31.05.19 16:24, Dario Faggioli wrote:
> > --- a/xen/common/sched_credit.c
> > +++ b/xen/common/sched_credit.c
> > @@ -175,6 +175,9 @@ struct csched_vcpu {
> >       atomic_t credit;
> >       unsigned int residual;
> >   
> > +    /* last time when vCPU is scheduled out */
> > +    s_time_t last_run_time;
> > +
> >   #ifdef CSCHED_STATS
> >       struct {
> >           int credit_last;
> > 
> > The comment is not accurate. And I'm afraid that it could be
> > misleading
> > for someone reading it, but then realizing that the code does
> > something
> > slightly different, and hence not being able to tell which one of
> > the
> > two things is correct.
> 
> Well, I copy-pasted that. And was wrong here. Me actually against the
> text comments inlined into the code. It happens that code changes
> faster than comments and in result comments become misleading very
> often.
> I'd rather drop the comment at all.
> 
Well, in general, I've mixed feelings on the subject.

About this specific case, if we find a suitable new name for the field,
I agree that the comment may very well become pretty useless.

> > So, either we change the comment (but I'm not capable, right now,
> > of
> > finding something that is short and, at the same time, clear
> > enough),
> > or we change how the variable is using.
> 
> As per the current code I'd rename the member to `last_sched_time`.
> To reflect that it is the time when the vcpu went through scheduling
> path.
> 
Ok, yes, this is something I personally can live with. :-)

Regards
-- 
Dario Faggioli, Ph.D
http://about.me/dario.faggioli
Virtualization Software Engineer
SUSE Labs, SUSE https://www.suse.com/
-------------------------------------------------------------------
<<This happens because _I_ choose it to happen!>> (Raistlin Majere)

Attachment: signature.asc
Description: This is a digitally signed message part

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