hugepages: rework

Changed hugepage metric type and type instances.
Made a single entry in types.db for "free" and
    "used" hugepages.
Calculate the used hugepages.
Removed read_syshugepage_dir; replaced with
    walk_directory.
Cleared up descriptions in .pod.
Made the .conf plugin option names less cryptic.
Linux-only plugin; changed configure.ac accordingly.
Stripped function declarations and header includes
    that are not used.
Tidied up globals and made some globals local.
Removed some hardcoded values and fixed spacing.
Other general tidy-ups.

Change-Id: Id0243397d3fd4ac1be90266b266341318c05c903
Signed-off-by: Kim Jones <kim-marie.jones@intel.com>

diff --git a/README b/README
index 1693c75ef04549f748f630c67bf0b8e697febd3f..e97140bab32f84e4bc5ee59c5a22ab57203d54c1 100644 (file)
--- a/README
+++ b/README
@@ -129,8 +129,7 @@ Features
       Hard disk temperatures using hddtempd.
 
     - hugepages
-      Report the total number of hugpages, the number of free/reserved
-      hugepages, and the size of the available hugepages. More info on
+      Report the number of used and free hugepages. More info on
       hugepages can be found here:
       https://www.kernel.org/doc/Documentation/vm/hugetlbpage.txt.
 

diff --git a/configure.ac b/configure.ac
index a69f16bb4d5a43336a4de356cdf233f704819b94..c52950bf1fd7ab13e99dedc1ab92d734691d8e68 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -482,9 +482,6 @@ fi
 # For hddtemp module
 AC_CHECK_HEADERS(linux/major.h)
 
-# For hugepages module
-AC_CHECK_HEADERS(dirent.h)
-
 # For md module (Linux only)
 if test "x$ac_system" = "xLinux"
 then
@@ -5549,6 +5546,7 @@ plugin_entropy="no"
 plugin_ethstat="no"
 plugin_fhcount="no"
 plugin_fscache="no"
+plugin_hugepages="no"
 plugin_interface="no"
 plugin_ipmi="no"
 plugin_ipvs="no"
@@ -5595,6 +5593,7 @@ then
        plugin_entropy="yes"
        plugin_fhcount="yes"
        plugin_fscache="yes"
+       plugin_hugepages="yes"
        plugin_interface="yes"
        plugin_ipc="yes"
        plugin_irq="yes"
@@ -5974,7 +5973,7 @@ AC_PLUGIN([fscache],             [$plugin_fscache],         [fscache statistics]
 AC_PLUGIN([gmond],               [$with_libganglia],        [Ganglia plugin])
 AC_PLUGIN([grpc],                [$with_grpc],              [gRPC plugin])
 AC_PLUGIN([hddtemp],             [yes],                     [Query hddtempd])
-AC_PLUGIN([hugepages],           [yes],                     [Hugepages statistics])
+AC_PLUGIN([hugepages],           [$plugin_hugepages],       [Hugepages statistics])
 AC_PLUGIN([interface],           [$plugin_interface],       [Interface traffic statistics])
 AC_PLUGIN([ipc],                 [$plugin_ipc],             [IPC statistics])
 AC_PLUGIN([ipmi],                [$plugin_ipmi],            [IPMI sensor statistics])

diff --git a/src/Makefile.am b/src/Makefile.am
index 0cc7e1cbaf22b8aea1380ae7231c5d67b228c638..587c9a4e18ea18c72b38943d63a63c4071eda54c 100644 (file)
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -446,7 +446,6 @@ if BUILD_PLUGIN_HUGEPAGES
 pkglib_LTLIBRARIES += hugepages.la
 hugepages_la_SOURCES = hugepages.c
 hugepages_la_LDFLAGS = $(PLUGIN_LDFLAGS)
-hugepages_la_LIBADD =
 endif
 
 if BUILD_PLUGIN_INTERFACE

diff --git a/src/collectd.conf.in b/src/collectd.conf.in
index 2b57a6873065ad3bb703ee6994c14b2c61fc8437..998e2117a7a479062ab60fe11e4244e2648f0913 100644 (file)
--- a/src/collectd.conf.in
+++ b/src/collectd.conf.in
@@ -581,8 +581,8 @@
 #</Plugin>
 
 #<Plugin hugepages>
-#    EnableNuma "true"
-#    EnableMM "true"
+#    ReportPerNodeHP "true"
+#    ReportRootHP "true"
 #</Plugin>
 
 #<Plugin interface>

diff --git a/src/collectd.conf.pod b/src/collectd.conf.pod
index 0a7bd0144abe57867d875dde4a86705695c477ce..65fdc53d2f099f4d13cac929ffaa9c1df7aa63db 100644 (file)
--- a/src/collectd.conf.pod
+++ b/src/collectd.conf.pod
@@ -2701,21 +2701,27 @@ TCP-Port to connect to. Defaults to B<7634>.
 
 =head2 Plugin C<hugepages>
 
-To get values from B<hugepages> collectd read directories
+To collect B<hugepages> information, collectd reads directories
 "/sys/devices/system/node/*/hugepages" and
-"/sys/kernel/mm/hugepages"
+"/sys/kernel/mm/hugepages".
 Reading of these directories can be disabled by following
 options. Default is enabled.
 
 =over 4
 
-=item B<EnableNuma> I<true>|I<false>
+=item B<ReportPerNodeHP> I<true>|I<false>
 
-Enable/Disable reading stats in "/sys/devices/system/node/*/hugepages"
+If enabled, information will be collected from the hugepage
+counters in "/sys/devices/system/node/*/hugepages".
+This is used to check the per-node hugepage statistics on
+a NUMA system.
 
-=item B<EnableMM> I<true>|I<false>
+=item B<ReportRootHP> I<true>|I<false>
 
-Enable/Disable reading stats in "/sys/kernel/mm/hugepages"
+If enabled, information will be collected from the hugepage
+counters in "/sys/kernel/mm/hugepages".
+This can be used on both NUMA and non-NUMA systems to check
+the overall hugepage statistics.
 
 =back
 

diff --git a/src/hugepages.c b/src/hugepages.c
index fb736a6a991758554af96e211d70b55059e83804..ee43237032955608270bb662c7f39737cddc4afd 100644 (file)
--- a/src/hugepages.c
+++ b/src/hugepages.c
@@ -24,65 +24,59 @@
  *
  * Authors:
  *   Jaroslav Safka <jaroslavx.safka@intel.com>
+ *   Kim-Marie Jones <kim-marie.jones@intel.com>
  */
 
 #include "collectd.h"
 #include "common.h" /* auxiliary functions */
 #include "plugin.h" /* plugin_register_*, plugin_dispatch_values */
 
-#include <stdio.h>
-#include <string.h>
-#include <dirent.h>
-
-static int huge_read (void);
-static int huge_config_callback (const char *key, const char *val);
-
-static const char PLUGIN_NAME[] = "hugepages";
-static const char SYS_NODE[] = "/sys/devices/system/node";
-static const char NODE[] = "node";
-static const char HUGEPAGES_DIR[] = "hugepages";
-static const char SYS_NODE_HUGEPAGES[] = "/sys/devices/system/node/%s/hugepages";
-static const char SYS_MM_HUGEPAGES[] = "/sys/kernel/mm/hugepages";
-static const char CONFIG_NAME[] = "hugepages";
-static const char CFG_ENA_NUMA[] = "EnableNuma";
-static const char CFG_ENA_MM[] = "EnableMM";
-
-static const char *CONFIG_KEYS[] = {
-  CFG_ENA_NUMA,
-  CFG_ENA_MM,
+static const char g_plugin_name[] = "hugepages";
+static const char g_cfg_rpt_numa[] = "ReportPerNodeHP";
+static const char g_cfg_rpt_mm[] = "ReportRootHP";
+
+static const char *g_config_keys[] = {
+  g_cfg_rpt_numa,
+  g_cfg_rpt_mm,
+};
+static size_t g_config_keys_num = STATIC_ARRAY_SIZE(g_config_keys);
+static int g_flag_rpt_numa = 1;
+static int g_flag_rpt_mm = 1;
+
+struct entry_info {
+  char *d_name;
+  char *node;
 };
-static const size_t CONFIG_KEYS_NUM = sizeof(CONFIG_KEYS)/sizeof(*CONFIG_NAME);
-static int g_config_ena_numa = 1;
-static int g_config_ena_mm = 1;
 
-static int huge_config_callback (const char *key, const char *val)
+static int huge_config_callback(const char *key, const char *val)
 {
-  INFO("HugePages config key='%s', val='%s'", key, val);
+  INFO("%s: HugePages config key='%s', val='%s'", g_plugin_name, key, val);
 
-  if (0 == strcasecmp(key, CFG_ENA_NUMA)) {
-    g_config_ena_numa = IS_TRUE(val);
+  if (strcasecmp(key, g_cfg_rpt_numa) == 0) {
+    g_flag_rpt_numa = IS_TRUE(val);
     return 0;
   }
-  if (0 == strcasecmp(key, CFG_ENA_MM)) {
-    g_config_ena_mm = IS_TRUE(val);
+  if (strcasecmp(key, g_cfg_rpt_mm) == 0) {
+    g_flag_rpt_mm = IS_TRUE(val);
     return 0;
   }
 
   return -1;
 }
 
-static void submit_one (const char *plug_inst, const char *type,
-  const char *type_instance, derive_t value)
+static void submit_hp(const char *plug_inst, const char *type,
+  const char *type_instance, gauge_t free_value, gauge_t used_value)
 {
-  value_t values[1];
+  value_t values[2];
   value_list_t vl = VALUE_LIST_INIT;
 
-  values[0].derive = value;
+  values[0].gauge = free_value;
+  values[1].gauge = used_value;
 
   vl.values = values;
-  vl.values_len = 1;
+  vl.values_len = 2;
   sstrncpy (vl.host, hostname_g, sizeof (vl.host));
-  sstrncpy (vl.plugin, PLUGIN_NAME, sizeof (vl.plugin));
+  sstrncpy (vl.plugin, g_plugin_name, sizeof (vl.plugin));
   sstrncpy (vl.plugin_instance, plug_inst, sizeof (vl.plugin_instance));
   sstrncpy (vl.type, type, sizeof (vl.type));
 
@@ -90,128 +84,114 @@ static void submit_one (const char *plug_inst, const char *type,
     sstrncpy (vl.type_instance, type_instance, sizeof (vl.type_instance));
   }
 
-  DEBUG("submit_one pl_inst:%s, inst_type %s, type %s, val=%"PRIu64"",
-      plug_inst, type_instance, type, value);
+  DEBUG("submit_hp pl_inst:%s, inst_type %s, type %s, free=%lf, used=%lf",
+      plug_inst, type_instance, type, free_value, used_value);
 
   plugin_dispatch_values (&vl);
 }
 
-static int read_hugepage_entry(const char *path, const char* entry,
-    const char* plinst, const char* hpsize)
+static int read_hugepage_entry(const char *path, const char *entry,
+    void *e_info)
 {
-  char path2[512];
-  long value = 0;
-  snprintf(path2, sizeof(path2), "%s/%s", path, entry);
+  char path2[PATH_MAX];
+  char *type = "hugepages";
+  char *partial_type_inst = "free_used";
+  char type_instance[PATH_MAX];
+  char *strin = NULL;
+  struct entry_info *hpsize_plinst = (struct entry_info *) e_info;
+  static int flag = 0;
+  static double used_hp = 0;
+  static double free_hp = 0;
+  double value;
+
+  ssnprintf(path2, sizeof(path2), "%s/%s", path, entry);
 
   FILE *fh = fopen(path2, "rt");
-  if (NULL == fh) {
-    ERROR("Cannot open %s", path2);
+  if (fh == NULL) {
+    ERROR("%s: cannot open %s", g_plugin_name, path2);
     return -1;
   }
 
-  if (fscanf(fh, "%ld", &value) !=1) {
-    ERROR("Cannot parse file %s", path2);
+  if (fscanf(fh, "%lf", &value) !=1) {
+    ERROR("%s: cannot parse file %s", g_plugin_name, path2);
     fclose(fh);
     return -1;
   }
 
-  submit_one (plinst, entry, hpsize, value);
-
-  fclose(fh);
-  return 0;
-}
-
-static int read_syshugepage_dir(const char* path, const char* dirhpsize,
-    const char* node)
-{
-  DIR       *dir = NULL;
-  struct dirent *entry = NULL;
-  struct dirent *result = NULL;
-  size_t name_max = 0;
-  size_t len = 0;
-
-  dir = opendir(path);
-  if (NULL == dir) {
-    ERROR("Cannot open directory %s", path);
-    return -1;
-  }
-
-  name_max = pathconf(path, _PC_NAME_MAX);
-  if (name_max == -1) {    /* Limit not defined, or error */
-    name_max = 255;     /* Take a guess */
-  }
-
-  len = offsetof(struct dirent, d_name) + name_max + 1;
-  entry = malloc(len);
-  if (entry == NULL) {
-    ERROR("Malloc returned NULL");
-    return -1;
+  if (strcmp(entry, "nr_hugepages") == 0) {
+    used_hp += value;
+    flag++;
+  } else if (strcmp(entry, "surplus_hugepages") == 0) {
+    used_hp += value;
+    flag++;
+  } else if (strcmp(entry, "free_hugepages") == 0) {
+    used_hp -= value;
+    free_hp = value;
+    flag++;
   }
 
-  while (0 == readdir_r(dir, entry, &result)) {
-    if (NULL == result) {
-      /* end of dir */
-      break;
-    }
-    if (result->d_name[0] == '.') {
-      /* not interesting "." and ".." */
-      continue;
+  if (flag == 3) {
+    /* Can now submit "used" and "free" values.
+     * 0x2D is the ASCII "-" character, after which the string
+     *   contains "<size>kB"
+     * The string passed as param 3 to submit_hp is of the format:
+     *   <type>-<partial_type_inst>-<size>kB
+     */
+    strin = strchr(hpsize_plinst->d_name, 0x2D);
+    if (strin != NULL) {
+      ssnprintf(type_instance, sizeof(type_instance), "%s%s", partial_type_inst, strin);
+    } else {
+      ssnprintf(type_instance, sizeof(type_instance), "%s%s", partial_type_inst,
+          hpsize_plinst->d_name);
     }
+    submit_hp(hpsize_plinst->node, type, type_instance, free_hp, used_hp);
 
-    read_hugepage_entry(path, result->d_name, node, dirhpsize);
+    /* Reset for next time */
+    flag = 0;
+    used_hp = 0;
+    free_hp = 0;
   }
 
-  free(entry);
-  closedir(dir);
-
-
+  fclose(fh);
   return 0;
 }
 
 static int read_syshugepages(const char* path, const char* node)
 {
-  DIR       *dir = NULL;
-  struct dirent *entry = NULL;
+  const char hugepages_dir[] = "hugepages";
+  DIR *dir = NULL;
   struct dirent *result = NULL;
-  size_t name_max = 0;
-  size_t len = 0;
-  char path2[255];
+  char path2[PATH_MAX];
+  struct entry_info e_info;
 
   dir = opendir(path);
-  if (NULL == dir) {
-    ERROR("Cannot open directory %s", path);
+  if (dir == NULL) {
+    ERROR("%s: cannot open directory %s", g_plugin_name, path);
     return -1;
   }
 
-  name_max = pathconf(path, _PC_NAME_MAX);
-  if (name_max == -1) {    /* Limit not defined, or error */
-    name_max = 255;     /* Take a guess */
-  }
-  len = offsetof(struct dirent, d_name) + name_max + 1;
-  entry = malloc(len);
-  if (entry == NULL) {
-    ERROR("Malloc returned NULL");
+  if (pathconf(path, _PC_NAME_MAX) == -1) {
+    /* Limit not defined, or error */
+    ERROR("%s: pathconf failed", g_plugin_name);
+    closedir(dir);
     return -1;
   }
 
-  while (0 == readdir_r(dir, entry, &result)) {
-    /* read "hugepages-XXXXXkB" entries */
-    if (NULL == result) {
-      /* end of dir */
-      break;
-    }
-
-    if (strncmp(result->d_name, HUGEPAGES_DIR, sizeof(HUGEPAGES_DIR)-1)) {
+  /* read "hugepages-XXXXXkB" entries */
+  while ((result = readdir(dir)) != NULL) {
+    if (strncmp(result->d_name, hugepages_dir, sizeof(hugepages_dir)-1)) {
       /* not node dir */
       continue;
     }
 
     /* /sys/devices/system/node/node?/hugepages/ */
-    snprintf(path2, sizeof(path2), "%s/%s", path, result->d_name);
-    read_syshugepage_dir(path2, result->d_name, node);
+    ssnprintf(path2, sizeof(path2), "%s/%s", path, result->d_name);
+
+    e_info.d_name = result->d_name;
+    e_info.node = (char *) node;
+    walk_directory(path2, read_hugepage_entry, (void *) &e_info, 0);
   }
 
-  free(entry);
   closedir(dir);
 
   return 0;
@@ -219,68 +199,60 @@ static int read_syshugepages(const char* path, const char* node)
 
 static int read_nodes(void)
 {
-  DIR       *dir = NULL;
-  struct dirent *entry = NULL;
+  const char sys_node[] = "/sys/devices/system/node";
+  const char node_string[] = "node";
+  const char sys_node_hugepages[] = "/sys/devices/system/node/%s/hugepages";
+  DIR *dir = NULL;
   struct dirent *result = NULL;
-  size_t name_max = 0;
-  size_t len = 0;
-  char path[255];
+  char path[PATH_MAX];
 
-  dir = opendir(SYS_NODE);
-  if (NULL == dir) {
-    ERROR("Cannot open directory %s", SYS_NODE);
+  dir = opendir(sys_node);
+  if (dir == NULL) {
+    ERROR("%s: cannot open directory %s", g_plugin_name, sys_node);
     return -1;
   }
 
-  name_max = pathconf(SYS_NODE, _PC_NAME_MAX);
-  if (name_max == -1) {    /* Limit not defined, or error */
-    name_max = 255;     /* Take a guess */
-  }
-  len = offsetof(struct dirent, d_name) + name_max + 1;
-  entry = malloc(len);
-  if (entry == NULL) {
-    ERROR("Malloc returned NULL");
+  if (pathconf(sys_node, _PC_NAME_MAX) == -1) {
+    /* Limit not defined, or error */
+    ERROR("%s: pathconf failed", g_plugin_name);
+    closedir(dir);
     return -1;
   }
 
-  while (0 == readdir_r(dir, entry, &result)) {
-    if (NULL == result) {
-      /* end of dir */
-      break;
-    }
-
-    if (strncmp(result->d_name, NODE, sizeof(NODE)-1)) {
+  while ((result = readdir(dir)) != NULL) {
+    if (strncmp(result->d_name, node_string, sizeof(node_string)-1)) {
       /* not node dir */
       continue;
     }
 
-    snprintf(path, sizeof(path), SYS_NODE_HUGEPAGES, result->d_name);
+    ssnprintf(path, sizeof(path), sys_node_hugepages, result->d_name);
     read_syshugepages(path, result->d_name);
   }
 
-  free(entry);
   closedir(dir);
 
   return 0;
 }
 
 
-static int huge_read (void)
+static int huge_read(void)
 {
-  if (g_config_ena_mm) {
-    read_syshugepages(SYS_MM_HUGEPAGES, "mm");
+  const char sys_mm_hugepages[] = "/sys/kernel/mm/hugepages";
+
+  if (g_flag_rpt_mm) {
+    read_syshugepages(sys_mm_hugepages, "mm");
   }
-  if (g_config_ena_numa) {
+  if (g_flag_rpt_numa) {
     read_nodes();
   }
 
   return 0;
 }
 
-void module_register (void)
+void module_register(void)
 {
-       plugin_register_config(CONFIG_NAME, huge_config_callback, CONFIG_KEYS,
-                         CONFIG_KEYS_NUM);
-  plugin_register_read (PLUGIN_NAME, huge_read);
+  plugin_register_config(g_plugin_name, huge_config_callback, g_config_keys,
+      g_config_keys_num);
+  plugin_register_read(g_plugin_name, huge_read);
 }
 

diff --git a/src/types.db b/src/types.db
index 8a1581a4e3b6cd7ac15f6b775f9e7acd87501b13..ff103d1f7238f2ca2a4d38470aebd140493983f6 100644 (file)
--- a/src/types.db
+++ b/src/types.db
@@ -82,7 +82,6 @@ file_size               value:GAUGE:0:U
 files                   value:GAUGE:0:U
 flow                    value:GAUGE:0:U
 fork_rate               value:DERIVE:0:U
-free_hugepages          value:DERIVE:0:U
 frequency               value:GAUGE:0:U
 frequency_error         value:GAUGE:-2:2 
 frequency_offset        value:GAUGE:-1000000:1000000
@@ -92,6 +91,7 @@ hash_collisions         value:DERIVE:0:U
 http_request_methods    value:DERIVE:0:U
 http_requests           value:DERIVE:0:U
 http_response_codes     value:DERIVE:0:U
+hugepages               free:GAUGE:0:4294967295, used:GAUGE:0:4294967295
 humidity                value:GAUGE:0:100
 if_collisions           value:DERIVE:0:U
 if_dropped              rx:DERIVE:0:U, tx:DERIVE:0:U
@@ -149,9 +149,6 @@ node_octets             rx:DERIVE:0:U, tx:DERIVE:0:U
 node_rssi               value:GAUGE:0:255
 node_stat               value:DERIVE:0:U
 node_tx_rate            value:GAUGE:0:127
-nr_overcommit_hugepages value:DERIVE:0:U
-nr_hugepages            value:DERIVE:0:U
-nr_hugepages_mempolicy  value:DERIVE:0:U
 objects                 value:GAUGE:0:U
 operations              value:DERIVE:0:U
 packets                 value:DERIVE:0:U
@@ -195,7 +192,6 @@ records                 value:GAUGE:0:U
 requests                value:GAUGE:0:U
 response_code           value:GAUGE:0:U
 response_time           value:GAUGE:0:U
-resv_hugepages          value:DERIVE:0:U
 root_delay              value:GAUGE:U:U
 root_dispersion         value:GAUGE:U:U
 route_etx               value:GAUGE:0:U
@@ -215,7 +211,6 @@ snr                     value:GAUGE:0:U
 spam_check              value:GAUGE:0:U
 spam_score              value:GAUGE:U:U
 spl                     value:GAUGE:U:U
-surplus_hugepages       value:DERIVE:0:U
 swap                    value:GAUGE:0:1099511627776
 swap_io                 value:DERIVE:0:U
 tcp_connections         value:GAUGE:0:4294967295