From a882a3ff25bcae8d703277ebd850fde0b1128ce9 Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

Date: Tue, 25 Apr 2017 17:23:04 -0500

Subject: [PATCH 1/5] Refactor: crmd: functionize setting throttle load target

Make as much in throttle.c static as possible, for better isolation.

---

 crmd/control.c  |  4 ++--

 crmd/throttle.c | 46 ++++++++++++++++++++++++++--------------------

 crmd/throttle.h |  4 +---

 3 files changed, 29 insertions(+), 25 deletions(-)

diff --git a/crmd/control.c b/crmd/control.c

index f4823b9..af9c2c2 100644

--- a/crmd/control.c

+++ b/crmd/control.c

@@ -1046,7 +1046,7 @@ config_query_callback(xmlNode * msg, int call_id, int rc, xmlNode * output, void

     value = crmd_pref(config_hash, "load-threshold");

     if(value) {

-        throttle_load_target = strtof(value, NULL) / 100;

+        throttle_set_load_target(strtof(value, NULL) / 100.0);

     }

     value = crmd_pref(config_hash, "no-quorum-policy");

diff --git a/crmd/throttle.c b/crmd/throttle.c

index ce330fe..b9add7d 100644

--- a/crmd/throttle.c

+++ b/crmd/throttle.c

@@ -33,8 +33,7 @@

 #include <throttle.h>

-enum throttle_state_e 

-{

+enum throttle_state_e {

     throttle_extreme = 0x1000,

     throttle_high = 0x0100,

     throttle_med  = 0x0010,

@@ -42,24 +41,24 @@ enum throttle_state_e

     throttle_none = 0x0000,

 };

-struct throttle_record_s 

-{

-        int max;

-        enum throttle_state_e mode;

-        char *node;

+struct throttle_record_s {

+    int max;

+    enum throttle_state_e mode;

+    char *node;

 };

-int throttle_job_max = 0;

-float throttle_load_target = 0.0;

+static int throttle_job_max = 0;

+static float throttle_load_target = 0.0;

 #define THROTTLE_FACTOR_LOW    1.2

 #define THROTTLE_FACTOR_MEDIUM 1.6

 #define THROTTLE_FACTOR_HIGH   2.0

-GHashTable *throttle_records = NULL;

-mainloop_timer_t *throttle_timer = NULL;

+static GHashTable *throttle_records = NULL;

+static mainloop_timer_t *throttle_timer = NULL;

-int throttle_num_cores(void)

+static int

+throttle_num_cores(void)

 {

     static int cores = 0;

     char buffer[256];

@@ -102,14 +101,16 @@ int throttle_num_cores(void)

  *       This will return NULL if the daemon is being run via valgrind.

  *       This should be called only on Linux systems.

  */

-static char *find_cib_loadfile(void) 

+static char *

+find_cib_loadfile(void)

 {

     int pid = crm_procfs_pid_of("cib");

     return pid? crm_strdup_printf("/proc/%d/stat", pid) : NULL;

 }

-static bool throttle_cib_load(float *load) 

+static bool

+throttle_cib_load(float *load)

 {

 /*

        /proc/[pid]/stat

@@ -233,7 +234,8 @@ static bool throttle_cib_load(float *load)

     return FALSE;

 }

-static bool throttle_load_avg(float *load)

+static bool

+throttle_load_avg(float *load)

 {

     char buffer[256];

     FILE *stream = NULL;

@@ -266,7 +268,8 @@ static bool throttle_load_avg(float *load)

     return FALSE;

 }

-static bool throttle_io_load(float *load, unsigned int *blocked)

+static bool

+throttle_io_load(float *load, unsigned int *blocked)

 {

     char buffer[64*1024];

     FILE *stream = NULL;

@@ -514,7 +517,13 @@ throttle_record_free(gpointer p)

 }

 void

-throttle_update_job_max(const char *preference) 

+throttle_set_load_target(float target)

+{

+    throttle_load_target = target;

+}

+

+void

+throttle_update_job_max(const char *preference)

 {

     int max = 0;

@@ -547,7 +556,6 @@ throttle_update_job_max(const char *preference)

     }

 }

-

 void

 throttle_init(void)

 {

@@ -568,7 +576,6 @@ throttle_fini(void)

     g_hash_table_destroy(throttle_records); throttle_records = NULL;

 }

-

 int

 throttle_get_total_job_limit(int l)

 {

@@ -673,4 +680,3 @@ throttle_update(xmlNode *xml)

     crm_debug("Host %s supports a maximum of %d jobs and throttle mode %.4x.  New job limit is %d",

               from, max, mode, throttle_get_job_limit(from));

 }

-

diff --git a/crmd/throttle.h b/crmd/throttle.h

index bdb33e7..9d1b97b 100644

--- a/crmd/throttle.h

+++ b/crmd/throttle.h

@@ -17,12 +17,10 @@

  */

-extern float throttle_load_target;

-

 void throttle_init(void);

 void throttle_fini(void);

-int throttle_num_cores(void);

+void throttle_set_load_target(float target);

 void throttle_update(xmlNode *xml);

 void throttle_update_job_max(const char *preference);

 int throttle_get_job_limit(const char *node);

-- 

1.8.3.1

From acbbca50ab2fc9f71b9517d62e22bde7891ee67d Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

Date: Fri, 28 Apr 2017 14:09:36 -0500

Subject: [PATCH 2/5] Refactor: crmd: functionize checking throttle thresholds

... to reduce code duplication

---

 crmd/throttle.c | 92 ++++++++++++++++++++++++++++++++-------------------------

 1 file changed, 51 insertions(+), 41 deletions(-)

diff --git a/crmd/throttle.c b/crmd/throttle.c

index b9add7d..8ab6f01 100644

--- a/crmd/throttle.c

+++ b/crmd/throttle.c

@@ -350,45 +350,68 @@ throttle_io_load(float *load, unsigned int *blocked)

     return FALSE;

 }

+/*!

+ * \internal

+ * \brief Check a load value against throttling thresholds

+ *

+ * \param[in] load        Load value to check

+ * \param[in] desc        Description of metric (for logging)

+ * \param[in] thresholds  Low/medium/high/extreme thresholds

+ *

+ * \return Throttle mode corresponding to load value

+ */

 static enum throttle_state_e

-throttle_handle_load(float load, const char *desc, int cores)

+throttle_check_thresholds(float load, const char *desc, float thresholds[4])

 {

-    float adjusted_load = load;

-

-    if(cores <= 0) {

-        /* No fudging of the supplied load value */

-

-    } else if(cores == 1) {

-        /* On a single core machine, a load of 1.0 is already too high */

-        adjusted_load = load * THROTTLE_FACTOR_MEDIUM;

-

-    } else {

-        /* Normalize the load to be per-core */

-        adjusted_load = load / cores;

-    }

+    if (load > thresholds[3]) {

+        crm_notice("Extreme %s detected: %f", desc, load);

+        return throttle_extreme;

-    if(adjusted_load > THROTTLE_FACTOR_HIGH * throttle_load_target) {

+    } else if (load > thresholds[2]) {

         crm_notice("High %s detected: %f", desc, load);

         return throttle_high;

-    } else if(adjusted_load > THROTTLE_FACTOR_MEDIUM * throttle_load_target) {

+    } else if (load > thresholds[1]) {

         crm_info("Moderate %s detected: %f", desc, load);

         return throttle_med;

-    } else if(adjusted_load > THROTTLE_FACTOR_LOW * throttle_load_target) {

+    } else if (load > thresholds[0]) {

         crm_debug("Noticeable %s detected: %f", desc, load);

         return throttle_low;

     }

-    crm_trace("Negligible %s detected: %f", desc, adjusted_load);

+    crm_trace("Negligible %s detected: %f", desc, load);

     return throttle_none;

 }

 static enum throttle_state_e

+throttle_handle_load(float load, const char *desc, int cores)

+{

+    float normalize;

+    float thresholds[4];

+

+    if (cores == 1) {

+        /* On a single core machine, a load of 1.0 is already too high */

+        normalize = 0.6;

+

+    } else {

+        /* Normalize the load to be per-core */

+        normalize = cores;

+    }

+    thresholds[0] = throttle_load_target * normalize * THROTTLE_FACTOR_LOW;

+    thresholds[1] = throttle_load_target * normalize * THROTTLE_FACTOR_MEDIUM;

+    thresholds[2] = throttle_load_target * normalize * THROTTLE_FACTOR_HIGH;

+    thresholds[3] = load + 1.0; /* never extreme */

+

+    return throttle_check_thresholds(load, desc, thresholds);

+}

+

+static enum throttle_state_e

 throttle_mode(void)

 {

     int cores;

     float load;

+    float thresholds[4];

     unsigned int blocked = 0;

     enum throttle_state_e mode = throttle_none;

@@ -399,16 +422,16 @@ throttle_mode(void)

     cores = throttle_num_cores();

     if(throttle_cib_load(&load)) {

         float cib_max_cpu = 0.95;

-        const char *desc = "CIB load";

-        /* The CIB is a single threaded task and thus cannot consume

+

+        /* The CIB is a single-threaded task and thus cannot consume

          * more than 100% of a CPU (and 1/cores of the overall system

          * load).

          *

-         * On a many cored system, the CIB might therefor be maxed out

+         * On a many-cored system, the CIB might therefore be maxed out

          * (causing operations to fail or appear to fail) even though

          * the overall system load is still reasonable.

          *

-         * Therefor the 'normal' thresholds can not apply here and we

+         * Therefore, the 'normal' thresholds can not apply here, and we

          * need a special case.

          */

         if(cores == 1) {

@@ -418,26 +441,13 @@ throttle_mode(void)

             cib_max_cpu = throttle_load_target;

         }

-        if(load > 1.5 * cib_max_cpu) {

-            /* Can only happen on machines with a low number of cores */

-            crm_notice("Extreme %s detected: %f", desc, load);

-            mode |= throttle_extreme;

-

-        } else if(load > cib_max_cpu) {

-            crm_notice("High %s detected: %f", desc, load);

-            mode |= throttle_high;

+        thresholds[0] = cib_max_cpu * 0.8;

+        thresholds[1] = cib_max_cpu * 0.9;

+        thresholds[2] = cib_max_cpu;

+        /* Can only happen on machines with a low number of cores */

+        thresholds[3] = cib_max_cpu * 1.5;

-        } else if(load > cib_max_cpu * 0.9) {

-            crm_info("Moderate %s detected: %f", desc, load);

-            mode |= throttle_med;

-

-        } else if(load > cib_max_cpu * 0.8) {

-            crm_debug("Noticeable %s detected: %f", desc, load);

-            mode |= throttle_low;

-

-        } else {

-            crm_trace("Negligible %s detected: %f", desc, load);

-        }

+        mode |= throttle_check_thresholds(load, "CIB load", thresholds);

     }

     if(throttle_load_target <= 0) {

-- 

1.8.3.1

From 89175e75f3b38b10ea163c1a8d621d1296570e7f Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

Date: Fri, 28 Apr 2017 14:30:55 -0500

Subject: [PATCH 3/5] Feature: libcrmcommon: add function to get number of CPU

 cores

Compared to the previous implementation in crmd/throttle.c, this

parses /proc/stat, which is smaller than /proc/cpuinfo.

---

 include/crm/common/internal.h |  1 +

 lib/common/procfs.c           | 30 ++++++++++++++++++++++++++++++

 2 files changed, 31 insertions(+)

diff --git a/include/crm/common/internal.h b/include/crm/common/internal.h

index 475128f..c34b03b 100644

--- a/include/crm/common/internal.h

+++ b/include/crm/common/internal.h

@@ -54,6 +54,7 @@ int crm_write_sync(int fd, const char *contents);

 int crm_procfs_process_info(struct dirent *entry, char *name, int *pid);

 int crm_procfs_pid_of(const char *name);

+unsigned int crm_procfs_num_cores(void);

 /* internal XML schema functions (from xml.c) */

diff --git a/lib/common/procfs.c b/lib/common/procfs.c

index 12d01ff..fbbf9eb 100644

--- a/lib/common/procfs.c

+++ b/lib/common/procfs.c

@@ -28,6 +28,7 @@

 #include <sys/stat.h>

 #include <sys/types.h>

 #include <dirent.h>

+#include <ctype.h>

 /*!

  * \internal

@@ -140,3 +141,32 @@ crm_procfs_pid_of(const char *name)

     closedir(dp);

     return pid;

 }

+

+/*!

+ * \internal

+ * \brief Calculate number of logical CPU cores from procfs

+ *

+ * \return Number of cores (or 1 if unable to determine)

+ */

+unsigned int

+crm_procfs_num_cores(void)

+{

+    int cores = 0;

+    FILE *stream = NULL;

+

+    /* Parse /proc/stat instead of /proc/cpuinfo because it's smaller */

+    stream = fopen("/proc/stat", "r");

+    if (stream == NULL) {

+        crm_perror(LOG_INFO, "Could not open /proc/stat");

+    } else {

+        char buffer[2048];

+

+        while (fgets(buffer, sizeof(buffer), stream)) {

+            if (!strncmp(buffer, "cpu", 3) && isdigit(buffer[3])) {

+                ++cores;

+            }

+        }

+        fclose(stream);

+    }

+    return cores? cores : 1;

+}

-- 

1.8.3.1

From 0307614c92664078ffae0566324de85c2f990353 Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

Date: Fri, 28 Apr 2017 14:41:50 -0500

Subject: [PATCH 4/5] Low: crmd: remove I/O load checks

Due to bugs, the crmd's throttling checks for I/O load and blocked processes

would always get 0. In any case, both are already included in the load average

checked elsewhere, so there is no need to check them.

---

 crmd/throttle.c | 88 ---------------------------------------------------------

 1 file changed, 88 deletions(-)

diff --git a/crmd/throttle.c b/crmd/throttle.c

index 8ab6f01..387e58d 100644

--- a/crmd/throttle.c

+++ b/crmd/throttle.c

@@ -268,88 +268,6 @@ throttle_load_avg(float *load)

     return FALSE;

 }

-static bool

-throttle_io_load(float *load, unsigned int *blocked)

-{

-    char buffer[64*1024];

-    FILE *stream = NULL;

-    const char *loadfile = "/proc/stat";

-

-    if(load == NULL) {

-        return FALSE;

-    }

-

-    stream = fopen(loadfile, "r");

-    if(stream == NULL) {

-        int rc = errno;

-        crm_warn("Couldn't read %s: %s (%d)", loadfile, pcmk_strerror(rc), rc);

-        return FALSE;

-    }

-

-    if(fgets(buffer, sizeof(buffer), stream)) {

-        /* Borrowed from procps-ng's sysinfo.c */

-

-        char *b = NULL;

-        unsigned long long cpu_use = 0;

-        unsigned long long cpu_nic = 0;

-        unsigned long long cpu_sys = 0;

-        unsigned long long cpu_idl = 0;

-        unsigned long long cpu_iow = 0; /* not separated out until the 2.5.41 kernel */

-        unsigned long long cpu_xxx = 0; /* not separated out until the 2.6.0-test4 kernel */

-        unsigned long long cpu_yyy = 0; /* not separated out until the 2.6.0-test4 kernel */

-        unsigned long long cpu_zzz = 0; /* not separated out until the 2.6.11 kernel */

-

-        long long divo2 = 0;

-        long long duse = 0;

-        long long dsys = 0;

-        long long didl =0;

-        long long diow =0;

-        long long dstl = 0;

-        long long Div = 0;

-

-        b = strstr(buffer, "cpu ");

-        if(b) sscanf(b,  "cpu  %Lu %Lu %Lu %Lu %Lu %Lu %Lu %Lu",

-               &cpu_use, &cpu_nic, &cpu_sys, &cpu_idl, &cpu_iow, &cpu_xxx, &cpu_yyy, &cpu_zzz);

-

-        if(blocked) {

-            b = strstr(buffer, "procs_blocked ");

-            if(b) sscanf(b,  "procs_blocked %u", blocked);

-        }

-

-        duse = cpu_use + cpu_nic;

-        dsys = cpu_sys + cpu_xxx + cpu_yyy;

-        didl = cpu_idl;

-        diow = cpu_iow;

-        dstl = cpu_zzz;

-        Div = duse + dsys + didl + diow + dstl;

-        if (!Div) Div = 1, didl = 1;

-        divo2 = Div / 2UL;

-

-        /* vmstat output:

-         *

-         * procs -----------memory---------- ---swap-- -----io---- -system-- ----cpu---- 

-         * r  b   swpd   free   buff  cache     si   so    bi    bo   in   cs us sy id wa

-         * 1  0 5537800 958592 204180 1737740    1    1    12    15    0    0  2  1 97  0

-         *

-         * The last four columns are calculated as:

-         *

-         * (unsigned)( (100*duse			+ divo2) / Div ),

-         * (unsigned)( (100*dsys			+ divo2) / Div ),

-         * (unsigned)( (100*didl			+ divo2) / Div ),

-         * (unsigned)( (100*diow			+ divo2) / Div )

-         *

-         */

-        *load = (diow + divo2) / Div;

-        crm_debug("Current IO load is %f", *load);

-

-        fclose(stream);

-        return TRUE;

-    }

-

-    fclose(stream);

-    return FALSE;

-}

-

 /*!

  * \internal

  * \brief Check a load value against throttling thresholds

@@ -412,7 +330,6 @@ throttle_mode(void)

     int cores;

     float load;

     float thresholds[4];

-    unsigned int blocked = 0;

     enum throttle_state_e mode = throttle_none;

 #if defined(ON_BSD) || defined(ON_SOLARIS)

@@ -459,11 +376,6 @@ throttle_mode(void)

         mode |= throttle_handle_load(load, "CPU load", cores);

     }

-    if(throttle_io_load(&load, &blocked)) {

-        mode |= throttle_handle_load(load, "IO load", 0);

-        mode |= throttle_handle_load(blocked, "blocked IO ratio", cores);

-    }

-

     if(mode & throttle_extreme) {

         return throttle_extreme;

     } else if(mode & throttle_high) {

-- 

1.8.3.1

From da00177e186dc7b7e63fecb7e0d461003eda2eea Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

Date: Fri, 28 Apr 2017 14:56:12 -0500

Subject: [PATCH 5/5] Fix: crmd,libcrmcommon: update throttle when CPUs are

 hot-plugged

Previously, the number of CPU cores was determined the first time it was

needed, and remembered permanently after that. That becomes inaccurate when

cores are hot-plugged in and out of a virtual machine. Now, the number of cores

is parsed each time it is needed (using the new libcrmcommon function).

---

 crmd/throttle.c | 42 ++++--------------------------------------

 1 file changed, 4 insertions(+), 38 deletions(-)

diff --git a/crmd/throttle.c b/crmd/throttle.c

index 387e58d..90ddb90 100644

--- a/crmd/throttle.c

+++ b/crmd/throttle.c

@@ -57,40 +57,6 @@ static float throttle_load_target = 0.0;

 static GHashTable *throttle_records = NULL;

 static mainloop_timer_t *throttle_timer = NULL;

-static int

-throttle_num_cores(void)

-{

-    static int cores = 0;

-    char buffer[256];

-    FILE *stream = NULL;

-    const char *cpufile = "/proc/cpuinfo";

-

-    if(cores) {

-        return cores;

-    }

-    stream = fopen(cpufile, "r");

-    if(stream == NULL) {

-        int rc = errno;

-        crm_warn("Couldn't read %s, assuming a single processor: %s (%d)", cpufile, pcmk_strerror(rc), rc);

-        return 1;

-    }

-

-    while (fgets(buffer, sizeof(buffer), stream)) {

-        if(strstr(buffer, "processor") == buffer) {

-            cores++;

-        }

-    }

-

-    fclose(stream);

-

-    if(cores == 0) {

-        crm_warn("No processors found in %s, assuming 1", cpufile);

-        return 1;

-    }

-

-    return cores;

-}

-

 /*!

  * \internal

  * \brief Return name of /proc file containing the CIB deamon's load statistics

@@ -259,7 +225,6 @@ throttle_load_avg(float *load)

         *load = strtof(buffer, NULL);

         if(nl) { nl[0] = 0; }

-        crm_debug("Current load is %f (full: %s)", *load, buffer);

         fclose(stream);

         return TRUE;

     }

@@ -327,7 +292,7 @@ throttle_handle_load(float load, const char *desc, int cores)

 static enum throttle_state_e

 throttle_mode(void)

 {

-    int cores;

+    unsigned int cores;

     float load;

     float thresholds[4];

     enum throttle_state_e mode = throttle_none;

@@ -336,7 +301,7 @@ throttle_mode(void)

     return throttle_none;

 #endif

-    cores = throttle_num_cores();

+    cores = crm_procfs_num_cores();

     if(throttle_cib_load(&load)) {

         float cib_max_cpu = 0.95;

@@ -373,6 +338,7 @@ throttle_mode(void)

     }

     if(throttle_load_avg(&load)) {

+        crm_debug("Current load is %f across %u core(s)", load, cores);

         mode |= throttle_handle_load(load, "CPU load", cores);

     }

@@ -449,7 +415,7 @@ throttle_update_job_max(const char *preference)

 {

     int max = 0;

-    throttle_job_max = 2 * throttle_num_cores();

+    throttle_job_max = 2 * crm_procfs_num_cores();

     if(preference) {

         /* Global preference from the CIB */

-- 

1.8.3.1