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 5301ceed69976f2a4a6142a452998b3a0e65635f..072b9cac24adb8e0747c415fe7fc58e5fb2c1d0a 100644 (file)
--- 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 05ee7738460dcfb2084b750ecb8dfa907d8820ae..1a736197980773cecc2039abc9b952a3f1dad2e5 100644 (file)
--- a/include/haproxy/activity.h
+++ b/include/haproxy/activity.h
@@ -69,24 +69,22 @@ static inline void activity_count_runtime()
        }
 
        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 53721efcd4fd726c759a95449879f774ccff01e5..79aad3e1cdb1243aca20200d5467cdc1359efa18 100644 (file)
--- 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 @@ static int cfg_parse_prof_tasks(char **args, int section_type, struct proxy *cur
        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 @@ static int cli_parse_set_profiling(char **args, char *payload, struct appctx *ap
        }
        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 @@ static int cli_io_handler_show_profiling(struct appctx *appctx)
        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;
        }