Scheduling related tracing improvements

as it is always acts on v->processor of the vcpu that we are tickling.

Getting rid of it makes the code easier to understand and better looking.

While there, remove a spurious blank line.

Signed-off-by Dario Faggioli <[email protected]>

Reviewed-by Konrad Rzeszutek Wilk <[email protected]>

---
Cc: George Dunlap <[email protected]>
---
Changes from v1:
* fix wording inside the changelog, as suggested during review.

---
 xen/common/sched_credit.c |   10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

xen/common/sched_credit.c +4 -6

index 671bbee..5279b92 100644
@@ -360,9 +360,9 @@ boolean_param("tickle_one_idle_cpu", opt_tickle_one_idle);
 
 DEFINE_PER_CPU(unsigned int, last_tickle_cpu);
 
-static inline void
-__runq_tickle(unsigned int cpu, struct csched_vcpu *new)
+static inline void __runq_tickle(struct csched_vcpu *new)
 {
+    unsigned int cpu = new->vcpu->processor;
     struct csched_vcpu * const cur = CSCHED_VCPU(curr_on_cpu(cpu));
     struct csched_private *prv = CSCHED_PRIV(per_cpu(scheduler, cpu));
     cpumask_t mask, idle_mask, *online;

@@ -375,7 +375,6 @@ __runq_tickle(unsigned int cpu, struct csched_vcpu *new)
     cpumask_and(&idle_mask, prv->idlers, online);
     idlers_empty = cpumask_empty(&idle_mask);
 
-
     /*
      * If the pcpu is idle, or there are no idlers and the new
      * vcpu is a higher priority than the old vcpu, run it here.

@@ -980,11 +979,10 @@ static void
 csched_vcpu_wake(const struct scheduler *ops, struct vcpu *vc)
 {
     struct csched_vcpu * const svc = CSCHED_VCPU(vc);
-    const unsigned int cpu = vc->processor;
 
     BUG_ON( is_idle_vcpu(vc) );
 
-    if ( unlikely(curr_on_cpu(cpu) == vc) )
+    if ( unlikely(curr_on_cpu(vc->processor) == vc) )
     {
         SCHED_STAT_CRANK(vcpu_wake_running);
         return;

@@ -1028,7 +1026,7 @@ csched_vcpu_wake(const struct scheduler *ops, struct vcpu *vc)
 
     /* Put the VCPU on the runq and tickle CPUs */
     __runq_insert(svc);
-    __runq_tickle(cpu, svc);
+    __runq_tickle(svc);
 }
 
 static void

vcpu_wake() and vcpu_sleep() are called before the specific schedulers wakeup and sleep routines (in fact, it is them that calls those specific routine).

Make the trace reflect that, by moving the records up. In fact, it is more natural and easy to find the record of the event (e.g., the wakeup) *before* the records of the actions that deals with the event itself.

Signed-off-by Dario Faggioli <[email protected]>

Reviewed-by Konrad Rzeszutek Wilk <[email protected]>

---
Cc: George Dunlap <[email protected]>

---
 xen/common/schedule.c |   16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

xen/common/schedule.c +10 -6

index 7306d71..c87922f 100644
@@ -381,7 +381,11 @@ void sched_destroy_domain(struct domain *d)
 void vcpu_sleep_nosync(struct vcpu *v)
 {
     unsigned long flags;
-    spinlock_t *lock = vcpu_schedule_lock_irqsave(v, &flags);
+    spinlock_t *lock;
+
+    TRACE_2D(TRC_SCHED_SLEEP, v->domain->domain_id, v->vcpu_id);
+
+    lock = vcpu_schedule_lock_irqsave(v, &flags);
 
     if ( likely(!vcpu_runnable(v)) )
     {

@@ -392,8 +396,6 @@ void vcpu_sleep_nosync(struct vcpu *v)
     }
 
     vcpu_schedule_unlock_irqrestore(lock, flags, v);
-
-    TRACE_2D(TRC_SCHED_SLEEP, v->domain->domain_id, v->vcpu_id);
 }
 
 void vcpu_sleep_sync(struct vcpu *v)

@@ -409,7 +411,11 @@ void vcpu_sleep_sync(struct vcpu *v)
 void vcpu_wake(struct vcpu *v)
 {
     unsigned long flags;
-    spinlock_t *lock = vcpu_schedule_lock_irqsave(v, &flags);
+    spinlock_t *lock;
+
+    TRACE_2D(TRC_SCHED_WAKE, v->domain->domain_id, v->vcpu_id);
+
+    lock = vcpu_schedule_lock_irqsave(v, &flags);
 
     if ( likely(vcpu_runnable(v)) )
     {

@@ -424,8 +430,6 @@ void vcpu_wake(struct vcpu *v)
     }
 
     vcpu_schedule_unlock_irqrestore(lock, flags, v);
-
-    TRACE_2D(TRC_SCHED_WAKE, v->domain->domain_id, v->vcpu_id);
 }
 
 void vcpu_unblock(struct vcpu *v)

by doing the following two things:

- move TRC_SCHED_DOM_{ADD,REM}, into the functions that do the actual scheduling-related domain initialization;

- add two 'generic' DOM_{ADD,REM} events. They're made part of the TRC_DOM0 tracing class, as Dom0 is, usually, from where domains are created and destroyed.

Signed-off-by Dario Faggioli <[email protected]>

---
Cc: George Dunlap <[email protected]>
Cc: Konrad Rzeszutek Wilk <[email protected]>
---
Changes from v1:
* added generic domain creation and destruction events, as suggested during review.

---
 xen/common/domain.c        |    5 ++++-
 xen/common/schedule.c      |    4 ++--
 xen/include/public/trace.h |    6 ++++++
 3 files changed, 12 insertions(+), 3 deletions(-)

xen/common/domain.c +4 -1

index 425767c..45273d4 100644
@@ -270,6 +270,8 @@ struct domain *domain_create(domid_t domid, unsigned int domcr_flags,
 
     d->domain_id = domid;
 
+    TRACE_1D(TRC_DOM0_DOM_ADD, d->domain_id);
+
     lock_profile_register_struct(LOCKPROF_TYPE_PERDOM, d, domid, "Domain");
 
     if ( (err = xsm_alloc_security_domain(d)) != 0 )

@@ -864,10 +866,11 @@ void domain_destroy(struct domain *d)
     if ( atomic_cmpxchg(&d->refcnt, 0, DOMAIN_DESTROYED) != 0 )
         return;
 
+    TRACE_1D(TRC_DOM0_DOM_REM, d->domain_id);
+
     cpupool_rm_domain(d);
 
     /* Delete from task list and task hashtable. */
-    TRACE_1D(TRC_SCHED_DOM_REM, d->domain_id);
     spin_lock(&domlist_update_lock);
     pd = &domain_list;
     while ( *pd != d ) 

xen/common/schedule.c +2 -2

index c87922f..27695e3 100644
@@ -241,8 +241,6 @@ int sched_init_vcpu(struct vcpu *v, unsigned int processor)
     if ( v->sched_priv == NULL )
         return 1;
 
-    TRACE_2D(TRC_SCHED_DOM_ADD, v->domain->domain_id, v->vcpu_id);
-
     /* Idle VCPUs are scheduled immediately, so don't put them in runqueue. */
     if ( is_idle_domain(d) )
     {

@@ -369,12 +367,14 @@ void sched_destroy_vcpu(struct vcpu *v)
 int sched_init_domain(struct domain *d)
 {
     SCHED_STAT_CRANK(dom_init);
+    TRACE_1D(TRC_SCHED_DOM_ADD, d->domain_id);
     return SCHED_OP(DOM2OP(d), init_domain, d);
 }
 
 void sched_destroy_domain(struct domain *d)
 {
     SCHED_STAT_CRANK(dom_destroy);
+    TRACE_1D(TRC_SCHED_DOM_REM, d->domain_id);
     SCHED_OP(DOM2OP(d), destroy_domain, d);
 }
 

xen/include/public/trace.h +6 -0

index 274f8f6..5ef9c37 100644
@@ -85,6 +85,9 @@
       ((TRC_SCHED_##_c << TRC_SCHED_ID_SHIFT) & TRC_SCHED_ID_MASK) ) + \
     (_e & TRC_SCHED_EVT_MASK) )
 
+/* Trace classes for DOM0 operations */
+#define TRC_DOM0_DOMOPS     0x00041000   /* Domains manipulations */
+
 /* Trace classes for Hardware */
 #define TRC_HW_PM           0x00801000   /* Power management traces */
 #define TRC_HW_IRQ          0x00802000   /* Traces relating to the handling of IRQs */

@@ -113,6 +116,9 @@
 #define TRC_SCHED_SWITCH_INFNEXT (TRC_SCHED_VERBOSE + 15)
 #define TRC_SCHED_SHUTDOWN_CODE  (TRC_SCHED_VERBOSE + 16)
 
+#define TRC_DOM0_DOM_ADD         (TRC_DOM0_DOMOPS + 1)
+#define TRC_DOM0_DOM_REM         (TRC_DOM0_DOMOPS + 2)
+
 #define TRC_MEM_PAGE_GRANT_MAP      (TRC_MEM + 1)
 #define TRC_MEM_PAGE_GRANT_UNMAP    (TRC_MEM + 2)
 #define TRC_MEM_PAGE_GRANT_TRANSFER (TRC_MEM + 3)

Quoted 11 lines · Dario Faggioli

by doing the following two things:

 - move TRC_SCHED_DOM_{ADD,REM}, into the functions that do the actual scheduling-related domain initialization;

 - add two 'generic' DOM_{ADD,REM} events. They're made part of the TRC_DOM0 tracing class, as Dom0 is, usually, from where domains are created and destroyed.

Quoted 1 line

Signed-off-by Dario Faggioli <[email protected]>

Reviewed-by George Dunlap <[email protected]>

Since this changes domain.c, I guess this would need an ack from "The Rest" -- probably either from Jan or Ian, since they're somewhat familiar with this code...?

Quoted 1 line

Quoted 1 line · On 24.02.16 at 12:21, <[email protected]> wrote:

Quoted 14 lines · George Dunlap

Quoted 12 lines · Dario Faggioli

by doing the following two things:

- move TRC_SCHED_DOM_{ADD,REM}, into the functions that do the actual scheduling-related domain initialization;

- add two 'generic' DOM_{ADD,REM} events. They're made part of the TRC_DOM0 tracing class, as Dom0 is, usually, from where domains are created and destroyed.

Signed-off-by Dario Faggioli <[email protected]>

Quoted 6 lines

Reviewed-by George Dunlap <[email protected]>

Since this changes domain.c, I guess this would need an ack from "The Rest" -- probably either from Jan or Ian, since they're somewhat familiar with this code...?

Ping (Ian, Jan)?

Quoted 18 lines

Quoted 1 line

Quoted 1 line · On 24.02.16 at 12:21, <[email protected]> wrote:

Quoted 16 lines · George Dunlap

Quoted 6 lines · Dario Faggioli

by doing the following two things:

 - move TRC_SCHED_DOM_{ADD,REM}, into the functions that do the actual scheduling-related domain initialization;

Quoted 6 lines

 - add two 'generic' DOM_{ADD,REM} events. They're made part of the TRC_DOM0 tracing class, as Dom0 is, usually, from where domains are created and destroyed.

Signed-off-by Dario Faggioli <[email protected]>

Reviewed-by George Dunlap <[email protected]>

Quoted 6 lines

Quoted 5 lines

Since this changes domain.c, I guess this would need an ack from "The
Rest" -- probably either from Jan or Ian, since they're somewhat familiar with this code...?

Ping (Ian, Jan)?

This has even passed the push gate already.

when tracing runstate changes, the vcpu and domain IDs are encoded in the lower and higher, respectively, parts of a 32 bits integer. When decoding a trace with xentrace_format, this makes it possible to display such events like this:

CPU0  833435853624 (+     768)  running_to_runnable [ dom:vcpu = 0x7fff0000 ]
CPU0  833435854416 (+     792)  runnable_to_running [ dom:vcpu = 0x00000007 ]

For consistency, we should do the same when displaying the events coming from the Credit2 scheduler (when using the same tool), and to do that, we need to invert the order in which the fields are being put in the trace struct right now.

Signed-off-by Dario Faggioli <[email protected]>

Reviewed-by Konrad Rzeszutek Wilk <[email protected]>

---
Cc: George Dunlap <[email protected]>
Cc: Olaf Hering <[email protected]>

---
 xen/common/sched_credit2.c |   16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

xen/common/sched_credit2.c +8 -8

index 78220a7..cf40f68 100644
@@ -382,7 +382,7 @@ __update_svc_load(const struct scheduler *ops,
 
     {
         struct {
-            unsigned dom:16,vcpu:16;
+            unsigned vcpu:16, dom:16;
             unsigned v_avgload:32;
         } d;
         d.dom = svc->vcpu->domain->domain_id;

@@ -450,7 +450,7 @@ runq_insert(const struct scheduler *ops, unsigned int cpu, struct csched2_vcpu *
 
     {
         struct {
-            unsigned dom:16,vcpu:16;
+            unsigned vcpu:16, dom:16;
             unsigned pos;
         } d;
         d.dom = svc->vcpu->domain->domain_id;

@@ -536,7 +536,7 @@ runq_tickle(const struct scheduler *ops, unsigned int cpu, struct csched2_vcpu *
 
         /* TRACE */ {
             struct {
-                unsigned dom:16,vcpu:16;
+                unsigned vcpu:16, dom:16;
                 unsigned credit;
             } d;
             d.dom = cur->vcpu->domain->domain_id;

@@ -561,9 +561,9 @@ tickle:
 
     /* TRACE */ {
         struct {
-            unsigned cpu:8;
+            unsigned cpu:16, pad:16;
         } d;
-        d.cpu = ipid;
+        d.cpu = ipid; d.pad = 0;
         trace_var(TRC_CSCHED2_TICKLE, 0,
                   sizeof(d),
                   (unsigned char *)&d);

@@ -634,7 +634,7 @@ static void reset_credit(const struct scheduler *ops, int cpu, s_time_t now,
 
         /* TRACE */ {
             struct {
-                unsigned dom:16,vcpu:16;
+                unsigned vcpu:16, dom:16;
                 unsigned credit_start, credit_end;
                 unsigned multiplier;
             } d;

@@ -683,7 +683,7 @@ void burn_credits(struct csched2_runqueue_data *rqd, struct csched2_vcpu *svc, s
     /* TRACE */
     {
         struct {
-            unsigned dom:16,vcpu:16;
+            unsigned vcpu:16, dom:16;
             unsigned credit;
             int delta;
         } d;

@@ -812,7 +812,7 @@ __runq_assign(struct csched2_vcpu *svc, struct csched2_runqueue_data *rqd)
     /* TRACE */
     {
         struct {
-            unsigned dom:16,vcpu:16;
+            unsigned vcpu:16, dom:16;
             unsigned rqi:16;
         } d;
         d.dom = svc->vcpu->domain->domain_id;

Quoted 11 lines · Dario Faggioli

when tracing runstate changes, the vcpu and domain IDs are encoded in the lower and higher, respectively, parts of a 32 bits integer. When decoding a trace with xentrace_format, this makes it possible to display such events like this:

CPU0  833435853624 (+     768)  running_to_runnable [ dom:vcpu = 0x7fff0000 ]
CPU0  833435854416 (+     792)  runnable_to_running [ dom:vcpu = 0x00000007 ]

Quoted 8 lines

For consistency, we should do the same when displaying the events coming from the Credit2 scheduler (when using the same tool), and to do that, we need to invert the order in which the fields are being put in the trace struct right now.

Signed-off-by Dario Faggioli <[email protected]>

Reviewed-by Konrad Rzeszutek Wilk <[email protected]>

I was going to say, "We should change xentrace_format and xenalyze in lockstep", but it turns out that they don't support these trace records
yet!   I must have some patches to xenalyze in a local branch somewhere
that I never upstreamed properly.

So, all is well:

Acked-by George Dunlap <[email protected]>

when tracing runstate changes, the vcpu and domain IDs are encoded in the lower and higher, respectively, parts of a 32 bits integer. When decoding a trace with xentrace_format, this makes it possible to display such events like this:

CPU0  833435853624 (+     768)  running_to_runnable [ dom:vcpu = 0x7fff0000 ]
CPU0  833435854416 (+     792)  runnable_to_running [ dom:vcpu = 0x00000007 ]

For consistency, we should do the same when displaying the events coming from the RTDS scheduler (when using the same tool), and to do that, we need to invert the order in which the fields are being put in the trace struct right now.

While there, we also:
- fix the use of TRC_RTDS_SCHED_TASKLET (it should only be involved when a tasklet is scheduled, not _every_ time rt_schedule() is invoked!);
- remove a very chatty and useless (nothing has been picked!) use of TRC_RTDS_RUNQ_PICK.

In fact, one can already figure out when nothing has been picked from the runqueue, by looking at when cpu_idle is invoked --which is the same thing one would do if on Credit or Credit2.

Signed-off-by Dario Faggioli <[email protected]>

Reviewed-by Konrad Rzeszutek Wilk <[email protected]>

---
Cc: George Dunlap <[email protected]>
Cc: Meng Xu <[email protected]>
Cc: Tianyang Chen <[email protected]>
Cc: Olaf Hering <[email protected]>
---
Changes from v1:
* enhanced changelog, as suggested during review.

---
 xen/common/sched_rt.c |   27 ++++-----------------------
 1 file changed, 4 insertions(+), 23 deletions(-)

xen/common/sched_rt.c +4 -23

index 2e5430f..53de6d6 100644
@@ -362,7 +362,7 @@ rt_update_deadline(s_time_t now, struct rt_vcpu *svc)
     /* TRACE */
     {
         struct {
-            unsigned dom:16,vcpu:16;
+            unsigned vcpu:16, dom:16;
             unsigned cur_deadline_lo, cur_deadline_hi;
             unsigned cur_budget_lo, cur_budget_hi;
         } d;

@@ -711,7 +711,7 @@ burn_budget(const struct scheduler *ops, struct rt_vcpu *svc, s_time_t now)
     /* TRACE */
     {
         struct {
-            unsigned dom:16, vcpu:16;
+            unsigned vcpu:16, dom:16;
             unsigned cur_budget_lo;
             unsigned cur_budget_hi;
             int delta;

@@ -763,7 +763,7 @@ __runq_pick(const struct scheduler *ops, const cpumask_t *mask)
         if( svc != NULL )
         {
             struct {
-                unsigned dom:16, vcpu:16;
+                unsigned vcpu:16, dom:16;
                 unsigned cur_deadline_lo, cur_deadline_hi;
                 unsigned cur_budget_lo, cur_budget_hi;
             } d;

@@ -777,8 +777,6 @@ __runq_pick(const struct scheduler *ops, const cpumask_t *mask)
                       sizeof(d),
                       (unsigned char *) &d);
         }
-        else
-            trace_var(TRC_RTDS_RUNQ_PICK, 1, 0, NULL);
     }
 
     return svc;

@@ -845,6 +843,7 @@ rt_schedule(const struct scheduler *ops, s_time_t now, bool_t tasklet_work_sched
 
     if ( tasklet_work_scheduled )
     {
+        trace_var(TRC_RTDS_SCHED_TASKLET, 1, 0,  NULL);
         snext = rt_vcpu(idle_vcpu[cpu]);
     }
     else

@@ -885,24 +884,6 @@ rt_schedule(const struct scheduler *ops, s_time_t now, bool_t tasklet_work_sched
     ret.time = MIN(snext->budget, MAX_SCHEDULE); /* sched quantum */
     ret.task = snext->vcpu;
 
-    /* TRACE */
-    {
-        struct {
-            unsigned dom:16,vcpu:16;
-            unsigned cur_deadline_lo, cur_deadline_hi;
-            unsigned cur_budget_lo, cur_budget_hi;
-        } d;
-        d.dom = snext->vcpu->domain->domain_id;
-        d.vcpu = snext->vcpu->vcpu_id;
-        d.cur_deadline_lo = (unsigned) snext->cur_deadline;
-        d.cur_deadline_hi = (unsigned) (snext->cur_deadline >> 32);
-        d.cur_budget_lo = (unsigned) snext->cur_budget;
-        d.cur_budget_hi = (unsigned) (snext->cur_budget >> 32);
-        trace_var(TRC_RTDS_SCHED_TASKLET, 1,
-                  sizeof(d),
-                  (unsigned char *)&d);
-    }
-
     return ret;
 }
 

when tracing runstate changes, the vcpu and domain IDs are encoded in the lower and higher, respectively, parts of a 32 bits integer. When decoding a trace with xentrace_format, this makes it possible to display such events like this:

CPU0  833435853624 (+     768)  running_to_runnable [ dom:vcpu = 0x7fff0000 ]
CPU0  833435854416 (+     792)  runnable_to_running [ dom:vcpu = 0x00000007 ]

For consistency, we should do the same when displaying the events coming from the RTDS scheduler (when using the same tool), and to do that, we need to invert the order in which the fields are being put in the trace struct right now.

While there, we also:
- fix the use of TRC_RTDS_SCHED_TASKLET (it should only be involved when a tasklet is scheduled, not _every_ time rt_schedule() is invoked!);
- remove a very chatty and useless (nothing has been picked!) use of TRC_RTDS_RUNQ_PICK.

In fact, one can already figure out when nothing has been picked from the runqueue, by looking at when cpu_idle is invoked --which is the same thing one would do if on Credit or Credit2.

Signed-off-by Dario Faggioli <[email protected]>

Reviewed-by Konrad Rzeszutek Wilk <[email protected]>

---
Cc: George Dunlap <[email protected]>
Cc: Meng Xu <[email protected]>
Cc: Tianyang Chen <[email protected]>
Cc: Olaf Hering <[email protected]>
---
Changes from v1:
* enhanced changelog, as suggested during review.

---
 xen/common/sched_rt.c |   27 ++++-----------------------
 1 file changed, 4 insertions(+), 23 deletions(-)

xen/common/sched_rt.c +4 -16

index 2e5430f..53de6d6 100644
@@ -362,7 +362,7 @@ rt_update_deadline(s_time_t now, struct rt_vcpu *svc)
     /* TRACE */
     {
         struct {
-            unsigned dom:16,vcpu:16;
+            unsigned vcpu:16, dom:16;
             unsigned cur_deadline_lo, cur_deadline_hi;
             unsigned cur_budget_lo, cur_budget_hi;
         } d;

@@ -711,7 +711,7 @@ burn_budget(const struct scheduler *ops, struct rt_vcpu *svc, s_time_t now)
     /* TRACE */
     {
         struct {
-            unsigned dom:16, vcpu:16;
+            unsigned vcpu:16, dom:16;
             unsigned cur_budget_lo;
             unsigned cur_budget_hi;
             int delta;

@@ -763,7 +763,7 @@ __runq_pick(const struct scheduler *ops, const cpumask_t *mask)
         if( svc != NULL )
         {
             struct {
-                unsigned dom:16, vcpu:16;
+                unsigned vcpu:16, dom:16;
                 unsigned cur_deadline_lo, cur_deadline_hi;
                 unsigned cur_budget_lo, cur_budget_hi;
             } d;

@@ -777,8 +777,6 @@ __runq_pick(const struct scheduler *ops, const cpumask_t *mask)
                       sizeof(d),
                       (unsigned char *) &d);
         }
-        else
-            trace_var(TRC_RTDS_RUNQ_PICK, 1, 0, NULL);
     }
 
     return svc;

@@ -845,6 +843,7 @@ rt_schedule(const struct scheduler *ops, s_time_t now, bool_t tasklet_work_sched
 
     if ( tasklet_work_scheduled )
     {
+        trace_var(TRC_RTDS_SCHED_TASKLET, 1, 0,  NULL);
         snext = rt_vcpu(idle_vcpu[cpu]);
     }
     else

@@ -885,24 +884,6 @@ rt_schedule(const struct scheduler *ops, s_time_t now, bool_t tasklet_work_sched
     ret.time = MIN(snext->budget, MAX_SCHEDULE); /* sched quantum */
     ret.task = snext->vcpu;
 
-    /* TRACE */
-    {
-        struct {
-            unsigned dom:16,vcpu:16;
-            unsigned cur_deadline_lo, cur_deadline_hi;
-            unsigned cur_budget_lo, cur_budget_hi;
-        } d;
-        d.dom = snext->vcpu->domain->domain_id;
-        d.vcpu = snext->vcpu->vcpu_id;
-        d.cur_deadline_lo = (unsigned) snext->cur_deadline;
-        d.cur_deadline_hi = (unsigned) (snext->cur_deadline >> 32);

it is enabled for pretty much all of them already. There were just a few that had it disabled.

When tracing a scheduler, timing information is really important, so enable it everywhere scheduling related.

Note that this was not really a problem if looking at the traces with xenalyze, but it was if using xentrace_format.

Signed-off-by Dario Faggioli <[email protected]>

---
Cc: George Dunlap <[email protected]>
Cc: Meng Xu <[email protected]>
Cc: Tianyang Chen <[email protected]>
Cc: Olaf Hering <[email protected]>

---
 xen/common/sched_credit.c  |    2 +-
 xen/common/sched_credit2.c |    6 +++---
 xen/common/sched_rt.c      |    2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

xen/common/sched_credit.c +1 -1

index 5279b92..bd2f37f 100644
@@ -476,7 +476,7 @@ static inline void __runq_tickle(struct csched_vcpu *new)
         {
             /* Avoid TRACE_*: saves checking !tb_init_done each step */
             for_each_cpu(cpu, &mask)
-                __trace_var(TRC_CSCHED_TICKLE, 0, sizeof(cpu), &cpu);
+                __trace_var(TRC_CSCHED_TICKLE, 1, sizeof(cpu), &cpu);
         }
 
         /* Send scheduler interrupts to designated CPUs */

xen/common/sched_credit2.c +3 -3

index cf40f68..2934e26 100644
@@ -456,7 +456,7 @@ runq_insert(const struct scheduler *ops, unsigned int cpu, struct csched2_vcpu *
         d.dom = svc->vcpu->domain->domain_id;
         d.vcpu = svc->vcpu->vcpu_id;
         d.pos = pos;
-        trace_var(TRC_CSCHED2_RUNQ_POS, 0,
+        trace_var(TRC_CSCHED2_RUNQ_POS, 1,
                   sizeof(d),
                   (unsigned char *)&d);
     }

@@ -564,7 +564,7 @@ tickle:
             unsigned cpu:16, pad:16;
         } d;
         d.cpu = ipid; d.pad = 0;
-        trace_var(TRC_CSCHED2_TICKLE, 0,
+        trace_var(TRC_CSCHED2_TICKLE, 1,
                   sizeof(d),
                   (unsigned char *)&d);
     }

@@ -1721,7 +1721,7 @@ csched2_schedule(
      */
     if ( tasklet_work_scheduled )
     {
-        trace_var(TRC_CSCHED2_SCHED_TASKLET, 0, 0,  NULL);
+        trace_var(TRC_CSCHED2_SCHED_TASKLET, 1, 0,  NULL);
         snext = CSCHED2_VCPU(idle_vcpu[cpu]);
     }
     else

xen/common/sched_rt.c +0 -0

index 53de6d6..33ac9de 100644

Hi Dario,

Since this patch did some obvious change, I will reply with reviewed-by, although my reviewed-by does not count much. ;-)

Quoted 4 lines · Dario Faggioli

Note that this was not really a problem if looking at the traces with xenalyze, but it was if using xentrace_format.

I just have a quick (and perhaps naive) question: :-)
If xenanlyze works better than xentrace_format, why shouldn't we stick
to xenanlyze?
Is there some functionality xentrace_format can but xenalyze cannot?

(I have to confess that I only used xenalyze but didn't use xentrace_format before. :-()

Quoted 5 lines · Meng Xu

Hi Dario,

Since this patch did some obvious change, I will reply with reviewed-by, although my reviewed-by does not count much. ;-)

That can't be less true. First of all, you're the original author of this code, and you and, although I'm the maintainer, your group are the one doing active development on it, so your opinion does have a weight.

But even if that wasn't the case, every reviewed-by is important, and helps the project. It will be maintainers' and committer's job to properly take each one into account in the most appropriate way, but that does not mean it's not worthwhile for you (or anyone else) to review the patches and express your acknowledgment, or send in your comments. :-)

Actually, do feel free to do as much review (and, in case it applies, send in your reviewed-by tag) as you like and can, either on RTDS or anywhere else... The project is in great need of that!!

Quoted 8 lines

Quoted 4 lines · Dario Faggioli

Note that this was not really a problem if looking at the traces with xenalyze, but it was if using xentrace_format.

Quoted 9 lines

I just have a quick (and perhaps naive) question: :-)
If xenanlyze works better than xentrace_format, why shouldn't we stick
to xenanlyze?
Is there some functionality xentrace_format can but xenalyze cannot?

(I have to confess that I only used xenalyze but didn't use xentrace_format before. :-()

xenalyze is indeed more advanced, but I don't think this means we should ignore or neglect xentrace_format: we've got it in tree, so we should not let it bitrot. I'm not in all our users' heads, so I don't know whether --and if yes why-- people may prefer the latter over the former, but I see room for someone wanting something basic and simple, in some cases.

Actually, I've been in a couple of situations myself, where the raw output of xentrace_format is easier to consume and, quick-&-dirtily, post-process, than the much more elaborated one of xenalyze.

For instance, the thing that you can just change on the fly the way a trace is shown (by tweaking the format file) looks an interesting feature to me, even considering all the limitations of "pure" xentrace. And if one want to change the formats for her own purposes, I feel like it is important that the one that we ship is updated, and can be used as a decent base for that.

Quoted 5 lines · Meng Xu

Hi Dario,

Since this patch did some obvious change, I will reply with reviewed-by, although my reviewed-by does not count much. ;-)

That can't be less true. First of all, you're the original author of this code, and you and, although I'm the maintainer, your group are the one doing active development on it, so your opinion does have a weight.

But even if that wasn't the case, every reviewed-by is important, and helps the project. It will be maintainers' and committer's job to properly take each one into account in the most appropriate way, but that does not mean it's not worthwhile for you (or anyone else) to review the patches and express your acknowledgment, or send in your comments. :-)

Actually, do feel free to do as much review (and, in case it applies, send in your reviewed-by tag) as you like and can, either on RTDS or anywhere else... The project is in great need of that!!

Quoted 7 lines

Quoted 4 lines · Dario Faggioli

Note that this was not really a problem if looking at the traces with xenalyze, but it was if using xentrace_format.

Quoted 9 lines

I just have a quick (and perhaps naive) question: :-)
If xenanlyze works better than xentrace_format, why shouldn't we stick
to xenanlyze?
Is there some functionality xentrace_format can but xenalyze cannot?

(I have to confess that I only used xenalyze but didn't use xentrace_format before. :-()

xenalyze is indeed more advanced, but I don't think this means we should ignore or neglect xentrace_format: we've got it in tree, so we should not let it bitrot. I'm not in all our users' heads, so I don't know whether --and if yes why-- people may prefer the latter over the former, but I see room for someone wanting something basic and simple, in some cases.

Actually, I've been in a couple of situations myself, where the raw output of xentrace_format is easier to consume and, quick-&-dirtily, post-process, than the much more elaborated one of xenalyze.

For instance, the thing that you can just change on the fly the way a trace is shown (by tweaking the format file) looks an interesting feature to me, even considering all the limitations of "pure" xentrace. And if one want to change the formats for her own purposes, I feel like it is important that the one that we ship is updated, and can be used as a decent base for that.

Quoted 11 lines · Dario Faggioli

For instance, the thing that you can just change on the fly the way a
trace is shown (by tweaking the format file) looks an interesting feature to me, even considering all the limitations of "pure" xentrace.
And if one want to change the formats for her own purposes, I feel like
it is important that the one that we ship is updated, and can be used
as a decent base for that.

So I certainly agree that xentrace_formats should be maintained so that
it works.  I hadn't thought before about the advantage of being able to
change the formats file more easily than adding new records to xenalyze,
but that's a good point.

But I do want to ask, how neccessary / useful is it to make the *TSC* information available to xentrace_format?

The reason most of the traces don't include a timestamp is that it increases the record size by a non-negligible amount -- in all the cases
here the traces are 1, 2, or 3 bytes without the tsc, so you're basically doubling the size of what gets traced.

How does adding the TSC significantly help someone using xentrace_format?

Quoted 13 lines · George Dunlap

Quoted 11 lines · Dario Faggioli

For instance, the thing that you can just change on the fly the way a
trace is shown (by tweaking the format file) looks an interesting feature to me, even considering all the limitations of "pure" xentrace.
And if one want to change the formats for her own purposes, I feel like
it is important that the one that we ship is updated, and can be used
as a decent base for that.

Quoted 8 lines

So I certainly agree that xentrace_formats should be maintained so that
it works. I hadn't thought before about the advantage of being able to
change the formats file more easily than adding new records to xenalyze,
but that's a good point.

Yeah... To be fair, it's much more the exception than the rule, IMO, that you really need xentrace_format instead of xenalyze... But it can happen, and that's one possible reason.

Quoted 3 lines

But I do want to ask, how neccessary / useful is it to make the *TSC* information available to xentrace_format?

So, when tracing the scheduler, I personally don't see much value in having the record of an event, if I don't also have the time at which the event happened.

So, this is an example of xentrace_format with all this series applied, except for this patch:

CPU0  2970034498098 (+ 9454788)  domain_wake       [ dom:vcpu = 0x00000001 ]
CPU0  2970034499304 (+    1206)  blocked_to_runnable [ dom:vcpu = 0x00000001 ]
CPU0  0 (+       0)  csched:tickle        [ cpu = 0 ]
CPU0  2970034511640 (+   12336)  switch_infprev    [ old_domid = 0x00007fff, runtime = 3956318 ]
CPU0  2970034512090 (+     450)  switch_infnext    [ new_domid = 0x00000000, time = 4446, r_time = 30000000 ]
CPU0  2970034512480 (+     390)  __enter_scheduler [ prev<dom:vcpu> = 0x7fff0000, next<dom:vcpu> = 0x00000001 ]
CPU0  2970034513002 (+     522)  running_to_runnable [ dom:vcpu = 0x7fff0000 ]
CPU0  2970034513422 (+     420)  runnable_to_running [ dom:vcpu = 0x00000001 ]

So, suppose, for instance, I want to figure out how much time passes between when a pcpu is tickled, and when it actually schedules and pick up the task that woke up. From just the trace above, I can't do that.

I know that this may not always be reliable when using only xentrace_format (because of TSC not being synchronized or drifting), but if used well (e.g., with pinning) or on good/new enough hardware (with synch and constant rate TSCs), I think it should be possible.

On the other hand this is how a similar trace looks if TSC is enabled, where the above can be achived:

CPU5  9965509909596 (+ 9268404)  domain_wake       [ dom:vcpu = 0x00000004 ]
CPU5  9965509911030 (+    1434)  blocked_to_runnable [ dom:vcpu = 0x00000004 ]
CPU5  9965509912962 (+    1932)  csched:tickle        [ cpu = 5 ]
CPU5  9965509924002 (+   11040)  switch_infprev    [ old_domid = 0x00007fff, runtime = 3879052 ]
CPU5  9965509924506 (+     504)  switch_infnext    [ new_domid = 0x00000000, time = 5000, r_time = 30000000 ]
CPU5  9965509924824 (+     318)  __enter_scheduler [ prev<dom:vcpu> = 0x7fff0005, next<dom:vcpu> = 0x00000004 ]
CPU5  9965509925478 (+     654)  running_to_runnable [ dom:vcpu = 0x7fff0005 ]
CPU5  9965509925892 (+     414)  runnable_to_running [ dom:vcpu = 0x00000004 ]

This is not an issue with xenalyze, and I think that is because you fiddle with timestamps in it, in order to compensate for the per- cpuness/desynch/etc. issues. I haven't checked the code where that happens in xenalyze, though, so I don't know whether having the TSC in more records would also help xenalyze or not.

Quoted 6 lines

The reason most of the traces don't include a timestamp is that it increases the record size by a non-negligible amount -- in all the cases
here the traces are 1, 2, or 3 bytes without the tsc, so you're basically doubling the size of what gets traced.

I see, but I think it's worth in this case.

Perhaps, we can think of ways of enabling and disabling logging TSC dynamically, either at compile or run time. Doing at run time, given the way tracing is currently implemented, will most likely incur in some overhead. Very small, but still something, and I'm not sure we're ok introducing it.

Doing it at compile time would be a lot less flexible, but perhaps a decent compromise, i.e., I at least always have the events... If I want to know exactly when they happened, for all of them, I need a Xen version build to provide that (like I need a debug build to have ASSERT()s and symbols).

Quoted 3 lines

How does adding the TSC significantly help someone using xentrace_format?

Hope I answered to this. Let me know what you think. :-)

to include the vcpu IDs, in a way that matches
how the "dom:vcpu" couple is displayed in other
events (runstate changes).

Also add the trace for TRC_SCHED_SHUTDOWN_CODE which was missing and was done via SCHEDOP_shutdown_code hypercall. (TRC_SCHED_SHUTDOWN trace was present).

Signed-off-by Dario Faggioli <[email protected]>

Reviewed-by Konrad Rzeszutek Wilk <[email protected]>

---
Cc: George Dunlap <[email protected]>
Cc: Ian Jackson <[email protected]>
Cc: Ian Campbell <[email protected]>
Cc: Wei Liu <[email protected]>
Cc: Olaf Hering <[email protected]>
---
Changes from v1:
* enhanced changelog, as suggested during review.

---
 tools/xentrace/formats |   18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

tools/xentrace/formats +9 -9

index 5d7b72a..5257cf0 100644
@@ -19,22 +19,22 @@
 0x00021311  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  offline_to_runnable [ dom:vcpu = 0x%(1)08x ]
 0x00021321  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  offline_to_blocked  [ dom:vcpu = 0x%(1)08x ]
 
-0x00028001  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  sched_add_domain  [ domid = 0x%(1)08x, edomid = 0x%(2)08x ]
-0x00028002  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  sched_rem_domain  [ domid = 0x%(1)08x, edomid = 0x%(2)08x ]
-0x00028003  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  domain_sleep      [ domid = 0x%(1)08x, edomid = 0x%(2)08x ]
-0x00028004  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  domain_wake       [ domid = 0x%(1)08x, edomid = 0x%(2)08x ]
-0x00028005  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  do_yield          [ domid = 0x%(1)08x, edomid = 0x%(2)08x ]
-0x00028006  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  do_block          [ domid = 0x%(1)08x, edomid = 0x%(2)08x ]
-0x00022006  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  do_block          [ dom:vcpu = 0x%(1)08x, domid = 0x%(2)08x ]
-0x00028007  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  domain_shutdown	  [ domid = 0x%(1)08x, edomid = 0x%(2)08x, reason = 0x%(3)08x ]
+0x00028001  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  sched_add_domain  [ domid = 0x%(1)08x ]
+0x00028002  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  sched_rem_domain  [ domid = 0x%(1)08x ]
+0x00028003  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  domain_sleep      [ dom:vcpu = 0x%(1)04x%(2)04x ]
+0x00028004  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  domain_wake       [ dom:vcpu = 0x%(1)04x%(2)04x ]
+0x00028005  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  do_yield          [ dom:vcpu = 0x%(1)04x%(2)04x ]
+0x00028006  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  do_block          [ dom:vcpu = 0x%(1)04x%(2)04x ]
+0x00028007  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  domain_shutdown	  [ dom:vcpu = 0x%(1)04x%(2)04x, reason = 0x%(3)08x ]
 0x00028008  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  sched_ctl
 0x00028009  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  sched_adjdom      [ domid = 0x%(1)08x ]
-0x0002800a  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  __enter_scheduler [ prev<domid:edomid> = 0x%(1)08x : 0x%(2)08x, next<domid:edomid> = 0x%(3)08x : 0x%(4)08x ]
+0x0002800a  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  __enter_scheduler [ prev<dom:vcpu> = 0x%(1)04x%(2)04x, next<dom:vcpu> = 0x%(3)04x%(4)04x ]
 0x0002800b  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  s_timer_fn
 0x0002800c  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  t_timer_fn
 0x0002800d  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  dom_timer_fn
 0x0002800e  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  switch_infprev    [ old_domid = 0x%(1)08x, runtime = %(2)d ]
 0x0002800f  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  switch_infnext    [ new_domid = 0x%(1)08x, time = %(2)d, r_time = %(3)d ]
+0x00028010  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  domain_shutdown_code [ dom:vcpu = 0x%(1)04x%(2)04x, reason = 0x%(3)08x ]
 
 0x00081001  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  VMENTRY
 0x00081002  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  VMEXIT      [ exitcode = 0x%(1)08x, rIP  = 0x%(2)08x ]

so the trace will show properly decoded info,
rather than just a bunch of hex codes.

Signed-off-by Dario Faggioli <[email protected]>

Reviewed-by Konrad Rzeszutek Wilk <[email protected]>

---
Cc: George Dunlap <[email protected]>
Cc: Ian Jackson <[email protected]>
Cc: Ian Campbell <[email protected]>
Cc: Wei Liu <[email protected]>
Cc: Olaf Hering <[email protected]>
---
Changes from v1:
* SCHED_DOM_{ADD,REM} handling slightly changed, to avoid confusion with DOM0_DOM_{ADD,REM} (introduced later in the series);
* '} * r =' turned into '} *r =', as requested during review.

---
 tools/xentrace/xenalyze.c |  156 ++++++++++++++++++++++++++++++++++++---------
 1 file changed, 126 insertions(+), 30 deletions(-)

tools/xentrace/xenalyze.c +126 -30

index 6520790..17021f1 100644
@@ -1519,27 +1519,6 @@ struct pv_data {
 };
 
 /* Sched data */
-
-enum {
-    SCHED_DOM_ADD=1,
-    SCHED_DOM_REM,
-    SCHED_SLEEP,
-    SCHED_WAKE,
-    SCHED_YIELD,
-    SCHED_BLOCK,
-    SCHED_SHUTDOWN,
-    SCHED_CTL,
-    SCHED_ADJDOM,
-    SCHED_SWITCH,
-    SCHED_S_TIMER_FN,
-    SCHED_T_TIMER_FN,
-    SCHED_DOM_TIMER_FN,
-    SCHED_SWITCH_INFPREV,
-    SCHED_SWITCH_INFNEXT,
-    SCHED_SHUTDOWN_CODE,
-    SCHED_MAX
-};
-
 enum {
     RUNSTATE_RUNNING=0,
     RUNSTATE_RUNNABLE,

@@ -7431,6 +7410,17 @@ no_update:
     return;
 }
 
+void dump_sched_switch(struct record_info *ri)
+{
+    struct {
+        unsigned int prev_dom, prev_vcpu, next_dom, next_vcpu;
+    } *r = (typeof(r))ri->d;
+
+    printf(" %s sched_switch prev d%uv%u next d%uv%u\n",
+           ri->dump_header, r->prev_dom, r->prev_vcpu,
+           r->next_dom, r->next_vcpu);
+}
+
 void sched_switch_process(struct pcpu_info *p)
 {
     struct vcpu_data *prev, *next;

@@ -7440,10 +7430,7 @@ void sched_switch_process(struct pcpu_info *p)
     } * r = (typeof(r))ri->d;
 
     if(opt.dump_all)
-        printf("%s sched_switch prev d%uv%u next d%uv%u\n",
-               ri->dump_header,
-               r->prev_dom, r->prev_vcpu,
-               r->next_dom, r->next_vcpu);
+        dump_sched_switch(ri);
 
     if(r->prev_vcpu > MAX_CPUS)
     {

@@ -7559,6 +7546,14 @@ void sched_summary_domain(struct domain_data *d)
     }
 }
 
+void dump_sched_vcpu_action(struct record_info *ri, const char *action)
+{
+    struct {
+        unsigned int domid, vcpuid;
+    } *r = (typeof(r))ri->d;
+
+    printf(" %s %s d%uv%u\n", ri->dump_header, action, r->domid, r->vcpuid);
+}
 
 void sched_process(struct pcpu_info *p)
 {

@@ -7573,13 +7568,114 @@ void sched_process(struct pcpu_info *p)
         default:
             process_generic(&p->ri);
         }
-    } else {
-        if(ri->evt.sub == 1)
-            sched_runstate_process(p);
-        else {
-            UPDATE_VOLUME(p, sched_verbose, ri->size);
+        return;
+    }
+
+    if(ri->evt.sub == 1) {
+        /* TRC_SCHED_MIN */
+        sched_runstate_process(p);
+    } else if (ri->evt.sub == 8) {
+        /* TRC_SCHED_VERBOSE */
+        switch(ri->event)
+        {
+        case TRC_SCHED_DOM_ADD:
+            if(opt.dump_all) {
+                struct {
+                    unsigned int domid;
+                } *r = (typeof(r))ri->d;
+
+                printf(" %s sched_init_domain d%u\n", ri->dump_header, r->domid);
+            }
+            break;
+        case TRC_SCHED_DOM_REM:
+            if(opt.dump_all) {
+                struct {
+                    unsigned int domid, vcpuid;
+                } *r = (typeof(r))ri->d;
+
+                printf(" %s sched_destroy_domain d%u\n", ri->dump_header, r->domid);
+            }
+            break;
+        case TRC_SCHED_SLEEP:
+            if(opt.dump_all)
+                dump_sched_vcpu_action(ri, "vcpu_sleep");
+            break;
+        case TRC_SCHED_WAKE:
+            if(opt.dump_all)
+                dump_sched_vcpu_action(ri, "vcpu_wake");
+            break;
+        case TRC_SCHED_YIELD:
+            if(opt.dump_all)
+                dump_sched_vcpu_action(ri, "vcpu_yield");
+            break;
+        case TRC_SCHED_BLOCK:
+            if(opt.dump_all)
+                dump_sched_vcpu_action(ri, "vcpu_block");
+            break;
+        case TRC_SCHED_SHUTDOWN:
+        case TRC_SCHED_SHUTDOWN_CODE:
+            if(opt.dump_all) {
+                struct {
+                    unsigned int domid, vcpuid, reason;
+                } *r = (typeof(r))ri->d;
+
+                printf(" %s %s d%uv%u, reason = %u\n", ri->dump_header,
+                       ri->event == TRC_SCHED_SHUTDOWN ? "sched_shutdown" :
+                       "sched_shutdown_code", r->domid, r->vcpuid, r->reason);
+            }
+            break;
+        case TRC_SCHED_ADJDOM:
+            if(opt.dump_all) {
+                struct {
+                    unsigned int domid;
+                } *r = (typeof(r))ri->d;
+
+                printf(" %s sched_adjust d%u\n", ri->dump_header, r->domid);
+            }
+            break;
+        case TRC_SCHED_SWITCH:
+            dump_sched_switch(ri);
+            break;
+        case TRC_SCHED_SWITCH_INFPREV:
+            if(opt.dump_all) {
+                struct {
+                    unsigned int domid, runtime;
+                } *r = (typeof(r))ri->d;
+
+                printf(" %s sched_switch prev d%u, run for %u.%uus\n",
+                       ri->dump_header, r->domid, r->runtime / 1000,
+                       r->runtime % 1000);
+            }
+            break;
+        case TRC_SCHED_SWITCH_INFNEXT:
+            if(opt.dump_all)
+            {
+                struct {
+                    unsigned int domid, rsince;
+                    int slice;
+                } *r = (typeof(r))ri->d;
+
+                printf(" %s sched_switch next d%u", ri->dump_header, r->domid);
+                if ( r->rsince != 0 )
+                    printf(", was runnable for %u.%uus, ", r->rsince / 1000,
+                           r->rsince % 1000);
+                if ( r->slice > 0 )
+                    printf("next slice %u.%uus\n", r->slice / 1000,
+                           r->slice % 1000);
+                printf("\n");
+            }
+            break;
+        case TRC_SCHED_CTL:
+        case TRC_SCHED_S_TIMER_FN:
+        case TRC_SCHED_T_TIMER_FN:
+        case TRC_SCHED_DOM_TIMER_FN:
+            break;
+        default:
             process_generic(&p->ri);
         }
+    } else {
+        UPDATE_VOLUME(p, sched_verbose, ri->size);
+        process_generic(&p->ri);
     }
 }
 

so the trace will show properly decoded info,
rather than just a bunch of hex codes.

Signed-off-by Dario Faggioli <[email protected]>

Reviewed-by Konrad Rzeszutek Wilk <[email protected]>

---
Cc: George Dunlap <[email protected]>
Cc: Ian Jackson <[email protected]>
Cc: Ian Campbell <[email protected]>
Cc: Wei Liu <[email protected]>
Cc: Olaf Hering <[email protected]>
---
Changes from v1:
* '} * r =' turned into '} *r =', as requested
during review.

---
 tools/xentrace/xenalyze.c |   57 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

tools/xentrace/xenalyze.c +57 -0

index 17021f1..4ab2dba 100644
@@ -7673,6 +7673,63 @@ void sched_process(struct pcpu_info *p)
         default:
             process_generic(&p->ri);
         }
+    } else if(ri->evt.sub == 2) {
+        /* TRC_SCHED_CLASS */
+        switch(ri->event)
+        {
+        /* CREDIT (TRC_CSCHED_xxx) */
+        case TRC_SCHED_CLASS_EVT(CSCHED, 1): /* SCHED_TASKLET */
+            if(opt.dump_all)
+                printf(" %s csched:sched_tasklet\n", ri->dump_header);
+            break;
+        case TRC_SCHED_CLASS_EVT(CSCHED, 2): /* ACCOUNT_START */
+        case TRC_SCHED_CLASS_EVT(CSCHED, 3): /* ACCOUNT_STOP  */
+            if(opt.dump_all) {
+                struct {
+                    unsigned int domid, vcpuid, actv_cnt;
+                } *r = (typeof(r))ri->d;
+
+                printf(" %s csched:acct_%s d%uv%u, active_vcpus %u\n",
+                       ri->dump_header,
+                       ri->event == TRC_SCHED_CLASS_EVT(CSCHED, 2) ?
+                       "start" : "stop",
+                       r->domid, r->vcpuid,
+                       r->actv_cnt);
+            }
+            break;
+        case TRC_SCHED_CLASS_EVT(CSCHED, 4): /* STOLEN_VCPU   */
+            if(opt.dump_all) {
+                struct {
+                    unsigned int peer_cpu, domid, vcpuid;
+                } *r = (typeof(r))ri->d;
+
+                printf(" %s csched:stolen_vcpu d%uv%u from cpu %u\n",
+                       ri->dump_header, r->domid, r->vcpuid, r->peer_cpu);
+            }
+            break;
+        case TRC_SCHED_CLASS_EVT(CSCHED, 5): /* PICKED_CPU    */
+            if(opt.dump_all) {
+                struct {
+                    unsigned int domid, vcpuid, cpu;
+                } *r = (typeof(r))ri->d;
+
+                printf(" %s csched:pick_cpu %u for d%uv%u\n",
+                       ri->dump_header, r->cpu, r->domid, r->vcpuid);
+            }
+            break;
+        case TRC_SCHED_CLASS_EVT(CSCHED, 6): /* TICKLE        */
+            if(opt.dump_all) {
+                struct {
+                    unsigned int cpu;
+                } *r = (typeof(r))ri->d;
+
+                printf(" %s csched:runq_tickle, cpu %u\n",
+                       ri->dump_header, r->cpu);
+            }
+            break;
+        default:
+            process_generic(ri);
+        }
     } else {
         UPDATE_VOLUME(p, sched_verbose, ri->size);
         process_generic(&p->ri);

so the trace will show properly decoded info,
rather than just a bunch of hex codes.

Signed-off-by Dario Faggioli <[email protected]>

Reviewed-by Konrad Rzeszutek Wilk <[email protected]>

---
Cc: George Dunlap <[email protected]>
Cc: Ian Jackson <[email protected]>
Cc: Ian Campbell <[email protected]>
Cc: Wei Liu <[email protected]>
Cc: Olaf Hering <[email protected]>
---
Changes from v1:
* '} * r =' turned into '} *r =', as requested
during review.

---
 tools/xentrace/xenalyze.c |   57 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

tools/xentrace/xenalyze.c +0 -0

index 17021f1..4ab2dba 100644

tools/xentrace/xenalyze.c +51 -0

+            if(opt.dump_all)
+                printf(" %s csched:sched_tasklet\n", ri->dump_header);
+            break;
+        case TRC_SCHED_CLASS_EVT(CSCHED, 2): /* ACCOUNT_START */
+        case TRC_SCHED_CLASS_EVT(CSCHED, 3): /* ACCOUNT_STOP  */
+            if(opt.dump_all) {
+                struct {
+                    unsigned int domid, vcpuid, actv_cnt;
+                } *r = (typeof(r))ri->d;
+
+                printf(" %s csched:acct_%s d%uv%u, active_vcpus %u\n",
+                       ri->dump_header,
+                       ri->event == TRC_SCHED_CLASS_EVT(CSCHED, 2) ?
+                       "start" : "stop",
+                       r->domid, r->vcpuid,
+                       r->actv_cnt);
+            }
+            break;
+        case TRC_SCHED_CLASS_EVT(CSCHED, 4): /* STOLEN_VCPU   */
+            if(opt.dump_all) {
+                struct {
+                    unsigned int peer_cpu, domid, vcpuid;
+                } *r = (typeof(r))ri->d;
+
+                printf(" %s csched:stolen_vcpu d%uv%u from cpu %u\n",
+                       ri->dump_header, r->domid, r->vcpuid, r->peer_cpu);
+            }
+            break;
+        case TRC_SCHED_CLASS_EVT(CSCHED, 5): /* PICKED_CPU    */
+            if(opt.dump_all) {
+                struct {
+                    unsigned int domid, vcpuid, cpu;
+                } *r = (typeof(r))ri->d;
+
+                printf(" %s csched:pick_cpu %u for d%uv%u\n",
+                       ri->dump_header, r->cpu, r->domid, r->vcpuid);
+            }
+            break;
+        case TRC_SCHED_CLASS_EVT(CSCHED, 6): /* TICKLE        */
+            if(opt.dump_all) {
+                struct {
+                    unsigned int cpu;
+                } *r = (typeof(r))ri->d;
+
+                printf(" %s csched:runq_tickle, cpu %u\n",
+                       ri->dump_header, r->cpu);
+            }
+            break;
+        default:
+            process_generic(ri);
+        }
     } else {
         UPDATE_VOLUME(p, sched_verbose, ri->size);
         process_generic(&p->ri);

Quoted 7 lines · Dario Faggioli

tools/xentrace/xenalyze.c +0 -0

index 17021f1..4ab2dba 100644
@@ -7673,6 +7673,63 @@ void sched_process(struct pcpu_info *p)

         default:

Quoted 8 lines

             process_generic(&p->ri);
         }
+    } else if(ri->evt.sub == 2) {
+        /* TRC_SCHED_CLASS */
+        switch(ri->event)
+        {
+        /* CREDIT (TRC_CSCHED_xxx) */
+        case TRC_SCHED_CLASS_EVT(CSCHED, 1): /* SCHED_TASKLET */

Sorry, just one more comment:

It would probably be good at some point to find a way to expose these in
a public header, rather than having to manually keep this file in sync
with the values in sched_*.c.  But that's a nice-to-have for another day. :-)

(Maybe a cleanup for a GSoC / OPW student...?)

so the trace will show properly decoded info,
rather than just a bunch of hex codes.

Signed-off-by Dario Faggioli <[email protected]>

Reviewed-by Konrad Rzeszutek Wilk <[email protected]>

---
Cc: George Dunlap <[email protected]>
Cc: Ian Jackson <[email protected]>
Cc: Ian Campbell <[email protected]>
Cc: Wei Liu <[email protected]>
Cc: Olaf Hering <[email protected]>
Cc: Konrad Rzeszutek Wilk <[email protected]>
---
Changes from v1:
* '} * r =' turned into '} *r =', as requested during review.

---
 tools/xentrace/xenalyze.c |  101 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 101 insertions(+)

tools/xentrace/xenalyze.c +101 -0

index 4ab2dba..8f97f3a 100644
@@ -7727,6 +7727,107 @@ void sched_process(struct pcpu_info *p)
                        ri->dump_header, r->cpu);
             }
             break;
+        /* CREDIT 2 (TRC_CSCHED2_xxx) */
+        case TRC_SCHED_CLASS_EVT(CSCHED2, 1): /* TICK              */
+        case TRC_SCHED_CLASS_EVT(CSCHED2, 4): /* CREDIT_ADD        */
+        case TRC_SCHED_CLASS_EVT(CSCHED2, 9): /* UPDATE_LOAD       */
+            break;
+        case TRC_SCHED_CLASS_EVT(CSCHED2, 2): /* RUNQ_POS          */
+            if(opt.dump_all) {
+                struct {
+                    unsigned int vcpuid:16, domid:16, pos;
+                } *r = (typeof(r))ri->d;
+
+                printf(" %s csched2:runq_insert d%uv%u, position %u\n",
+                       ri->dump_header, r->domid, r->vcpuid, r->pos);
+            }
+            break;
+        case TRC_SCHED_CLASS_EVT(CSCHED2, 3): /* CREDIT_BURN       */
+            if(opt.dump_all) {
+                struct {
+                    unsigned int vcpuid:16, domid:16, credit;
+                    int delta;
+                } *r = (typeof(r))ri->d;
+
+                printf(" %s csched2:burn_credits d%uv%u, credit = %u, delta = %d\n",
+                       ri->dump_header, r->domid, r->vcpuid,
+                       r->credit, r->delta);
+            }
+            break;
+        case TRC_SCHED_CLASS_EVT(CSCHED2, 5): /* TICKLE_CHECK      */
+            if(opt.dump_all) {
+                struct {
+                    unsigned int vcpuid:16, domid:16;
+                    unsigned int credit;
+                } *r = (typeof(r))ri->d;
+
+                printf(" %s csched2:tickle_check d%uv%u, credit = %u\n",
+                       ri->dump_header, r->domid, r->vcpuid, r->credit);
+            }
+            break;
+        case TRC_SCHED_CLASS_EVT(CSCHED2, 6): /* TICKLE            */
+            if(opt.dump_all) {
+                struct {
+                    unsigned int cpu:16;
+                } *r = (typeof(r))ri->d;
+
+                printf(" %s csched2:runq_tickle cpu %u\n",
+                       ri->dump_header, r->cpu);
+            }
+            break;
+        case TRC_SCHED_CLASS_EVT(CSCHED2, 7):  /* CREDIT_RESET     */
+            if(opt.dump_all) {
+                struct {
+                    unsigned int vcpuid:16, domid:16;
+                    unsigned int credit_start, credit_end;
+                    unsigned int multiplier;
+                } *r = (typeof(r))ri->d;
+
+                printf(" %s csched2:reset_credits d%uv%u, "
+                       "credit_start = %u, credit_end = %u, mult = %u\n",
+                       ri->dump_header, r->domid, r->vcpuid,
+                       r->credit_start, r->credit_end, r->multiplier);
+            }
+            break;
+        case TRC_SCHED_CLASS_EVT(CSCHED2, 8):  /* SCHED_TASKLET    */
+            if(opt.dump_all)
+                printf(" %s csched2:sched_tasklet\n", ri->dump_header);
+            break;
+        case TRC_SCHED_CLASS_EVT(CSCHED2, 10): /* RUNQ_ASSIGN      */
+            if(opt.dump_all) {
+                struct {
+                    unsigned int vcpuid:16, domid:16;
+                    unsigned int rqi;
+                } *r = (typeof(r))ri->d;
+
+                printf(" %s csched2:runq_assign d%uv%u on rq# %u\n",
+                       ri->dump_header, r->domid, r->vcpuid, r->rqi);
+            }
+            break;
+        case TRC_SCHED_CLASS_EVT(CSCHED2, 11): /* UPDATE_VCPU_LOAD */
+            if(opt.dump_all) {
+                struct {
+                    unsigned int vcpuid:16, domid:16;
+                    unsigned int avgload;
+                } *r = (typeof(r))ri->d;
+
+                printf(" %s csched2:update_vcpu_load d%uv%u, avg_load = %u\n",
+                       ri->dump_header, r->domid, r->vcpuid, r->avgload);
+            }
+            break;
+        case TRC_SCHED_CLASS_EVT(CSCHED2, 12): /* UPDATE_RUNQ_LOAD */
+            if(opt.dump_all) {
+                struct {
+                    unsigned int rq_load:4, rq_avgload:28;
+                    unsigned int rq_id:4, b_avgload:28;
+                } *r = (typeof(r))ri->d;
+
+                printf(" %s csched2:update_rq_load rq# %u, load = %u, "
+                       "avgload = %u, b_avgload = %u\n",
+                       ri->dump_header, r->rq_id, r->rq_load,
+                       r->rq_avgload, r->b_avgload);
+            }
+            break;
         default:
             process_generic(ri);
         }

so the trace will show properly decoded info,
rather than just a bunch of hex codes.

Signed-off-by Dario Faggioli <[email protected]>

Reviewed-by Konrad Rzeszutek Wilk <[email protected]>

---
Cc: George Dunlap <[email protected]>
Cc: Meng Xu <[email protected]>
Cc: Tianyang Chen <[email protected]>
Cc: Ian Jackson <[email protected]>
Cc: Ian Campbell <[email protected]>
Cc: Wei Liu <[email protected]>
Cc: Olaf Hering <[email protected]>
---
Changes from v1:
* '} * r =' turned into '} *r =', as requested
during review.

---
 tools/xentrace/xenalyze.c |   59 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

tools/xentrace/xenalyze.c +10 -0

index 8f97f3a..dd21229 100644
@@ -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);
+            }

Quoted 29 lines · Dario Faggioli

---
 tools/xentrace/xenalyze.c |   59
+++++++++++++++++++++++++++++++++++++++++++++

 1 file changed, 59 insertions(+)

tools/xentrace/xenalyze.c +16 -0

index 8f97f3a..dd21229 100644
@@ -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;

tools/xentrace/xenalyze.c void sched_process(struct pcpu_info *p) · line 7852

Quoted 6 lines

tools/xentrace/xenalyze.c +4 -0

+                    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?

Quoted 30 lines · George Dunlap

Quoted 29 lines · Dario Faggioli

---
 tools/xentrace/xenalyze.c |   59
+++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

tools/xentrace/xenalyze.c +16 -0

index 8f97f3a..dd21229 100644
@@ -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;

Quoted 9 lines

tools/xentrace/xenalyze.c void sched_process(struct pcpu_info *p) · line 7852

Quoted 6 lines

tools/xentrace/xenalyze.c +4 -0

+                    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).

Quoted 24 lines · Dario Faggioli

Quoted 18 lines · George Dunlap

Quoted 6 lines

+        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;

Quoted 8 lines

+                    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.

Quoted 5 lines

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?

Dario Faggioli (16):
      xen: sched: __runq_tickle takes a useless cpu parameter
      xen: sched: move up the trace record for vcpu_wake and vcpu_sleep
      xen: sched: improve domain creation tracing
      xen: credit2: pack trace data better for xentrace_format
      xen: RTDS: pack trace data better for xentrace_format
      xen: sched: tracing: enable TSC tracing for all events

      xentrace: formats: update format of scheduling events
      xentrace: formats: add events from Credit scheduler
      xentrace: formats: add events from Credit2 scheduler
      xentrace: formats: add events from RTDS scheduler
      xentrace: formats: add domain create and destroy events.

      xenalyze: handle scheduling events

      xenalyze: handle Credit1 scheduler events

      xenalyze: handle Credit2 scheduler events

      xenalyze: handle RTDS scheduler events

      xenalyze: handle DOM0 operaions events

Hello committers, George,

This is basically a ping for this series, as I think most of it can actually go in, unless I've missed something.

So, let me try to recap:

Quoted 10 lines · Dario Faggioli

Dario Faggioli (16):
      xen: sched: __runq_tickle takes a useless cpu parameter
      xen: sched: move up the trace record for vcpu_wake and vcpu_sleep
      xen: sched: improve domain creation tracing
      xen: credit2: pack trace data better for xentrace_format
      xen: RTDS: pack trace data better for xentrace_format
      xen: sched: tracing: enable TSC tracing for all events

Until here, it's in already.

Quoted 6 lines

      xentrace: formats: update format of scheduling events
      xentrace: formats: add events from Credit scheduler
      xentrace: formats: add events from Credit2 scheduler
      xentrace: formats: add events from RTDS scheduler
      xentrace: formats: add domain create and destroy events.

About these, they've got Konrad's Reviewed-by, and George said:

"I haven't reviewed the formats file changes, but all of them look reasonable; so for patches 7-11:

Acked-by George Dunlap <[email protected]>"

Quoted 2 lines

      xenalyze: handle scheduling events

Reviewed-by Konrad Rzeszutek Wilk <[email protected]>

Acked-by George Dunlap <[email protected]>

Quoted 2 lines

      xenalyze: handle Credit1 scheduler events

Reviewed-by Konrad Rzeszutek Wilk <[email protected]>

Acked-by George Dunlap <[email protected]>

Quoted 2 lines

      xenalyze: handle Credit2 scheduler events

Reviewed-by Konrad Rzeszutek Wilk <[email protected]>

Acked-by George Dunlap <[email protected]>

Quoted 2 lines

      xenalyze: handle RTDS scheduler events

This should be skipped. In fact, we agreed with George that the plan would be:

"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?"

So, just ignore this patch.

Quoted 2 lines

      xenalyze: handle DOM0 operaions events

This one is actually missing Georges's Ack.

I'd be fine with this being skipped as well, and I will resubmit as separate patch, or as part of the followup series mentioned above.

Or George can Ack it, and it just can go in now.

But this is not a big deal.

What I would like, if possible, is for patches until 14 to be checked in, so I can submit the follow up (together with other patches that I have stacked on top of that).

Thanks and regards,
Dario

Signature · 4 lines

<<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)

Quoted 1 line

Quoted 1 line · On 04.03.16 at 19:25, <[email protected]> wrote:

Hello committers, George,

This is basically a ping for this series, as I think most of it can actually go in, unless I've missed something.

So, let me try to recap:

Quoted 10 lines · Dario Faggioli

Dario Faggioli (16):
xen: sched: __runq_tickle takes a useless cpu parameter
xen: sched: move up the trace record for vcpu_wake and
vcpu_sleep
xen: sched: improve domain creation tracing
xen: credit2: pack trace data better for xentrace_format
xen: RTDS: pack trace data better for xentrace_format
xen: sched: tracing: enable TSC tracing for all events

Until here, it's in already.

Quoted 19 lines

Quoted 1 line

Quoted 1 line · On 04.03.16 at 19:25, <[email protected]> wrote:

Hello committers, George,

This is basically a ping for this series, as I think most of it can actually go in, unless I've missed something.

So, let me try to recap:

Quoted 10 lines · Dario Faggioli

Dario Faggioli (16):
xen: sched: __runq_tickle takes a useless cpu parameter
xen: sched: move up the trace record for vcpu_wake and
vcpu_sleep
xen: sched: improve domain creation tracing
xen: credit2: pack trace data better for xentrace_format
xen: RTDS: pack trace data better for xentrace_format
xen: sched: tracing: enable TSC tracing for all events

Quoted 1 line

Until here, it's in already.

And that's the part I could reasonably take care of. I generally avoid committing larger chunks of tools/ stuff, with the expectation that Ian would take deal with those.

Jan

Quoted 1 line

Quoted 1 line · On 07.03.16 at 11:54, <[email protected]> wrote:

Quoted 21 lines · Jan Beulich

Quoted 20 lines

Quoted 1 line

Quoted 1 line · On 04.03.16 at 19:25, <[email protected]> wrote:

Hello committers, George,

This is basically a ping for this series, as I think most of it can actually go in, unless I've missed something.

So, let me try to recap:

Quoted 10 lines · Dario Faggioli

Dario Faggioli (16):
xen: sched: __runq_tickle takes a useless cpu parameter
xen: sched: move up the trace record for vcpu_wake and
vcpu_sleep
xen: sched: improve domain creation tracing
xen: credit2: pack trace data better for xentrace_format
xen: RTDS: pack trace data better for xentrace_format
xen: sched: tracing: enable TSC tracing for all events

Until here, it's in already.

Quoted 4 lines

And that's the part I could reasonably take care of. I generally avoid committing larger chunks of tools/ stuff, with the expectation that Ian would take deal with those.

Ian is away this week. To avoid having no tools stuff committed this whole week, I can prepare a branch for you to pull if you think that's OK.

Quoted 28 lines

Quoted 1 line

Quoted 1 line · On 07.03.16 at 11:54, <[email protected]> wrote:

Quoted 25 lines · Jan Beulich

Quoted 19 lines

Quoted 1 line

Quoted 1 line · On 04.03.16 at 19:25, <[email protected]> wrote:

Hello committers, George,

This is basically a ping for this series, as I think most of it can actually go in, unless I've missed something.

So, let me try to recap:

Quoted 10 lines · Dario Faggioli

Dario Faggioli (16):
xen: sched: __runq_tickle takes a useless cpu parameter
xen: sched: move up the trace record for vcpu_wake and
vcpu_sleep
xen: sched: improve domain creation tracing
xen: credit2: pack trace data better for xentrace_format
xen: RTDS: pack trace data better for xentrace_format
xen: sched: tracing: enable TSC tracing for all events

Quoted 1 line

Until here, it's in already.

And that's the part I could reasonably take care of. I generally avoid committing larger chunks of tools/ stuff, with the expectation that Ian would take deal with those.

Quoted 3 lines

Ian is away this week. To avoid having no tools stuff committed this whole week, I can prepare a branch for you to pull if you think that's OK.

Well, if these were urgent I'd say yes. But I don't think they are, so I'd leave it to either Konrad (who has basically asked for what you offer) if he wants to deal with it, of wait for Ian's return.

Jan

Quoted 6 lines · Jan Beulich

Quoted 2 lines

Quoted 1 line

Quoted 1 line · On 07.03.16 at 11:54, <[email protected]> wrote:

Well, if these were urgent I'd say yes. But I don't think they are, so I'd leave it to either Konrad (who has basically asked for what you offer) if he wants to deal with it, of wait for Ian's return.

It's not urgent, but I would like to avoid wasting any test cycle. No matter how innocent a patch looks, there is always risk that it breaks
something.

Konrad, your call.

Quoted 12 lines · Wei Liu

Quoted 6 lines · Jan Beulich

Quoted 2 lines

Quoted 1 line

Quoted 1 line · On 07.03.16 at 11:54, <[email protected]> wrote:

Well, if these were urgent I'd say yes. But I don't think they are, so I'd leave it to either Konrad (who has basically asked for what you offer) if he wants to deal with it, of wait for Ian's return.

It's not urgent, but I would like to avoid wasting any test cycle. No matter how innocent a patch looks, there is always risk that it breaks
something.

Quoted 2 lines

Konrad, your call.

In any case, and as far as these patches are concerned, here it is:

 git://xenbits.xen.org/people/dariof/xen.git  tracing/sched-events-improvements

 http://xenbits.xen.org/gitweb/?p=people/dariof/xen.git;a=shortlog;h=refs/heads/tracing/sched-events-improvements

Thanks and Regards,
Dario

Signature · 4 lines

<<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)