MINOR: activity: make profiling more manageable
In 2.0, commit d2d3348ac ("MINOR: activity: enable automatic profiling
turn on/off") introduced an automatic mode to enable/disable profiling.
The problem is that the automatic mode automatically changes to on/off,
which implied that the forced on/off modes aren't sticky anymore. It's
annoying when debugging because as soon as the load decreases, profiling
stops.
This makes a small change which ought to have been done first, which
consists in having two states for "auto" (auto-on, auto-off) to
distinguish them from the forced states. Setting to "auto" in the config
defaults to "auto-off" as before, and setting it on the CLI switches to
auto but keeps the current operating state.
This is simple enough to be backported to older releases if needed.
diff --git a/include/haproxy/activity-t.h b/include/haproxy/activity-t.h
index 5301cee..072b9ca 100644
--- a/include/haproxy/activity-t.h
+++ b/include/haproxy/activity-t.h
@@ -27,8 +27,9 @@
/* bit fields for the "profiling" global variable */
#define HA_PROF_TASKS_OFF 0x00000000 /* per-task CPU profiling forced disabled */
-#define HA_PROF_TASKS_AUTO 0x00000001 /* per-task CPU profiling automatic */
-#define HA_PROF_TASKS_ON 0x00000002 /* per-task CPU profiling forced enabled */
+#define HA_PROF_TASKS_AOFF 0x00000001 /* per-task CPU profiling off (automatic) */
+#define HA_PROF_TASKS_AON 0x00000002 /* per-task CPU profiling on (automatic) */
+#define HA_PROF_TASKS_ON 0x00000003 /* per-task CPU profiling forced enabled */
#define HA_PROF_TASKS_MASK 0x00000003 /* per-task CPU profiling mask */
/* per-thread activity reports. It's important that it's aligned on cache lines
diff --git a/include/haproxy/activity.h b/include/haproxy/activity.h
index 05ee773..1a73619 100644
--- a/include/haproxy/activity.h
+++ b/include/haproxy/activity.h
@@ -69,24 +69,22 @@
}
run_time = (before_poll.tv_sec - after_poll.tv_sec) * 1000000U + (before_poll.tv_usec - after_poll.tv_usec);
- swrate_add(&activity[tid].avg_loop_us, TIME_STATS_SAMPLES, run_time);
+ run_time = swrate_add(&activity[tid].avg_loop_us, TIME_STATS_SAMPLES, run_time);
- /* reaching the "up" threshold on average switches profiling to "on"
- * when automatic, and going back below the "down" threshold switches
- * to off.
+ /* In automatic mode, reaching the "up" threshold on average switches
+ * profiling to "on" when automatic, and going back below the "down"
+ * threshold switches to off. The forced modes don't check the load.
*/
if (!(task_profiling_mask & tid_bit)) {
if (unlikely((profiling & HA_PROF_TASKS_MASK) == HA_PROF_TASKS_ON ||
- ((profiling & HA_PROF_TASKS_MASK) == HA_PROF_TASKS_AUTO && run_time >= up))) {
- if (swrate_avg(activity[tid].avg_loop_us, TIME_STATS_SAMPLES) >= up)
- _HA_ATOMIC_OR(&task_profiling_mask, tid_bit);
- }
+ ((profiling & HA_PROF_TASKS_MASK) == HA_PROF_TASKS_AON &&
+ swrate_avg(run_time, TIME_STATS_SAMPLES) >= up)))
+ _HA_ATOMIC_OR(&task_profiling_mask, tid_bit);
} else {
if (unlikely((profiling & HA_PROF_TASKS_MASK) == HA_PROF_TASKS_OFF ||
- ((profiling & HA_PROF_TASKS_MASK) == HA_PROF_TASKS_AUTO && run_time <= down))) {
- if (swrate_avg(activity[tid].avg_loop_us, TIME_STATS_SAMPLES) <= down)
- _HA_ATOMIC_AND(&task_profiling_mask, ~tid_bit);
- }
+ ((profiling & HA_PROF_TASKS_MASK) == HA_PROF_TASKS_AOFF &&
+ swrate_avg(run_time, TIME_STATS_SAMPLES) <= down)))
+ _HA_ATOMIC_AND(&task_profiling_mask, ~tid_bit);
}
}
diff --git a/src/activity.c b/src/activity.c
index 53721ef..79aad3e 100644
--- a/src/activity.c
+++ b/src/activity.c
@@ -21,7 +21,7 @@
/* bit field of profiling options. Beware, may be modified at runtime! */
-unsigned int profiling = HA_PROF_TASKS_AUTO;
+unsigned int profiling = HA_PROF_TASKS_AOFF;
unsigned long task_profiling_mask = 0;
/* One struct per thread containing all collected measurements */
@@ -49,7 +49,7 @@
if (strcmp(args[1], "on") == 0)
profiling = (profiling & ~HA_PROF_TASKS_MASK) | HA_PROF_TASKS_ON;
else if (strcmp(args[1], "auto") == 0)
- profiling = (profiling & ~HA_PROF_TASKS_MASK) | HA_PROF_TASKS_AUTO;
+ profiling = (profiling & ~HA_PROF_TASKS_MASK) | HA_PROF_TASKS_AOFF;
else if (strcmp(args[1], "off") == 0)
profiling = (profiling & ~HA_PROF_TASKS_MASK) | HA_PROF_TASKS_OFF;
else {
@@ -75,8 +75,14 @@
}
else if (strcmp(args[3], "auto") == 0) {
unsigned int old = profiling;
- while (!_HA_ATOMIC_CAS(&profiling, &old, (old & ~HA_PROF_TASKS_MASK) | HA_PROF_TASKS_AUTO))
- ;
+ unsigned int new;
+
+ do {
+ if ((old & HA_PROF_TASKS_MASK) >= HA_PROF_TASKS_AON)
+ new = (old & ~HA_PROF_TASKS_MASK) | HA_PROF_TASKS_AON;
+ else
+ new = (old & ~HA_PROF_TASKS_MASK) | HA_PROF_TASKS_AOFF;
+ } while (!_HA_ATOMIC_CAS(&profiling, &old, new));
}
else if (strcmp(args[3], "off") == 0) {
unsigned int old = profiling;
@@ -103,7 +109,8 @@
chunk_reset(&trash);
switch (profiling & HA_PROF_TASKS_MASK) {
- case HA_PROF_TASKS_AUTO: str="auto"; break;
+ case HA_PROF_TASKS_AOFF: str="auto-off"; break;
+ case HA_PROF_TASKS_AON: str="auto-on"; break;
case HA_PROF_TASKS_ON: str="on"; break;
default: str="off"; break;
}