| Message ID | 20160216181322.27876.60708.stgit@Solace.station (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 16/02/16 18:13, Dario Faggioli wrote: > so the trace will show properly decoded info, > rather than just a bunch of hex codes. > > Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com> > Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > --- > Cc: George Dunlap <george.dunlap@eu.citrix.com> > Cc: Meng Xu <xumengpanda@gmail.com> > Cc: Tianyang Chen <tiche@seas.upenn.edu> > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > Cc: Ian Campbell <ian.campbell@citrix.com> > Cc: Wei Liu <wei.liu2@citrix.com> > Cc: Olaf Hering <olaf@aepfle.de> > --- > Changes from v1: > * '} * r =' turned into '} *r =', as requested > during review. > --- > tools/xentrace/xenalyze.c | 59 +++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 59 insertions(+) > > diff --git a/tools/xentrace/xenalyze.c b/tools/xentrace/xenalyze.c > index 8f97f3a..dd21229 100644 > --- a/tools/xentrace/xenalyze.c > +++ b/tools/xentrace/xenalyze.c > @@ -7828,6 +7828,65 @@ void sched_process(struct pcpu_info *p) > r->rq_avgload, r->b_avgload); > } > break; > + /* RTDS (TRC_RTDS_xxx) */ > + case TRC_SCHED_CLASS_EVT(RTDS, 1): /* TICKLE */ > + if(opt.dump_all) { > + struct { > + unsigned int cpu:16; > + } *r = (typeof(r))ri->d; > + > + printf(" %s rtds:runq_tickle cpu %u\n", > + ri->dump_header, r->cpu); > + } > + break; > + case TRC_SCHED_CLASS_EVT(RTDS, 2): /* RUNQ_PICK */ > + if(opt.dump_all) { > + struct { > + unsigned int vcpuid:16, domid:16; > + unsigned int cur_dl_lo, cur_dl_hi; > + unsigned int cur_bg_lo, cur_bg_hi; > + } *r = (typeof(r))ri->d; > + uint64_t dl = (((uint64_t)r->cur_dl_hi) << 32) + r->cur_dl_lo; > + uint64_t bg = (((uint64_t)r->cur_bg_hi) << 32) + r->cur_bg_lo; Why are you doing this, instead of just using uint64_t? -George
On Thu, 2016-02-18 at 15:28 +0000, George Dunlap wrote: > On 16/02/16 18:13, Dario Faggioli wrote: > > --- > > tools/xentrace/xenalyze.c | 59 > > +++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 59 insertions(+) > > > > diff --git a/tools/xentrace/xenalyze.c b/tools/xentrace/xenalyze.c > > index 8f97f3a..dd21229 100644 > > --- a/tools/xentrace/xenalyze.c > > +++ b/tools/xentrace/xenalyze.c > > @@ -7828,6 +7828,65 @@ void sched_process(struct pcpu_info *p) > > r->rq_avgload, r->b_avgload); > > } > > break; > > + /* RTDS (TRC_RTDS_xxx) */ > > + case TRC_SCHED_CLASS_EVT(RTDS, 1): /* TICKLE */ > > + if(opt.dump_all) { > > + struct { > > + unsigned int cpu:16; > > + } *r = (typeof(r))ri->d; > > + > > + printf(" %s rtds:runq_tickle cpu %u\n", > > + ri->dump_header, r->cpu); > > + } > > + break; > > + case TRC_SCHED_CLASS_EVT(RTDS, 2): /* RUNQ_PICK */ > > + if(opt.dump_all) { > > + struct { > > + unsigned int vcpuid:16, domid:16; > > + unsigned int cur_dl_lo, cur_dl_hi; > > + unsigned int cur_bg_lo, cur_bg_hi; > > + } *r = (typeof(r))ri->d; > > + uint64_t dl = (((uint64_t)r->cur_dl_hi) << 32) + > > r->cur_dl_lo; > > + uint64_t bg = (((uint64_t)r->cur_bg_hi) << 32) + > > r->cur_bg_lo; > > Why are you doing this, instead of just using uint64_t? > It was to make the struct in sched_rt.c and here exactly match, for ease of someone reading both the pieces of code at the same time, to understand what's being printed. However, yes, using uint64_t is probably equally understandable, and more readable in case one only look at this code, so I can change this (and resend). Regards, Dario -- <<This happens because I choose it to happen!>> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
On 18/02/16 17:02, Dario Faggioli wrote: > On Thu, 2016-02-18 at 15:28 +0000, George Dunlap wrote: >> On 16/02/16 18:13, Dario Faggioli wrote: >>> --- >>> tools/xentrace/xenalyze.c | 59 >>> +++++++++++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 59 insertions(+) >>> >>> diff --git a/tools/xentrace/xenalyze.c b/tools/xentrace/xenalyze.c >>> index 8f97f3a..dd21229 100644 >>> --- a/tools/xentrace/xenalyze.c >>> +++ b/tools/xentrace/xenalyze.c >>> @@ -7828,6 +7828,65 @@ void sched_process(struct pcpu_info *p) >>> r->rq_avgload, r->b_avgload); >>> } >>> break; >>> + /* RTDS (TRC_RTDS_xxx) */ >>> + case TRC_SCHED_CLASS_EVT(RTDS, 1): /* TICKLE */ >>> + if(opt.dump_all) { >>> + struct { >>> + unsigned int cpu:16; >>> + } *r = (typeof(r))ri->d; >>> + >>> + printf(" %s rtds:runq_tickle cpu %u\n", >>> + ri->dump_header, r->cpu); >>> + } >>> + break; >>> + case TRC_SCHED_CLASS_EVT(RTDS, 2): /* RUNQ_PICK */ >>> + if(opt.dump_all) { >>> + struct { >>> + unsigned int vcpuid:16, domid:16; >>> + unsigned int cur_dl_lo, cur_dl_hi; >>> + unsigned int cur_bg_lo, cur_bg_hi; >>> + } *r = (typeof(r))ri->d; >>> + uint64_t dl = (((uint64_t)r->cur_dl_hi) << 32) + >>> r->cur_dl_lo; >>> + uint64_t bg = (((uint64_t)r->cur_bg_hi) << 32) + >>> r->cur_bg_lo; >> >> Why are you doing this, instead of just using uint64_t? >> > It was to make the struct in sched_rt.c and here exactly match, for > ease of someone reading both the pieces of code at the same time, to > understand what's being printed. > > However, yes, using uint64_t is probably equally understandable, and > more readable in case one only look at this code, so I can change this > (and resend). Hrm, well perhaps having the struct match exactly is better. I think most of these patches can be checked in now. What about checking in the other patches, then sending a follow-up series with the struct changed in the scheduler, and then this patch with the resulting changes? -George
On Thu, 2016-02-18 at 17:06 +0000, George Dunlap wrote: > On 18/02/16 17:02, Dario Faggioli wrote: > > On Thu, 2016-02-18 at 15:28 +0000, George Dunlap wrote: > > > > > > > + case TRC_SCHED_CLASS_EVT(RTDS, 2): /* > > > > RUNQ_PICK */ > > > > + if(opt.dump_all) { > > > > + struct { > > > > + unsigned int vcpuid:16, domid:16; > > > > + unsigned int cur_dl_lo, cur_dl_hi; > > > > + unsigned int cur_bg_lo, cur_bg_hi; > > > > + } *r = (typeof(r))ri->d; > > > > + uint64_t dl = (((uint64_t)r->cur_dl_hi) << 32) > > > > + > > > > r->cur_dl_lo; > > > > + uint64_t bg = (((uint64_t)r->cur_bg_hi) << 32) > > > > + > > > > r->cur_bg_lo; > > > > > > Why are you doing this, instead of just using uint64_t? > > > > > It was to make the struct in sched_rt.c and here exactly match, for > > ease of someone reading both the pieces of code at the same time, > > to > > understand what's being printed. > > > > However, yes, using uint64_t is probably equally understandable, > > and > > more readable in case one only look at this code, so I can change > > this > > (and resend). > > Hrm, well perhaps having the struct match exactly is better. > > I think most of these patches can be checked in now. What about > checking in the other patches, then sending a follow-up series with > the > struct changed in the scheduler, and then this patch with the > resulting > changes? > This would work for me. Regards, Dario -- <<This happens because I choose it to happen!>> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
diff --git a/tools/xentrace/xenalyze.c b/tools/xentrace/xenalyze.c index 8f97f3a..dd21229 100644 --- a/tools/xentrace/xenalyze.c +++ b/tools/xentrace/xenalyze.c @@ -7828,6 +7828,65 @@ void sched_process(struct pcpu_info *p) r->rq_avgload, r->b_avgload); } break; + /* RTDS (TRC_RTDS_xxx) */ + case TRC_SCHED_CLASS_EVT(RTDS, 1): /* TICKLE */ + if(opt.dump_all) { + struct { + unsigned int cpu:16; + } *r = (typeof(r))ri->d; + + printf(" %s rtds:runq_tickle cpu %u\n", + ri->dump_header, r->cpu); + } + break; + case TRC_SCHED_CLASS_EVT(RTDS, 2): /* RUNQ_PICK */ + if(opt.dump_all) { + struct { + unsigned int vcpuid:16, domid:16; + unsigned int cur_dl_lo, cur_dl_hi; + unsigned int cur_bg_lo, cur_bg_hi; + } *r = (typeof(r))ri->d; + uint64_t dl = (((uint64_t)r->cur_dl_hi) << 32) + r->cur_dl_lo; + uint64_t bg = (((uint64_t)r->cur_bg_hi) << 32) + r->cur_bg_lo; + + printf(" %s rtds:runq_pick d%uv%u, deadline = %"PRIu64", " + "budget = %"PRIu64"\n", ri->dump_header, + r->domid, r->vcpuid, dl, bg); + } + break; + case TRC_SCHED_CLASS_EVT(RTDS, 3): /* BUDGET_BURN */ + if(opt.dump_all) { + struct { + unsigned int vcpuid:16, domid:16; + unsigned int cur_bg_lo, cur_bg_hi; + int delta; + } *r = (typeof(r))ri->d; + uint64_t bg = (((uint64_t)r->cur_bg_hi) << 32) + r->cur_bg_lo; + + printf(" %s rtds:burn_budget d%uv%u, budget = %"PRIu64", " + "delta = %d\n", ri->dump_header, r->domid, + r->vcpuid, bg, r->delta); + } + break; + case TRC_SCHED_CLASS_EVT(RTDS, 4): /* BUDGET_REPLENISH */ + if(opt.dump_all) { + struct { + unsigned int vcpuid:16, domid:16; + unsigned int cur_dl_lo, cur_dl_hi; + unsigned int cur_bg_lo, cur_bg_hi; + } *r = (typeof(r))ri->d; + uint64_t dl = (((uint64_t)r->cur_dl_hi) << 32) + r->cur_dl_lo; + uint64_t bg = (((uint64_t)r->cur_bg_hi) << 32) + r->cur_bg_lo; + + printf(" %s rtds:repl_budget d%uv%u, deadline = %"PRIu64", " + "budget = %"PRIu64"\n", ri->dump_header, + r->domid, r->vcpuid, dl, bg); + } + break; + case TRC_SCHED_CLASS_EVT(RTDS, 5): /* SCHED_TASKLET */ + if(opt.dump_all) + printf(" %s rtds:sched_tasklet\n", ri->dump_header); + break; default: process_generic(ri); }