Re: [Xen-devel] [PATCH] xen: sched: improve debug dump output.

[Meng Xu <mengxu@xxxxxxxxxxxxx>]

>
> > > TBH, what I don't especially like in the output above is, within
> > > the
> > > vCPU info being printed:
> > >  - the spaces inside '[' ']';
> > >  - the big numbers;
> > >  - the fact that last_start is rather useless (it's more tracing
> > > info
> > >    than debug dump info, IMO);
> >
> > I feel the last_start is useful, at least to identify the previous
> > subtle bug in budget accounting. It tells us when the running VCPU
> > was
> > scheduled and indicates how the budget will be burned later.
> > When I saw the last_start is in the previous period and the
> > burn_budget() still use the old last_start to burn budget for the
> > current period, I figured out the bug.
> >
> That's ok, we're all different... That's what makes the World so
> beautiful. :-)
>
> > >  - the fact that the various queues info and CPUs info are not
> > >    displaed closer, and they even have "Domain info:" in between
> > > them
> > >    (which is because of schedule.c, not sched_rt.c, I know);
> > >  - the word "info" after "Global RunQueue", "Global DepletedQueue",
> > >    "Global Replenishment Events";
> > >  - the word "Global", in the names above;
> > >  - the onQ and runnable flags being printed, which I don't is
> > > really
> > >    necessary or useful;
> > >  - the lack of scheduler wide information (e.g., the tickled mask,
> > > the
> > >    next timeout of the replenishment timer, etc);
> > >
> > > But this is material for another patch. :-)
> >
> > I agree with all of the above output improvements, except for killing
> > the last_start info. :-)
> >
> Ok. I'm not yet working on a patch that does remove it. If/when I will
> and send it, you're more than entitled, and have the necessary power,
> to Nack it. ;-P


OK. Once this serials of patch gets in, I can send one patch to fix this output.
>
>
> > > Going back to printing "idle" or not, also remember that this is
> > > debug
> > > output, meant at being mostly useful for developers, or with help
> > > from
> > > developers. And developers can easily check in the code what having
> > > just the CPU ID printed means (in case it's not obvious, which I
> > > think
> > > it is, or they don't remember).
> > >
> > > That being said, it's not that I can't live with the added "idle"
> > > indication. But I like it less and would prefer not to add it.
> >
> > Sure! I was thinking if we should even avoid printing out the idle
> > CPU
> > number to make the output more concise on an idle systems.
> >
> Yeah, that was also my first doing. But, then, looking at the output, I
> found it a little bit too obfuscated the info about what pCPUs are
> actually in use within the scheduler (think of the case where it's not
> default, and is in a cpupool).
>
> So I ended up deciding to leave it there, all of them, idle or not.
>
> > After seeing the complete output, I think the current output is fine.
> >
> Great! So, you'll send your Acked-by soon?


Yep. I'm replying to your original email.

Meng
-----------
Meng Xu
PhD Student in Computer and Information Science
University of Pennsylvania
http://www.cis.upenn.edu/~mengxu/

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