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.

(cherry picked from commit 768c2c5678)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
This commit is contained in:
Willy Tarreau 2022-09-06 19:17:45 +02:00 committed by Christopher Faulet
parent 87f8732e1b
commit c432738cb0
3 changed files with 6 additions and 9 deletions

View File

@ -128,9 +128,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 */
};

View File

@ -402,9 +402,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 (task_profiling_mask & tid_bit)
tl->call_date = now_mono_time();
#endif
__tasklet_wakeup_on(tl, thr);
}
@ -486,9 +486,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);
}
@ -556,8 +556,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);

View File

@ -603,12 +603,11 @@ unsigned int run_tasks_from_lists(unsigned int budgets[])
if (unlikely(task_profiling_mask & tid_bit)) {
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);