[v2,07/12] xen/rt: Clean up trace handling

Commit Message

Andrew Cooper Sept. 20, 2021, 5:25 p.m. UTC

Most uses of bitfields and __packed are unnecessary.  There is also no need to
cast 'd' to (unsigned char *) before passing it to a function taking void *.

No functional change.

Signed-off-by: Andrew Cooper <[e-mail]>
---
CC: George Dunlap <[e-mail]>
CC: Ian Jackson <[e-mail]>
CC: Jan Beulich <[e-mail]>
CC: Stefano Stabellini <[e-mail]>
CC: Wei Liu <[e-mail]>
CC: Julien Grall <[e-mail]>
CC: Dario Faggioli <[e-mail]>
CC: Meng Xu <[e-mail]>

v2:
 * New
---
 xen/common/sched/rt.c | 97 ++++++++++++++++++++++++---------------------------
 1 file changed, 46 insertions(+), 51 deletions(-)

Comments

Dario Faggioli Sept. 24, 2021, 4:56 p.m. UTC | #1

On Mon, 2021-09-20 at 18:25 +0100, Andrew Cooper wrote:
> Most uses of bitfields and __packed are unnecessary.  There is also
> no need to
> cast 'd' to (unsigned char *) before passing it to a function taking
> void *.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <[e-mail]>
>
Reviewed-by: Dario Faggioli <[e-mail]>

Thanks and Regards

Patch

diff --git a/xen/common/sched/rt.c b/xen/common/sched/rt.c
index c58edca0de84..d5da35cac75e 100644
--- a/xen/common/sched/rt.c
+++ b/xen/common/sched/rt.c
@@ -455,21 +455,21 @@  rt_update_deadline(s_time_t now, struct rt_unit *svc)
     svc->cur_budget = svc->budget;
     svc->priority_level = 0;
 
-    /* TRACE */
+    if ( unlikely(tb_init_done) )
     {
-        struct __packed {
-            unsigned unit:16, dom:16;
-            unsigned priority_level;
+        struct {
+            uint16_t unit, dom;
+            uint32_t priority_level;
             uint64_t cur_deadline, cur_budget;
-        } d;
-        d.dom = svc->unit->domain->domain_id;
-        d.unit = svc->unit->unit_id;
-        d.priority_level = svc->priority_level;
-        d.cur_deadline = (uint64_t) svc->cur_deadline;
-        d.cur_budget = (uint64_t) svc->cur_budget;
-        trace_var(TRC_RTDS_BUDGET_REPLENISH, 1,
-                  sizeof(d),
-                  (unsigned char *) &d);
+        } d = {
+            .dom             = svc->unit->domain->domain_id,
+            .unit            = svc->unit->unit_id,
+            .priority_level  = svc->priority_level,
+            .cur_deadline    = svc->cur_deadline,
+            .cur_budget      = svc->cur_budget,
+        };
+
+        __trace_var(TRC_RTDS_BUDGET_REPLENISH, 1, sizeof(d), &d);
     }
 
     return;
@@ -965,7 +965,7 @@  burn_budget(const struct scheduler *ops, struct rt_unit *svc, s_time_t now)
         }
     }
 
-    /* TRACE */
+    if ( unlikely(tb_init_done) )
     {
         struct __packed {
             uint16_t unit, dom;
@@ -982,9 +982,7 @@  burn_budget(const struct scheduler *ops, struct rt_unit *svc, s_time_t now)
             .has_extratime   = !!(svc->flags & RTDS_extratime),
         };
 
-        trace_var(TRC_RTDS_BUDGET_BURN, 1,
-                  sizeof(d),
-                  (unsigned char *) &d);
+        __trace_var(TRC_RTDS_BUDGET_BURN, 1, sizeof(d), &d);
     }
 }
 
@@ -1019,22 +1017,19 @@  runq_pick(const struct scheduler *ops, const cpumask_t *mask, unsigned int cpu)
         break;
     }
 
-    /* TRACE */
+    if ( unlikely(tb_init_done) && svc )
     {
-        if( svc != NULL )
-        {
-            struct __packed {
-                unsigned unit:16, dom:16;
-                uint64_t cur_deadline, cur_budget;
-            } d;
-            d.dom = svc->unit->domain->domain_id;
-            d.unit = svc->unit->unit_id;
-            d.cur_deadline = (uint64_t) svc->cur_deadline;
-            d.cur_budget = (uint64_t) svc->cur_budget;
-            trace_var(TRC_RTDS_RUNQ_PICK, 1,
-                      sizeof(d),
-                      (unsigned char *) &d);
-        }
+        struct __packed {
+            uint16_t unit, dom;
+            uint64_t cur_deadline, cur_budget;
+        } d = {
+            .unit          = svc->unit->unit_id,
+            .dom           = svc->unit->domain->domain_id,
+            .cur_deadline  = svc->cur_deadline,
+            .cur_budget    = svc->cur_budget,
+        };
+
+        __trace_var(TRC_RTDS_RUNQ_PICK, 1, sizeof(d), &d);
     }
 
     return svc;
@@ -1055,18 +1050,19 @@  rt_schedule(const struct scheduler *ops, struct sched_unit *currunit,
     struct rt_unit *snext = NULL;
     bool migrated = false;
 
-    /* TRACE */
+    if ( unlikely(tb_init_done) )
     {
-        struct __packed {
-            unsigned cpu:16, tasklet:8, tickled:4, idle:4;
-        } d;
-        d.cpu = cur_cpu;
-        d.tasklet = tasklet_work_scheduled;
-        d.tickled = cpumask_test_cpu(sched_cpu, &prv->tickled);
-        d.idle = is_idle_unit(currunit);
-        trace_var(TRC_RTDS_SCHEDULE, 1,
-                  sizeof(d),
-                  (unsigned char *)&d);
+        struct {
+            uint16_t cpu;
+            uint8_t tasklet, tickled:4, idle:4;
+        } d = {
+            .cpu      = cur_cpu,
+            .tasklet  = tasklet_work_scheduled,
+            .tickled  = cpumask_test_cpu(sched_cpu, &prv->tickled),
+            .idle     = is_idle_unit(currunit),
+        };
+
+        __trace_var(TRC_RTDS_SCHEDULE, 1, sizeof(d), &d);
     }
 
     /* clear ticked bit now that we've been scheduled */
@@ -1223,16 +1219,15 @@  runq_tickle(const struct scheduler *ops, const struct rt_unit *new)
     SCHED_STAT_CRANK(tickled_no_cpu);
     return;
  out:
-    /* TRACE */
+    if ( unlikely(tb_init_done) )
     {
         struct {
-            unsigned cpu:16, pad:16;
-        } d;
-        d.cpu = cpu_to_tickle;
-        d.pad = 0;
-        trace_var(TRC_RTDS_TICKLE, 1,
-                  sizeof(d),
-                  (unsigned char *)&d);
+            uint16_t cpu, _pad;
+        } d = {
+            .cpu = cpu_to_tickle,
+        };
+
+        __trace_var(TRC_RTDS_TICKLE, 1, sizeof(d), &d);
     }
 
     cpumask_set_cpu(cpu_to_tickle, &prv->tickled);