Blob Blame History Raw
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