Re: [PATCH v2 15/16] xenalyze: handle RTDS scheduler events

[George Dunlap <george.dunlap@citrix.com>]

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