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
next prev 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox