All of lore.kernel.org
 help / color / mirror / Atom feed
From: Martin Wilck <mwilck@suse.com>
To: dm-devel@redhat.com
Cc: Martin Wilck <mwilck@suse.de>
Subject: [PATCH 15/33] libmultipath: move suspend logic to _dm_flush_map
Date: Tue, 28 Feb 2017 17:23:11 +0100	[thread overview]
Message-ID: <20170228162329.14517-16-mwilck@suse.com> (raw)
In-Reply-To: <20170228162329.14517-1-mwilck@suse.com>

From: Martin Wilck <mwilck@suse.de>

The function dm_suspend_and_flush() introduced in 9a4ff93
tries to remove child maps (partitions) after suspending
the mpath device. This may lock up if removing the partitions
requires I/O. It's better to use the following sequence
of actions: 1) clear queue_if_no_path; 2) remove partitions;
3) suspend; 4) remove (or resume and restore queue_if_no_path
in case of failure).

This patch modifies the implementation by moving the
queue_if_no_path/suspend logic into _dm_flush_map().
A call to _dm_flush_map() with need_suspend=1 replaces
the previous call to dm_suspend_and_flush().

With this change, the mpath device is only suspended after
removing partmaps, avoiding the deadlock.

Fixes: 9a4ff93 "Switch off 'queue_if_no_path' before removing maps"
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/devmapper.c | 102 ++++++++++++++++++++---------------------------
 libmultipath/devmapper.h |   9 +++--
 2 files changed, 49 insertions(+), 62 deletions(-)

diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index 1576dd01..044be2be 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -760,12 +760,6 @@ out:
 }
 
 static int
-has_partmap(const char *name, void *data)
-{
-	return 1;
-}
-
-static int
 partmap_in_use(const char *name, void *data)
 {
 	int part_count, *ret_count = (int *)data;
@@ -785,9 +779,13 @@ partmap_in_use(const char *name, void *data)
 	return 0;
 }
 
-int _dm_flush_map(const char *mapname, int need_sync, int deferred_remove)
+int _dm_flush_map (const char * mapname, int need_sync, int deferred_remove,
+		   int need_suspend, int retries)
 {
 	int r;
+	int queue_if_no_path = 0;
+	unsigned long long mapsize;
+	char params[PARAMS_SIZE] = {0};
 
 	if (!dm_is_mpath(mapname))
 		return 0; /* nothing to do */
@@ -797,6 +795,16 @@ int _dm_flush_map(const char *mapname, int need_sync, int deferred_remove)
 	if (!do_deferred(deferred_remove) && partmap_in_use(mapname, NULL))
 			return 1;
 
+	if (need_suspend &&
+	    !dm_get_map(mapname, &mapsize, params) &&
+	    strstr(params, "queue_if_no_path")) {
+		if (!dm_queue_if_no_path((char *)mapname, 0))
+			queue_if_no_path = 1;
+		else
+			/* Leave queue_if_no_path alone if unset failed */
+			queue_if_no_path = -1;
+	}
+
 	if (dm_remove_partmaps(mapname, need_sync, deferred_remove))
 		return 1;
 
@@ -805,17 +813,36 @@ int _dm_flush_map(const char *mapname, int need_sync, int deferred_remove)
 		return 1;
 	}
 
-	r = dm_device_remove(mapname, need_sync, deferred_remove);
+	do {
+		if (need_suspend && queue_if_no_path != -1)
+			dm_simplecmd_flush(DM_DEVICE_SUSPEND, mapname, 0);
 
-	if (r) {
-		if (do_deferred(deferred_remove) && dm_map_present(mapname)) {
-			condlog(4, "multipath map %s remove deferred",
+		r = dm_device_remove(mapname, need_sync, deferred_remove);
+
+		if (r) {
+			if (do_deferred(deferred_remove)
+			    && dm_map_present(mapname)) {
+				condlog(4, "multipath map %s remove deferred",
+					mapname);
+				return 2;
+			}
+			condlog(4, "multipath map %s removed", mapname);
+			return 0;
+		} else {
+			condlog(2, "failed to remove multipath map %s",
 				mapname);
-			return 2;
+			if (need_suspend && queue_if_no_path != -1) {
+				dm_simplecmd_noflush(DM_DEVICE_RESUME,
+						     mapname, 0);
+			}
 		}
-		condlog(4, "multipath map %s removed", mapname);
-		return 0;
-	}
+		if (retries)
+			sleep(1);
+	} while (retries-- > 0);
+
+	if (queue_if_no_path == 1)
+		dm_queue_if_no_path((char *)mapname, 1);
+
 	return 1;
 }
 
@@ -824,7 +851,7 @@ int _dm_flush_map(const char *mapname, int need_sync, int deferred_remove)
 int
 dm_flush_map_nopaths(const char * mapname, int deferred_remove)
 {
-	return _dm_flush_map(mapname, 1, deferred_remove);
+	return _dm_flush_map(mapname, 1, deferred_remove, 0, 0);
 }
 
 #else
@@ -832,52 +859,11 @@ dm_flush_map_nopaths(const char * mapname, int deferred_remove)
 int
 dm_flush_map_nopaths(const char * mapname, int deferred_remove)
 {
-	return _dm_flush_map(mapname, 1, 0);
+	return _dm_flush_map(mapname, 1, 0, 0, 0);
 }
 
 #endif
 
-int dm_suspend_and_flush_map (const char * mapname, int retries)
-{
-	int need_reset = 0, queue_if_no_path = 0;
-	unsigned long long mapsize;
-	char params[PARAMS_SIZE] = {0};
-	int udev_flags = 0;
-
-	if (!dm_is_mpath(mapname))
-		return 0; /* nothing to do */
-
-	/* if the device currently has no partitions, do not
-	   run kpartx on it if you fail to delete it */
-	if (do_foreach_partmaps(mapname, has_partmap, NULL) == 0)
-		udev_flags |= MPATH_UDEV_NO_KPARTX_FLAG;
-
-	if (!dm_get_map(mapname, &mapsize, params)) {
-		if (strstr(params, "queue_if_no_path"))
-			queue_if_no_path = 1;
-	}
-
-	if (queue_if_no_path && dm_queue_if_no_path((char *)mapname, 0) == 0)
-		need_reset = 1;
-
-	do {
-		if (!queue_if_no_path || need_reset)
-			dm_simplecmd_flush(DM_DEVICE_SUSPEND, mapname, 0);
-
-		if (!dm_flush_map(mapname)) {
-			condlog(4, "multipath map %s removed", mapname);
-			return 0;
-		}
-		dm_simplecmd_noflush(DM_DEVICE_RESUME, mapname, udev_flags);
-		if (retries)
-			sleep(1);
-	} while (retries-- > 0);
-	condlog(2, "failed to remove multipath map %s", mapname);
-	if (need_reset)
-		dm_queue_if_no_path((char *)mapname, 1);
-	return 1;
-}
-
 int dm_flush_maps (int retries)
 {
 	int r = 0;
diff --git a/libmultipath/devmapper.h b/libmultipath/devmapper.h
index 3ea43297..aca4454b 100644
--- a/libmultipath/devmapper.h
+++ b/libmultipath/devmapper.h
@@ -36,12 +36,13 @@ int dm_get_map(const char *, unsigned long long *, char *);
 int dm_get_status(char *, char *);
 int dm_type(const char *, char *);
 int dm_is_mpath(const char *);
-int _dm_flush_map (const char *, int, int);
+int _dm_flush_map (const char *, int, int, int, int);
 int dm_flush_map_nopaths(const char * mapname, int deferred_remove);
-#define dm_flush_map(mapname) _dm_flush_map(mapname, 1, 0)
-#define dm_flush_map_nosync(mapname) _dm_flush_map(mapname, 0, 0)
+#define dm_flush_map(mapname) _dm_flush_map(mapname, 1, 0, 0, 0)
+#define dm_flush_map_nosync(mapname) _dm_flush_map(mapname, 0, 0, 0, 0)
+#define dm_suspend_and_flush_map(mapname, retries) \
+	_dm_flush_map(mapname, 1, 0, 1, retries)
 int dm_cancel_deferred_remove(struct multipath *mpp);
-int dm_suspend_and_flush_map(const char * mapname, int retries);
 int dm_flush_maps (int retries);
 int dm_fail_path(char * mapname, char * path);
 int dm_reinstate_path(char * mapname, char * path);
-- 
2.11.0

  parent reply	other threads:[~2017-02-28 16:23 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-28 16:22 [PATCH 00/33] multipath-tools fixes from SUSE Martin Wilck
2017-02-28 16:22 ` [PATCH 01/33] multipathd.service: fixup Wants= and Before= statements Martin Wilck
2017-03-13 23:06   ` Benjamin Marzinski
2017-03-14  7:36     ` Martin Wilck
2017-02-28 16:22 ` [PATCH 02/33] multipathd: start daemon after udev trigger Martin Wilck
2017-02-28 16:22 ` [PATCH 03/33] Add support for "multipath=off" and "nompath" on kernel cmdline Martin Wilck
2017-02-28 16:23 ` [PATCH 04/33] multipath: do not check daemon from udev rules Martin Wilck
2017-04-05 21:54   ` Benjamin Marzinski
2017-04-06 12:10     ` Martin Wilck
2017-02-28 16:23 ` [PATCH 05/33] Invalid error code when using multipathd CLI Martin Wilck
2017-02-28 16:23 ` [PATCH 06/33] multipathd: set timeout for CLI commands correctly Martin Wilck
2017-04-05 22:07   ` Benjamin Marzinski
2017-04-12 20:26     ` Martin Wilck
2017-04-13 13:11     ` [PATCH] Revert "multipathd: set timeout for CLI commands correctly" Martin Wilck
2017-02-28 16:23 ` [PATCH 07/33] libmultipath: fall back to search paths by devt Martin Wilck
2017-02-28 16:23 ` [PATCH 08/33] libmultipath: Do not crash on empty features Martin Wilck
2017-02-28 16:23 ` [PATCH 09/33] multipathd: Set CLI timeout correctly Martin Wilck
2017-02-28 16:23 ` [PATCH 10/33] multipath: avoid crash when using modified configuration Martin Wilck
2017-02-28 16:23 ` [PATCH 11/33] multipathd: issue systemd READY after initial configuration Martin Wilck
2017-02-28 16:23 ` [PATCH 12/33] libmultipath/discovery: do not cache 'access_state' sysfs attribute Martin Wilck
2017-02-28 16:23 ` [PATCH 13/33] libmultipath: use existing alias from bindings file Martin Wilck
2017-02-28 16:23 ` [PATCH 14/33] multipath -ll: set DI_SERIAL Martin Wilck
2017-02-28 16:23 ` Martin Wilck [this message]
2017-04-05 22:44   ` [PATCH 15/33] libmultipath: move suspend logic to _dm_flush_map Benjamin Marzinski
2017-04-12 20:54     ` Martin Wilck
2017-04-13 13:05     ` [PATCH] libmultipath: fix skip_kpartx support for removing maps Martin Wilck
2017-04-14  8:42       ` Christophe Varoqui
2017-02-28 16:23 ` [PATCH 16/33] multipath: ignore -i if find_multipaths is set Martin Wilck
2017-02-28 16:23 ` [PATCH 17/33] multipathd: imply -n " Martin Wilck
2017-04-05 23:03   ` Benjamin Marzinski
2017-04-12 21:36     ` Martin Wilck
2017-04-13 21:54       ` Benjamin Marzinski
2017-02-28 16:23 ` [PATCH 18/33] multipathd: use weaker "force_reload" at startup Martin Wilck
2017-02-28 16:23 ` [PATCH 19/33] libmultipath: setup_features: log msg if queue_if_no_path is ignored Martin Wilck
2017-02-28 16:23 ` [PATCH 20/33] libmultipath: setup_feature: print log msg if no_path_retry cant be set Martin Wilck
2017-02-28 16:23 ` [PATCH 21/33] libmultipath: setup_feature: handle "retain_attached_hw_handler" Martin Wilck
2017-02-28 16:23 ` [PATCH 22/33] libmultipath: disassemble_map: skip no_path_retry check Martin Wilck
2017-02-28 16:23 ` [PATCH 23/33] libmultipath: disassemble_map: treat minio like assemble_map does Martin Wilck
2017-02-28 16:23 ` [PATCH 24/33] libmultipath: select_action: check special features separately Martin Wilck
2017-02-28 16:23 ` [PATCH 25/33] libmultipath: sysfs_attr_set_value: use const char* Martin Wilck
2017-02-28 16:23 ` [PATCH 26/33] libmultipath: reload map if not known to udev Martin Wilck
2017-02-28 16:23 ` [PATCH 27/33] libmultipath: differentiate ACT_NOTHING and ACT_IMPOSSIBLE Martin Wilck
2017-02-28 16:23 ` [PATCH 28/33] libmultipath: coalesce_paths: trigger uevent if nothing done Martin Wilck
2017-02-28 16:23 ` [PATCH 29/33] kpartx: sanitize delete partitions Martin Wilck
2017-02-28 16:23 ` [PATCH 30/33] tur: Add pthread_testcancel() Martin Wilck
2017-02-28 16:23 ` [PATCH 31/33] multipathd: fixup check for new path states Martin Wilck
2017-02-28 16:23 ` [PATCH 32/33] libmultipath/checkers: make RADOS checker optional Martin Wilck
2017-02-28 16:23 ` [PATCH 33/33] Make libdmmp build optional Martin Wilck
2017-02-28 22:44 ` [PATCH 00/33] multipath-tools fixes from SUSE Xose Vazquez Perez
2017-03-01  8:12   ` Martin Wilck
2017-03-23 18:43     ` multipath-tools (patch): Do not select sysfs prioritizer for RDAC arrays (was Re: [PATCH 00/33] multipath-tools fixes from SUSE) Xose Vazquez Perez
2017-03-23 20:40       ` Stewart, Sean
2017-03-22 19:02 ` [PATCH 00/33] multipath-tools fixes from SUSE Xose Vazquez Perez
2017-03-22 21:29   ` Christophe Varoqui
2017-03-23  8:30     ` Christophe Varoqui
2017-03-24  7:44       ` Martin Wilck
2017-04-12  7:38         ` Christophe Varoqui

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=20170228162329.14517-16-mwilck@suse.com \
    --to=mwilck@suse.com \
    --cc=dm-devel@redhat.com \
    --cc=mwilck@suse.de \
    /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.