diff mbox

[v2,04/16] xen: credit2: pack trace data better for xentrace_format

Message ID 20160216181136.27876.29825.stgit@Solace.station (mailing list archive)
State New, archived
Headers show

Commit Message

Dario Faggioli Feb. 16, 2016, 6:11 p.m. UTC
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 <dario.faggioli@citrix.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
Cc: George Dunlap <george.dunlap@citrix.com>
Cc: Olaf Hering <olaf@aepfle.de>
---
 xen/common/sched_credit2.c |   16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

Comments

George Dunlap Feb. 18, 2016, 11:10 a.m. UTC | #1
On 16/02/16 18:11, Dario Faggioli wrote:
> 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 <dario.faggioli@citrix.com>
> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

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 <george.dunlap@citrix.com>

> ---
> Cc: George Dunlap <george.dunlap@citrix.com>
> Cc: Olaf Hering <olaf@aepfle.de>
> ---
>  xen/common/sched_credit2.c |   16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
> index 78220a7..cf40f68 100644
> --- a/xen/common/sched_credit2.c
> +++ b/xen/common/sched_credit2.c
> @@ -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;
>
Dario Faggioli Feb. 18, 2016, 1:42 p.m. UTC | #2
On Thu, 2016-02-18 at 11:10 +0000, George Dunlap wrote:
> On 16/02/16 18:11, Dario Faggioli wrote:
> > 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 <dario.faggioli@citrix.com>
> > Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> 
> 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.
> 
If I understand what you mean, I'm doing exactly that in the second
half of this series (and per the latest email, the xentrace_format
part, you've seen it already). :-)

> So, all is well:
> 
> Acked-by: George Dunlap <george.dunlap@citrix.com>
> 
Thanks,
Dario
diff mbox

Patch

diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index 78220a7..cf40f68 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -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;