MINOR: task: permanently enable latency measurement on tasklets

When tasklet latency measurement was enabled in 2.4 with commit b2285de04
("MINOR: tasks: also compute the tasklet latency when DEBUG_TASK is set"),
the feature was conditionned on DEBUG_TASK because the field would add 8
bytes to the struct tasklet.

This approach was not a very good idea because the struct ends on an int
anyway thus it does finish with a 32-bit hole regardless of the presence
of this field. What is true however is that adding it turned a 64-byte
struct to 72-byte when caller debugging is enabled.

This patch revisits this with a minor change. Now only the lowest 32
bits of the call date are stored, so they always fit in the remaining
hole, and this allows to remove the dependency on DEBUG_TASK. With
debugging off, we're now seeing a 48-byte struct, and with debugging
on it's exactly 64 bytes, thus still exactly one cache line. 32 bits
allow a latency of 4 seconds on a tasklet, which already indicates a
completely dead process, so there's no point storing the upper bits at
all. And even in the event it would happen once in a while, the lost
upper bits do not really add any value to the debug reports. Also, now
one tasklet wakeup every 4 billion will not be sampled due to the test
on the value itself. Similarly we just don't care, it's statistics and
the measurements are not 9-digit accurate anyway.

diff --git a/include/haproxy/task-t.h b/include/haproxy/task-t.h
index f481364de02abe4ea4393106b2126da53914ab6d..db3a5f827b0d2768b3429e625d0476dfe4523fcd 100644 (file)
--- a/include/haproxy/task-t.h
+++ b/include/haproxy/task-t.h
@@ -126,9 +126,7 @@ struct tasklet {
         * list starts and this works because both are exclusive. Never ever
         * reorder these fields without taking this into account!
         */
-#ifdef DEBUG_TASK
-       uint64_t call_date;             /* date of the last tasklet wakeup or call */
-#endif
+       uint32_t call_date;             /* date of the last tasklet wakeup or call */
        int tid;                        /* TID of the tasklet owner, <0 if local */
 };
 

diff --git a/include/haproxy/task.h b/include/haproxy/task.h
index 7a58e3ed9c8cbf7628518f855bd149c2314e5016..34646130dfecbee4e52f695636547ad36868a2bc 100644 (file)
--- a/include/haproxy/task.h
+++ b/include/haproxy/task.h
@@ -359,9 +359,9 @@ static inline void _tasklet_wakeup_on(struct tasklet *tl, int thr, const char *f
        tl->debug.caller_idx = !tl->debug.caller_idx;
        tl->debug.caller_file[tl->debug.caller_idx] = file;
        tl->debug.caller_line[tl->debug.caller_idx] = line;
+#endif
        if (_HA_ATOMIC_LOAD(&th_ctx->flags) & TH_FL_TASK_PROFILING)
                tl->call_date = now_mono_time();
-#endif
        __tasklet_wakeup_on(tl, thr);
 }
 
@@ -446,9 +446,9 @@ static inline struct list *_tasklet_wakeup_after(struct list *head, struct taskl
        tl->debug.caller_idx = !tl->debug.caller_idx;
        tl->debug.caller_file[tl->debug.caller_idx] = file;
        tl->debug.caller_line[tl->debug.caller_idx] = line;
+#endif
        if (th_ctx->flags & TH_FL_TASK_PROFILING)
                tl->call_date = now_mono_time();
-#endif
        return __tasklet_wakeup_after(head, tl);
 }
 
@@ -517,8 +517,8 @@ static inline void tasklet_init(struct tasklet *t)
        t->state = TASK_F_TASKLET;
        t->process = NULL;
        t->tid = -1;
-#ifdef DEBUG_TASK
        t->call_date = 0;
+#ifdef DEBUG_TASK
        t->debug.caller_idx = 0;
 #endif
        LIST_INIT(&t->list);

diff --git a/src/task.c b/src/task.c
index 0399a711014bb2c9ecb06547c271dab6aa257a2e..395bc9ac47763515bb1f3e2f219138105be50814 100644 (file)
--- a/src/task.c
+++ b/src/task.c
@@ -572,12 +572,11 @@ unsigned int run_tasks_from_lists(unsigned int budgets[])
                        if (unlikely(_HA_ATOMIC_LOAD(&th_ctx->flags) & TH_FL_TASK_PROFILING)) {
                                profile_entry = sched_activity_entry(sched_activity, t->process);
                                before = now_mono_time();
-#ifdef DEBUG_TASK
+
                                if (((struct tasklet *)t)->call_date) {
-                                       HA_ATOMIC_ADD(&profile_entry->lat_time, before - ((struct tasklet *)t)->call_date);
+                                       HA_ATOMIC_ADD(&profile_entry->lat_time, (uint32_t)(before - ((struct tasklet *)t)->call_date));
                                        ((struct tasklet *)t)->call_date = 0;
                                }
-#endif
                        }
 
                        state = _HA_ATOMIC_FETCH_AND(&t->state, TASK_PERSISTENT);