From a882a3ff25bcae8d703277ebd850fde0b1128ce9 Mon Sep 17 00:00:00 2001 From: Ken Gaillot 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 -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 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 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 #include #include +#include /*! * \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 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 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