On Tue, Feb 28, 2017 at 05:23:11PM +0100, Martin Wilck wrote: > From: Martin Wilck <[email protected]> > > 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.
This patch drops support for disabling partitions, by removing - /* 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; > Fixes: 9a4ff93 "Switch off 'queue_if_no_path' before removing maps" > Signed-off-by: Martin Wilck <[email protected]> > --- > 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; This resume command needs to disable adding back partitions if there aren't any, otherwise it could create partitions on a device that wasn't supposed to have any. > + 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 > > -- > dm-devel mailing list > [email protected] > https://www.redhat.com/mailman/listinfo/dm-devel -- dm-devel mailing list [email protected] https://www.redhat.com/mailman/listinfo/dm-devel
