[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [lvm-devel] PATCH: Replace mlockall() with interal implementation



Here is updated version of the patch, that keeps the old mlockall()
functionality available with lvm.conf configure option.

Zdenek
>From 7ab80b42faca6d5b6ba061bc2fb105a7031f4d58 Mon Sep 17 00:00:00 2001
From: Zdenek Kabelac <zkabelac redhat com>
Date: Fri, 26 Feb 2010 15:13:19 +0100
Subject: [PATCH 04/36] Implement new memory locking function.

This patch adds a new implementation of locking function instead
of mlockall() that may lock way too much memory (>100MB).
New function instead uses mlock() system call and selectively locks
memory areas from /proc/self/maps. We try to avoid locking areas
where we are sure we will not use it during lock-ed state.

Patch also adds struct cmd_context to all memlock() calls to have
access to configuration.

For backward compatibility functionality of mlockall()
is preserved with "activation/use_mlockall" flag.

Signed-off-by: Zdenek Kabelac <zkabelac redhat com>
---
 doc/example.conf        |   12 ++++
 lib/activate/activate.c |   16 +++---
 lib/config/defaults.h   |    1 +
 lib/locking/locking.c   |   12 ++--
 lib/mm/memlock.c        |  161 +++++++++++++++++++++++++++++++++++++++--------
 lib/mm/memlock.h        |    8 +-
 tools/lvmcmdlib.c       |    4 +-
 7 files changed, 168 insertions(+), 46 deletions(-)

diff --git a/doc/example.conf b/doc/example.conf
index 4b92b1f..f589749 100644
--- a/doc/example.conf
+++ b/doc/example.conf
@@ -413,6 +413,18 @@ activation {
 
     mirror_log_fault_policy = "allocate"
     mirror_image_fault_policy = "remove"
+    
+    # Set to 1 to enable using mlockall() system call for lock memory. This may 
+    # lead to large memory consumption on some systems when some large mmap-ed 
+    # areas are loaded into memory and not really needed. LVM uses mlock() mechanism 
+    # and selectively locks only needed memory. In the case of an error in this code, 
+    # you may try to enable original mlockall() behavior.
+    # use_mlockall = 0
+    
+    # When we lock lvm process in memory we avoid locking of certain areas in memory.
+    # You may want to extend the list, to avoid more unneeded memory locks.
+    # Strings are looked in verbatim as substring in /proc/`pidof lvm`/maps lines.
+    # mlock_filter = [ "locale/locale-archive", "gconv/gconv-modules.cache" ]
 }
 
 
diff --git a/lib/activate/activate.c b/lib/activate/activate.c
index c0ffd53..eb94ac0 100644
--- a/lib/activate/activate.c
+++ b/lib/activate/activate.c
@@ -849,7 +849,7 @@ static int _lv_suspend(struct cmd_context *cmd, const char *lvid_s,
 		if (!error_if_not_suspended) {
 			r = 1;
 			if (info.suspended)
-				memlock_inc();
+				memlock_inc(cmd);
 		}
 		goto out;
 	}
@@ -868,13 +868,13 @@ static int _lv_suspend(struct cmd_context *cmd, const char *lvid_s,
 		/* FIXME Consider aborting here */
 		stack;
 
-	memlock_inc();
+	memlock_inc(cmd);
 
 	if (lv_is_origin(lv_pre) || lv_is_cow(lv_pre))
 		lockfs = 1;
 
 	if (!_lv_suspend_lv(lv, lockfs, flush_required)) {
-		memlock_dec();
+		memlock_dec(cmd);
 		fs_unlock();
 		goto out;
 	}
@@ -930,7 +930,7 @@ static int _lv_resume(struct cmd_context *cmd, const char *lvid_s,
 	if (!_lv_activate_lv(lv))
 		goto_out;
 
-	memlock_dec();
+	memlock_dec(cmd);
 	fs_unlock();
 
 	if (!monitor_dev_for_events(cmd, lv, 1))
@@ -1019,9 +1019,9 @@ int lv_deactivate(struct cmd_context *cmd, const char *lvid_s)
 	if (!monitor_dev_for_events(cmd, lv, 0))
 		stack;
 
-	memlock_inc();
+	memlock_inc(cmd);
 	r = _lv_deactivate(lv);
-	memlock_dec();
+	memlock_dec(cmd);
 	fs_unlock();
 
 	if (!lv_info(cmd, lv, &info, 1, 0) || info.exists)
@@ -1112,10 +1112,10 @@ static int _lv_activate(struct cmd_context *cmd, const char *lvid_s,
 	if (exclusive)
 		lv->status |= ACTIVATE_EXCL;
 
-	memlock_inc();
+	memlock_inc(cmd);
 	if (!(r = _lv_activate_lv(lv)))
 		stack;
-	memlock_dec();
+	memlock_dec(cmd);
 	fs_unlock();
 
 	if (r && !monitor_dev_for_events(cmd, lv, 1))
diff --git a/lib/config/defaults.h b/lib/config/defaults.h
index 94057e4..22a085d 100644
--- a/lib/config/defaults.h
+++ b/lib/config/defaults.h
@@ -43,6 +43,7 @@
 #define DEFAULT_FALLBACK_TO_CLUSTERED_LOCKING 1
 #define DEFAULT_WAIT_FOR_LOCKS 1
 #define DEFAULT_PRIORITISE_WRITE_LOCKS 1
+#define DEFAULT_USE_MLOCKALL 0
 
 #define DEFAULT_MIRRORLOG "disk"
 #define DEFAULT_MIRROR_LOG_FAULT_POLICY "allocate"
diff --git a/lib/locking/locking.c b/lib/locking/locking.c
index a636f7c..76dbda5 100644
--- a/lib/locking/locking.c
+++ b/lib/locking/locking.c
@@ -161,22 +161,22 @@ static void _unblock_signals(void)
 	_signals_blocked = 0;
 }
 
-static void _lock_memory(lv_operation_t lv_op)
+static void _lock_memory(struct cmd_context *cmd, lv_operation_t lv_op)
 {
 	if (!(_locking.flags & LCK_PRE_MEMLOCK))
 		return;
 
 	if (lv_op == LV_SUSPEND)
-		memlock_inc();
+		memlock_inc(cmd);
 }
 
-static void _unlock_memory(lv_operation_t lv_op)
+static void _unlock_memory(struct cmd_context *cmd, lv_operation_t lv_op)
 {
 	if (!(_locking.flags & LCK_PRE_MEMLOCK))
 		return;
 
 	if (lv_op == LV_RESUME)
-		memlock_dec();
+		memlock_dec(cmd);
 }
 
 void reset_locking(void)
@@ -363,7 +363,7 @@ static int _lock_vol(struct cmd_context *cmd, const char *resource,
 	int ret = 0;
 
 	_block_signals(flags);
-	_lock_memory(lv_op);
+	_lock_memory(cmd, lv_op);
 
 	assert(resource);
 
@@ -390,7 +390,7 @@ static int _lock_vol(struct cmd_context *cmd, const char *resource,
 		_update_vg_lock_count(resource, flags);
 	}
 
-	_unlock_memory(lv_op);
+	_unlock_memory(cmd, lv_op);
 	_unblock_signals();
 
 	return ret;
diff --git a/lib/mm/memlock.c b/lib/mm/memlock.c
index 3ceb5f4..72c9f8c 100644
--- a/lib/mm/memlock.c
+++ b/lib/mm/memlock.c
@@ -28,11 +28,11 @@
 
 #ifndef DEVMAPPER_SUPPORT
 
-void memlock_inc(void)
+void memlock_inc(struct cmd_context *cmd)
 {
 	return;
 }
-void memlock_dec(void)
+void memlock_dec(struct cmd_context *cmd)
 {
 	return;
 }
@@ -90,17 +90,129 @@ static void _release_memory(void)
 	free(_malloc_mem);
 }
 
-/* Stop memory getting swapped out */
-static void _lock_mem(void)
+typedef enum { LVM_MLOCK, LVM_MUNLOCK } lvmlock_t;
+/*
+ * mlock/munlock memory areas from /proc/self/maps
+ * format described in kernel/Documentation/filesystem/proc.txt
+ */
+static int _maps_line(struct cmd_context *cmd, lvmlock_t lock, const char* line)
 {
+	const struct config_node *cn;
+	struct config_value *cv;
+	long from, to;
+	int pos;
+	char fr, fw, fx, fp;
+
+	sscanf(line, "%lx-%lx %c%c%c%c%n",
+	       &from, &to, &fr, &fw, &fx, &fp, &pos);
+
+	/* skip  ---p,  select with r,w,x */
+	if (fr != 'r' && fw != 'w' && fx != 'x')
+		return 0;
+
+	if (strstr(line + pos, "[vsyscall]"))
+		return 0; /* always ignore this area */
+
+	/* blacklist specific files/areas */
+	if (!(cn = find_config_tree_node(cmd, "activation/mlock_filter"))) {
+		/* If no blacklist configured, use an internal set */
+		/* more candidates: /libreadline.so, /libdevmapper-event.so */
+		if (strstr(line + pos, "locale/locale-archive") ||
+		    strstr(line + pos, "gconv/gconv-modules.cache")) {
+			log_debug("mlock_filter-ed: %s", line);
+			return 0;
+		}
+	} 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");
+				continue;
+			}
+			if (strstr(line + pos, cv->v.str)) {
+				log_debug("mlock_filter-ed: %s", line);
+				return 0;
+			}
+		}
+	}
+
+	log_debug("%s %10ldKiB %12lx - %12lx %c%c%c%c %s",
+		  (lock == LVM_MLOCK) ? "mlock" : "munlock",
+		  (to - from + 1023) / 1024, from, to, fr, fw, fx, fp, line + pos);
+
+	if (lock == LVM_MLOCK) {
+		if (mlock((const void*)from, to - from) < 0) {
+			log_sys_error("mlock", line);
+			return 1;
+		}
+	} else {
+		if (munlock((const void*)from, to - from) < 0) {
+			log_sys_error("munlock", line);
+			return 1;
+		}
+	}
+
+	return 0;
+}
+
+static int _memlock_maps(struct cmd_context *cmd, lvmlock_t lock)
+{
+	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;
+	int ret = 0;
+
+	if (find_config_tree_bool(cmd, "activation/use_mlockall",
+				  DEFAULT_USE_MLOCKALL)) {
 #ifdef MCL_CURRENT
-	if (mlockall(MCL_CURRENT | MCL_FUTURE))
-		log_sys_error("mlockall", "");
-	else
-		log_very_verbose("Locking memory");
+		if (lock == LVM_MLOCK) {
+			if (mlockall(MCL_CURRENT | MCL_FUTURE)) {
+				log_sys_error("mlockall", "");
+				return 1;
+			}
+		} else {
+			if (munlockall()) {
+				log_sys_error("munlockall", "");
+				return 1;
+			}
+		}
+		return 0;
+#else
+		return 1;
 #endif
+	}
+
+	strcpy(procselfmaps, cmd->proc_dir);
+	strcat(procselfmaps, selfmaps);
+
+	if ((fh = fopen(procselfmaps, "r")) == NULL) {
+		log_sys_error("fopen", procselfmaps);
+		return 1;
+	}
+
+	while ((r = getline(&line, &len, fh)) != -1) {
+		line[r > 0 ? r - 1 : 0] = '\0'; /* remove \n */
+		if ((ret = _maps_line(cmd, lock, line) != 0))
+			break;
+	}
+
+	free(line);
+	fclose(fh);
+
+	return ret;
+}
+
+/* Stop memory getting swapped out */
+static void _lock_mem(struct cmd_context *cmd)
+{
 	_allocate_memory();
 
+	if (_memlock_maps(cmd, LVM_MLOCK) == 0)
+		log_very_verbose("Locking memory");
+
 	errno = 0;
 	if (((_priority = getpriority(PRIO_PROCESS, 0)) == -1) && errno)
 		log_sys_error("getpriority", "");
@@ -110,43 +222,40 @@ static void _lock_mem(void)
 				  _default_priority, strerror(errno));
 }
 
-static void _unlock_mem(void)
+static void _unlock_mem(struct cmd_context *cmd)
 {
-#ifdef MCL_CURRENT
-	if (munlockall())
-		log_sys_error("munlockall", "");
-	else
+	if (_memlock_maps(cmd, LVM_MUNLOCK) == 0)
 		log_very_verbose("Unlocking memory");
-#endif
+
 	_release_memory();
 	if (setpriority(PRIO_PROCESS, 0, _priority))
 		log_error("setpriority %u failed: %s", _priority,
 			  strerror(errno));
 }
 
-static void _lock_mem_if_needed(void) {
+static void _lock_mem_if_needed(struct cmd_context *cmd) {
 	if ((_memlock_count + _memlock_count_daemon) == 1)
-		_lock_mem();
+		_lock_mem(cmd);
 }
 
-static void _unlock_mem_if_possible(void) {
+static void _unlock_mem_if_possible(struct cmd_context *cmd) {
 	if ((_memlock_count + _memlock_count_daemon) == 0)
-		_unlock_mem();
+		_unlock_mem(cmd);
 }
 
-void memlock_inc(void)
+void memlock_inc(struct cmd_context *cmd)
 {
 	++_memlock_count;
-	_lock_mem_if_needed();
+	_lock_mem_if_needed(cmd);
 	log_debug("memlock_count inc to %d", _memlock_count);
 }
 
-void memlock_dec(void)
+void memlock_dec(struct cmd_context *cmd)
 {
 	if (!_memlock_count)
 		log_error(INTERNAL_ERROR "_memlock_count has dropped below 0.");
 	--_memlock_count;
-	_unlock_mem_if_possible();
+	_unlock_mem_if_possible(cmd);
 	log_debug("memlock_count dec to %d", _memlock_count);
 }
 
@@ -157,19 +266,19 @@ void memlock_dec(void)
  * of memlock or memlock_daemon is in effect.
  */
 
-void memlock_inc_daemon(void)
+void memlock_inc_daemon(struct cmd_context *cmd)
 {
 	++_memlock_count_daemon;
-	_lock_mem_if_needed();
+	_lock_mem_if_needed(cmd);
 	log_debug("memlock_count_daemon inc to %d", _memlock_count_daemon);
 }
 
-void memlock_dec_daemon(void)
+void memlock_dec_daemon(struct cmd_context *cmd)
 {
 	if (!_memlock_count_daemon)
 		log_error(INTERNAL_ERROR "_memlock_count_daemon has dropped below 0.");
 	--_memlock_count_daemon;
-	_unlock_mem_if_possible();
+	_unlock_mem_if_possible(cmd);
 	log_debug("memlock_count_daemon dec to %d", _memlock_count_daemon);
 }
 
diff --git a/lib/mm/memlock.h b/lib/mm/memlock.h
index 1f72eaa..fd19317 100644
--- a/lib/mm/memlock.h
+++ b/lib/mm/memlock.h
@@ -18,10 +18,10 @@
 
 struct cmd_context;
 
-void memlock_inc(void);
-void memlock_dec(void);
-void memlock_inc_daemon(void);
-void memlock_dec_daemon(void);
+void memlock_inc(struct cmd_context *cmd);
+void memlock_dec(struct cmd_context *cmd);
+void memlock_inc_daemon(struct cmd_context *cmd);
+void memlock_dec_daemon(struct cmd_context *cmd);
 int memlock(void);
 void memlock_init(struct cmd_context *cmd);
 
diff --git a/tools/lvmcmdlib.c b/tools/lvmcmdlib.c
index 580aad9..6b2bc9d 100644
--- a/tools/lvmcmdlib.c
+++ b/tools/lvmcmdlib.c
@@ -82,9 +82,9 @@ int lvm2_run(void *handle, const char *cmdline)
 	/* FIXME Temporary - move to libdevmapper */
 	ret = ECMD_PROCESSED;
 	if (!strcmp(cmdline, "_memlock_inc"))
-		memlock_inc_daemon();
+		memlock_inc_daemon(cmd);
 	else if (!strcmp(cmdline, "_memlock_dec"))
-		memlock_dec_daemon();
+		memlock_dec_daemon(cmd);
 	else
 		ret = lvm_run_command(cmd, argc, argv);
 
-- 
1.7.0


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]