Misc cleanups in the new mlock code, incl. improving some variable names

& messages; using more statics (for now) to avoid redundant
recalculation; validating config file just once on loading; keeping maps
file open.

diff --git a/lib/commands/toolcontext.c b/lib/commands/toolcontext.c
index 99db7d0dd25f7a0fd2639bf037d5b3c4ba2339eb..7aac3619fbf8f1ad6bd2179ffcb2877d2298e5bb 100644 (file)
--- a/lib/commands/toolcontext.c
+++ b/lib/commands/toolcontext.c
@@ -201,6 +201,8 @@ static int _process_config(struct cmd_context *cmd)
        mode_t old_umask;
        const char *read_ahead;
        struct stat st;
+       const struct config_node *cn;
+       struct config_value *cv;
 
        /* umask */
        cmd->default_settings.umask = find_config_tree_int(cmd,
@@ -303,6 +305,11 @@ static int _process_config(struct cmd_context *cmd)
                                                  "global/si_unit_consistency",
                                                  DEFAULT_SI_UNIT_CONSISTENCY);
 
+       if ((cn = find_config_tree_node(cmd, "activation/mlock_filter")))
+               for (cv = cn->v; cv; cv = cv->next) 
+                       if ((cv->type != CFG_STRING) || !cv->v.str[0]) 
+                               log_error("Ignoring invalid activation/mlock_filter entry in config file");
+
        return 1;
 }
 

diff --git a/lib/mm/memlock.c b/lib/mm/memlock.c
index 5ab54ce93e1682a2fc52fcc0b38de3b4e80b5511..ac9811a3ce6a44dfab492a065426df173f8b21ef 100644 (file)
--- a/lib/mm/memlock.c
+++ b/lib/mm/memlock.c
@@ -75,12 +75,17 @@ static const char * const _blacklist_maps[] = {
 
 typedef enum { LVM_MLOCK, LVM_MUNLOCK } lvmlock_t;
 
+static unsigned _use_mlockall;
+static FILE *_mapsh;
+static char _procselfmaps[PATH_MAX] = "";
+static const char _selfmaps[] = "/self/maps";
+
 struct maps_stats {
        size_t r_size;
        size_t w_size;
        size_t x_size;
 };
-static struct maps_stats _ms; /* statistic for maps locking */
+static struct maps_stats _mstats; /* statistic for maps locking */
 
 static void _touch_memory(void *mem, size_t size)
 {
@@ -120,7 +125,7 @@ static void _release_memory(void)
  * format described in kernel/Documentation/filesystem/proc.txt
  */
 static int _maps_line(struct cmd_context *cmd, lvmlock_t lock,
-                     const char* line, struct maps_stats* ms)
+                     const char* line, struct maps_stats* mstats)
 {
        const struct config_node *cn;
        struct config_value *cv;
@@ -145,27 +150,20 @@ static int _maps_line(struct cmd_context *cmd, lvmlock_t lock,
                        return 1;
 
        sz = to - from;
-       log_debug("%s %10ldKiB %12lx - %12lx %c%c%c%c %s",
-                 (lock == LVM_MLOCK) ? "Mlock" : "Munlock",
-                 ((long)sz + 1023) / 1024, from, to, fr, fw, fx, fp, line + pos);
-
        if (!(cn = find_config_tree_node(cmd, "activation/mlock_filter"))) {
                /* If no blacklist configured, use an internal set */
                for (i = 0; i < sizeof(_blacklist_maps) / sizeof(_blacklist_maps[0]); ++i)
                        if (strstr(line + pos, _blacklist_maps[i])) {
-                               log_debug("Filtered by string '%s' (%s)",
+                               log_debug("mlock default filter '%s' matches '%s': Skipping.",
                                          _blacklist_maps[i], line);
                                return 1;
                        }
        } else {
                for (cv = cn->v; cv; cv = cv->next) {
-                       if ((cv->type != CFG_STRING) || !cv->v.str[0]) {
-                               log_error("Ignoring invalid string in config file "
-                                         "activation/mlock_filter");
+                       if ((cv->type != CFG_STRING) || !cv->v.str[0])
                                continue;
-                       }
                        if (strstr(line + pos, cv->v.str)) {
-                               log_debug("Filtered by string '%s' (%s)",
+                               log_debug("mlock_filter '%s' matches '%s': Skipping.",
                                          cv->v.str, line);
                                return 1;
                        }
@@ -173,11 +171,15 @@ static int _maps_line(struct cmd_context *cmd, lvmlock_t lock,
        }
 
        if (fr == 'r')
-               ms->r_size += sz;
+               mstats->r_size += sz;
        if (fw == 'w')
-               ms->w_size += sz;
+               mstats->w_size += sz;
        if (fx == 'x')
-               ms->x_size += sz;
+               mstats->x_size += sz;
+
+       log_debug("%s %10ldKiB %12lx - %12lx %c%c%c%c %s",
+                 (lock == LVM_MLOCK) ? "mlock" : "munlock",
+                 ((long)sz + 1023) / 1024, from, to, fr, fw, fx, fp, line + pos);
 
        if (lock == LVM_MLOCK) {
                if (mlock((const void*)from, sz) < 0) {
@@ -194,18 +196,14 @@ static int _maps_line(struct cmd_context *cmd, lvmlock_t lock,
        return 1;
 }
 
-static int _memlock_maps(struct cmd_context *cmd, lvmlock_t lock, struct maps_stats* ms)
+static int _memlock_maps(struct cmd_context *cmd, lvmlock_t lock, struct maps_stats *mstats)
 {
-       static const char selfmaps[] = "/self/maps";
-       char *procselfmaps = alloca(strlen(cmd->proc_dir) + sizeof(selfmaps));
-       FILE *fh;
        char *line = NULL;
        size_t len;
-       ssize_t r;
+       ssize_t n;
        int ret = 0;
 
-       if (find_config_tree_bool(cmd, "activation/use_mlockall",
-                                 DEFAULT_USE_MLOCKALL)) {
+       if (_use_mlockall) {
 #ifdef MCL_CURRENT
                if (lock == LVM_MLOCK) {
                        if (mlockall(MCL_CURRENT | MCL_FUTURE)) {
@@ -224,25 +222,16 @@ static int _memlock_maps(struct cmd_context *cmd, lvmlock_t lock, struct maps_st
 #endif
        }
 
-       strcpy(procselfmaps, cmd->proc_dir);
-       strcat(procselfmaps, selfmaps);
-
-       if ((fh = fopen(procselfmaps, "r")) == NULL) {
-               log_sys_error("fopen", procselfmaps);
-               return 0;
-       }
-
-       while ((r = getline(&line, &len, fh)) != -1) {
-               line[r > 0 ? r - 1 : 0] = '\0'; /* remove \n */
-               if (!(ret = _maps_line(cmd, lock, line, ms)))
+       while ((n = getline(&line, &len, _mapsh)) != -1) {
+               line[n > 0 ? n - 1 : 0] = '\0'; /* remove \n */
+               if (!(ret = _maps_line(cmd, lock, line, mstats)))
                        break;
        }
 
        free(line);
-       fclose(fh);
 
-       log_debug("Mapped sizes:  r=%ld,  w=%ld,  x=%ld",
-                 (long)ms->r_size,  (long)ms->w_size, (long)ms->x_size);
+       log_debug("Mapped sizes: r=%ld, w=%ld, x=%ld",
+                 (long)mstats->r_size,  (long)mstats->w_size, (long)mstats->x_size);
 
        return ret;
 }
@@ -252,9 +241,27 @@ static void _lock_mem(struct cmd_context *cmd)
 {
        _allocate_memory();
 
-       memset(&_ms, 0, sizeof(_ms));
-       if (_memlock_maps(cmd, LVM_MLOCK, &_ms))
-               log_very_verbose("Locking memory");
+       _use_mlockall = find_config_tree_bool(cmd, "activation/use_mlockall", DEFAULT_USE_MLOCKALL);
+
+       if (!_use_mlockall) {
+               /* Initialise static variables first time */
+               memset(&_mstats, 0, sizeof(_mstats));
+
+               if (!*_procselfmaps) {
+                       _procselfmaps[PATH_MAX - 1] = '\0';
+                       strncpy(_procselfmaps, cmd->proc_dir, PATH_MAX - 1);
+                       strncat(_procselfmaps, _selfmaps, PATH_MAX - 1);
+               }
+
+               if (!(_mapsh = fopen(_procselfmaps, "r"))) {
+                       log_sys_error("fopen", _procselfmaps);
+                       return;
+               }
+       }
+
+       log_very_verbose("Locking memory");
+       if (!_memlock_maps(cmd, LVM_MLOCK, &_mstats))
+               stack;
 
        errno = 0;
        if (((_priority = getpriority(PRIO_PROCESS, 0)) == -1) && errno)
@@ -267,15 +274,25 @@ static void _lock_mem(struct cmd_context *cmd)
 
 static void _unlock_mem(struct cmd_context *cmd)
 {
-       struct maps_stats ums = { 0 };
+       struct maps_stats unlock_mstats = { 0 };
+
+       log_very_verbose("Unlocking memory");
+
+       if (!_use_mlockall)
+               rewind(_mapsh);
 
-       if (_memlock_maps(cmd, LVM_MUNLOCK, &ums))
-               log_very_verbose("Unlocking memory");
+       if (!_memlock_maps(cmd, LVM_MUNLOCK, &unlock_mstats))
+               stack;
 
-       if (memcmp(&_ms, &ums, sizeof(ums)))
-               log_error(INTERNAL_ERROR "Maps size mismatch (%ld,%ld,%ld) != (%ld,%ld,%ld)",
-                         (long)_ms.r_size, (long)_ms.w_size, (long)_ms.x_size,
-                         (long)ums.r_size, (long)ums.w_size, (long)ums.x_size);
+       if (!_use_mlockall) {
+               if (fclose(_mapsh))
+                       log_sys_error("fclose", _procselfmaps);
+
+               if (memcmp(&_mstats, &unlock_mstats, sizeof(unlock_mstats)))
+                       log_error(INTERNAL_ERROR "Maps size mismatch (%ld,%ld,%ld) != (%ld,%ld,%ld)",
+                                 (long)_mstats.r_size, (long)_mstats.w_size, (long)_mstats.x_size,
+                                 (long)unlock_mstats.r_size, (long)unlock_mstats.w_size, (long)unlock_mstats.x_size);
+       }
 
        _release_memory();
        if (setpriority(PRIO_PROCESS, 0, _priority))