All of lore.kernel.org
 help / color / mirror / Atom feed
From: Milan Broz <mbroz@redhat.com>
To: lvm-devel@redhat.com
Subject: [PATCH] Fix memory lock imbalance in locking code.
Date: Fri, 20 Nov 2009 20:35:36 +0100	[thread overview]
Message-ID: <4B06EF88.2050802@redhat.com> (raw)
In-Reply-To: <1258739361-17361-1-git-send-email-mbroz@redhat.com>

[another try after discussion]

Fix memory lock imbalance in locking code.

(This affects only cluster locking because only cluster
locking module set LCK_PRE_MEMLOCK.)

With currect code you get
# vgchange -a n
  Internal error: _memlock_count has dropped below 0.
when using cluster locking.

It is caused by _unlock_memory calls here

  if ((flags & (LCK_SCOPE_MASK | LCK_TYPE_MASK)) == LCK_LV_RESUME)
	memlock_dec();

Unfortunately it is also (wrongly) called in immediate unlock
(when LCK_HOLD is not set) from lock_vol
(LCK_UNLOCK is misinterpreted as LCK_LV_RESUME).

Avoid this by comparing original flags and provide memlock
code type of operation (suspend/resume).

Signed-off-by: Milan Broz <mbroz@redhat.com>
---
 lib/locking/locking.c |   37 ++++++++++++++++++++++++++++---------
 1 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/lib/locking/locking.c b/lib/locking/locking.c
index 9d86433..7bf3f78 100644
--- a/lib/locking/locking.c
+++ b/lib/locking/locking.c
@@ -42,6 +42,12 @@ static volatile sig_atomic_t _handler_installed;
 static struct sigaction _oldhandler;
 static int _oldmasked;
 
+typedef enum {
+        LV_NOOP,
+        LV_SUSPEND,
+        LV_RESUME
+} lv_operation_t;
+
 static void _catch_sigint(int unused __attribute__((unused)))
 {
 	_sigint_caught = 1;
@@ -159,21 +165,21 @@ static void _unblock_signals(void)
 	return;
 }
 
-static void _lock_memory(uint32_t flags)
+static void _lock_memory(lv_operation_t lv_op)
 {
 	if (!(_locking.flags & LCK_PRE_MEMLOCK))
 		return;
 
-	if ((flags & (LCK_SCOPE_MASK | LCK_TYPE_MASK)) == LCK_LV_SUSPEND)
+	if (lv_op == LV_SUSPEND)
 		memlock_inc();
 }
 
-static void _unlock_memory(uint32_t flags)
+static void _unlock_memory(lv_operation_t lv_op)
 {
 	if (!(_locking.flags & LCK_PRE_MEMLOCK))
 		return;
 
-	if ((flags & (LCK_SCOPE_MASK | LCK_TYPE_MASK)) == LCK_LV_RESUME)
+	if (lv_op == LV_RESUME)
 		memlock_dec();
 }
 
@@ -336,12 +342,13 @@ int check_lvm1_vg_inactive(struct cmd_context *cmd, const char *vgname)
  * VG locking is by VG name.
  * FIXME This should become VG uuid.
  */
-static int _lock_vol(struct cmd_context *cmd, const char *resource, uint32_t flags)
+static int _lock_vol(struct cmd_context *cmd, const char *resource,
+		     uint32_t flags, lv_operation_t lv_op)
 {
 	int ret = 0;
 
 	_block_signals(flags);
-	_lock_memory(flags);
+	_lock_memory(lv_op);
 
 	assert(resource);
 
@@ -368,7 +375,7 @@ static int _lock_vol(struct cmd_context *cmd, const char *resource, uint32_t fla
 		_update_vg_lock_count(resource, flags);
 	}
 
-	_unlock_memory(flags);
+	_unlock_memory(lv_op);
 	_unblock_signals();
 
 	return ret;
@@ -377,6 +384,18 @@ static int _lock_vol(struct cmd_context *cmd, const char *resource, uint32_t fla
 int lock_vol(struct cmd_context *cmd, const char *vol, uint32_t flags)
 {
 	char resource[258] __attribute((aligned(8)));
+	lv_operation_t lv_op;
+
+	switch (flags & (LCK_SCOPE_MASK | LCK_TYPE_MASK)) {
+		case LCK_LV_SUSPEND:
+				lv_op = LV_SUSPEND;
+				break;
+		case LCK_LV_RESUME:
+				lv_op = LV_RESUME;
+				break;
+		default:	lv_op = LV_NOOP;
+	}
+
 
 	if (flags == LCK_NONE) {
 		log_debug("Internal error: %s: LCK_NONE lock requested", vol);
@@ -416,7 +435,7 @@ int lock_vol(struct cmd_context *cmd, const char *vol, uint32_t flags)
 
 	strncpy(resource, vol, sizeof(resource));
 
-	if (!_lock_vol(cmd, resource, flags))
+	if (!_lock_vol(cmd, resource, flags, lv_op))
 		return 0;
 
 	/*
@@ -426,7 +445,7 @@ int lock_vol(struct cmd_context *cmd, const char *vol, uint32_t flags)
 	if (!(flags & LCK_CACHE) && !(flags & LCK_HOLD) &&
 	    ((flags & LCK_TYPE_MASK) != LCK_UNLOCK)) {
 		if (!_lock_vol(cmd, resource,
-			       (flags & ~LCK_TYPE_MASK) | LCK_UNLOCK))
+			       (flags & ~LCK_TYPE_MASK) | LCK_UNLOCK, lv_op))
 			return 0;
 	}
 




      reply	other threads:[~2009-11-20 19:35 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-11-20 17:49 [PATCH] Fix memory lock imbalance in locking code Milan Broz
2009-11-20 19:35 ` Milan Broz [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4B06EF88.2050802@redhat.com \
    --to=mbroz@redhat.com \
    --cc=lvm-devel@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.