From f0f43bc093a19deb7342b2639a4447872372ada9 Mon Sep 17 00:00:00 2001 From: Alasdair Kergon Date: Tue, 9 Mar 2010 03:16:11 +0000 Subject: [PATCH] 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. --- lib/commands/toolcontext.c | 7 +++ lib/mm/memlock.c | 109 +++++++++++++++++++++---------------- 2 files changed, 70 insertions(+), 46 deletions(-) diff --git a/lib/commands/toolcontext.c b/lib/commands/toolcontext.c index 99db7d0dd..7aac3619f 100644 --- 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 5ab54ce93..ac9811a3c 100644 --- 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)) -- 2.43.5