[PATCH 0/2] libmultipath: fix leaked device-mapper suspend after a failed remove

[PATCH 0/2] libmultipath: fix leaked device-mapper suspend after a failed remove

[Suresh Thelkar]

Under heavy device-mapper ioctl load (e.g. LUN-number recycling combined
with ALUA transitions), dm_flush_map__() could leave a multipath map
suspended indefinitely after a failed DM_DEVICE_REMOVE. A suspended map
wedges any later task that touches it in uninterruptible D state, and
because sync() walks every block device, this can hang I/O node-wide
until reboot. The storage itself is healthy; the defect is entirely in
user space.

This series fixes two defects on the failed-remove path:

  1. Query errors were misclassified as external removal. Anything that
     was not DM_IS_MPATH_YES was treated as "removed externally" and
     returned DM_FLUSH_OK without resuming. But dm_is_mpath() also
     returns DM_IS_MPATH_ERR (-1) when the status query itself fails
     transiently under ioctl pressure, so a still-suspended map was
     abandoned. Only DM_IS_MPATH_NO now counts as external removal; a
     query error is logged and falls through to the resume path.

  2. The recovery resume was fire-and-forget. Even when recovery was
     attempted, a single DM_DEVICE_RESUME was issued and its result
     never checked. The resume is now retried once if the ioctl fails,
     so the map is not left suspended on a transient failure.

Only libmultipath/devmapper.c changes. There is no change to the success
path or to behavior outside the failed-remove branch.

Validated with a deterministic LD_PRELOAD fault injector that forces
DM_DEV_REMOVE -> EBUSY followed by DM_TABLE_STATUS -> error (so
dm_is_mpath() returns -1) on a single scsi_debug-backed test map:
patched builds leave the map Active; unpatched builds leave it
Suspended.

GitHub PR for reference: https://github.com/opensvc/multipath-tools/pull/167

Suresh Thelkar (2):
  libmultipath: dm_flush_map__: do not treat query error as external
    removal
  libmultipath: dm_flush_map__: retry the resume after a failed remove

 libmultipath/devmapper.c | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

-- 
2.43.0

[PATCH 1/2] libmultipath: dm_flush_map__: do not treat query error as external removal

[Suresh Thelkar]

On a failed DM_DEVICE_REMOVE, dm_flush_map__() classified anything that
was not DM_IS_MPATH_YES as "removed externally" and returned DM_FLUSH_OK
without resuming the map it had just suspended. dm_is_mpath() also
returns DM_IS_MPATH_ERR (-1) when the status query itself fails, which
happens transiently under a heavy ioctl load (e.g. LUN-number recycling
plus ALUA transitions). A transient error was thus misread as a
successful external removal, and the map was left suspended
indefinitely, wedging later I/O node-wide via sync().

Only DM_IS_MPATH_NO now counts as "removed externally". A query error is
logged and falls through to the resume path instead of being reported as
success.

Signed-off-by: Suresh Thelkar <suresh.thelkar@yahoo.com>
---
 libmultipath/devmapper.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index e3870f59..a5570a85 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -1042,13 +1042,26 @@ int dm_flush_map__ (const char *mapname, int flags, int retries)
 			}
 			condlog(4, "multipath map %s removed", mapname);
 			return DM_FLUSH_OK;
-		} else if (dm_is_mpath(mapname) != DM_IS_MPATH_YES) {
-			condlog(4, "multipath map %s removed externally",
-				mapname);
-			return DM_FLUSH_OK; /* raced. someone else removed it */
 		} else {
-			condlog(2, "failed to remove multipath map %s",
-				mapname);
+			int is_mpath = dm_is_mpath(mapname);
+
+			if (is_mpath == DM_IS_MPATH_NO) {
+				condlog(4, "multipath map %s removed externally",
+					mapname);
+				return DM_FLUSH_OK; /* raced. someone else removed it */
+			}
+			/*
+			 * A query error (DM_IS_MPATH_ERR) must not be treated
+			 * as "removed externally": under an ioctl storm the
+			 * status query can transiently fail, and returning
+			 * DM_FLUSH_OK here would leave the map suspended.
+			 */
+			if (is_mpath == DM_IS_MPATH_ERR)
+				condlog(1, "%s: unable to verify map state after failed remove",
+					mapname);
+			else
+				condlog(2, "failed to remove multipath map %s",
+					mapname);
 			if ((flags & DMFL_SUSPEND) && queue_if_no_path != -1) {
 				dm_simplecmd_noflush(DM_DEVICE_RESUME,
 						     mapname, udev_flags);
-- 
2.43.0

[PATCH 2/2] libmultipath: dm_flush_map__: retry the resume after a failed remove

[Suresh Thelkar]

After a failed DM_DEVICE_REMOVE the map was suspended earlier in
dm_flush_map__(), and the recovery path issued a single fire-and-forget
DM_DEVICE_RESUME whose result was ignored. If that resume ioctl failed
(possible under an ioctl storm) the map stayed suspended, which wedges
any later task in uninterruptible D state and, because sync() walks
every block device, hangs I/O node-wide until reboot.

Since the map is known to have been suspended at the start of the loop,
the DM_DEVICE_RESUME return value is authoritative. Simply retry the
resume once if the ioctl fails, so a transient failure does not leave
the map suspended. The remove is still retried by the enclosing loop
when retries are requested.

Signed-off-by: Suresh Thelkar <suresh.thelkar@yahoo.com>
---
 libmultipath/devmapper.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index a5570a85..bea7e804 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -1062,10 +1062,12 @@ int dm_flush_map__ (const char *mapname, int flags, int retries)
 			else
 				condlog(2, "failed to remove multipath map %s",
 					mapname);
-			if ((flags & DMFL_SUSPEND) && queue_if_no_path != -1) {
-				dm_simplecmd_noflush(DM_DEVICE_RESUME,
-						     mapname, udev_flags);
-			}
+			/* The map was suspended above; resume it so a failed
+			 * remove does not leave it suspended. Retry once if the
+			 * resume ioctl itself fails. */
+			if ((flags & DMFL_SUSPEND) && queue_if_no_path != -1 &&
+			    !dm_simplecmd_noflush(DM_DEVICE_RESUME, mapname, udev_flags))
+				dm_simplecmd_noflush(DM_DEVICE_RESUME, mapname, udev_flags);
 		}
 		if (retries)
 			sleep(1);
-- 
2.43.0

Re: [PATCH 1/2] libmultipath: dm_flush_map__: do not treat query error as external removal

[Benjamin Marzinski]

On Mon, Oct 05, 2026 at 04:57:25AM +0000, Suresh Thelkar wrote:
> On a failed DM_DEVICE_REMOVE, dm_flush_map__() classified anything that
> was not DM_IS_MPATH_YES as "removed externally" and returned DM_FLUSH_OK
> without resuming the map it had just suspended. dm_is_mpath() also
> returns DM_IS_MPATH_ERR (-1) when the status query itself fails, which
> happens transiently under a heavy ioctl load (e.g. LUN-number recycling
> plus ALUA transitions). A transient error was thus misread as a
> successful external removal, and the map was left suspended
> indefinitely, wedging later I/O node-wide via sync().
> 
> Only DM_IS_MPATH_NO now counts as "removed externally". A query error is
> logged and falls through to the resume path instead of being reported as
> success.
> 
> Signed-off-by: Suresh Thelkar <suresh.thelkar@yahoo.com>

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>

Re: [PATCH 2/2] libmultipath: dm_flush_map__: retry the resume after a failed remove

[Benjamin Marzinski]

On Mon, Oct 05, 2026 at 04:57:26AM +0000, Suresh Thelkar wrote:
> After a failed DM_DEVICE_REMOVE the map was suspended earlier in
> dm_flush_map__(), and the recovery path issued a single fire-and-forget
> DM_DEVICE_RESUME whose result was ignored. If that resume ioctl failed
> (possible under an ioctl storm) the map stayed suspended, which wedges
> any later task in uninterruptible D state and, because sync() walks
> every block device, hangs I/O node-wide until reboot.
> 
> Since the map is known to have been suspended at the start of the loop,
> the DM_DEVICE_RESUME return value is authoritative. Simply retry the
> resume once if the ioctl fails, so a transient failure does not leave
> the map suspended. The remove is still retried by the enclosing loop
> when retries are requested.
> 
> Signed-off-by: Suresh Thelkar <suresh.thelkar@yahoo.com>

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>

Re: [PATCH 2/2] libmultipath: dm_flush_map__: retry the resume after a failed remove

[Martin Wilck]

On Mon, 2026-10-05 at 04:57 +0000, Suresh Thelkar wrote:
> After a failed DM_DEVICE_REMOVE the map was suspended earlier in
> dm_flush_map__(), and the recovery path issued a single fire-and-
> forget
> DM_DEVICE_RESUME whose result was ignored. If that resume ioctl
> failed
> (possible under an ioctl storm) the map stayed suspended, which
> wedges
> any later task in uninterruptible D state and, because sync() walks
> every block device, hangs I/O node-wide until reboot.
> 
> Since the map is known to have been suspended at the start of the
> loop,
> the DM_DEVICE_RESUME return value is authoritative. Simply retry the
> resume once if the ioctl fails, so a transient failure does not leave
> the map suspended. The remove is still retried by the enclosing loop
> when retries are requested.
> 
> Signed-off-by: Suresh Thelkar <suresh.thelkar@yahoo.com>
> ---
>  libmultipath/devmapper.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
> index a5570a85..bea7e804 100644
> --- a/libmultipath/devmapper.c
> +++ b/libmultipath/devmapper.c
> @@ -1062,10 +1062,12 @@ int dm_flush_map__ (const char *mapname, int
> flags, int retries)
>  			else
>  				condlog(2, "failed to remove
> multipath map %s",
>  					mapname);
> -			if ((flags & DMFL_SUSPEND) &&
> queue_if_no_path != -1) {
> -
> 				dm_simplecmd_noflush(DM_DEVICE_RESUME,
> -						     mapname,
> udev_flags);
> -			}
> +			/* The map was suspended above; resume it so
> a failed
> +			 * remove does not leave it suspended. Retry
> once if the
> +			 * resume ioctl itself fails. */
> +			if ((flags & DMFL_SUSPEND) &&
> queue_if_no_path != -1 &&
> +			    !dm_simplecmd_noflush(DM_DEVICE_RESUME,
> mapname, udev_flags))
> +				dm_simplecmd_noflush(DM_DEVICE_RESUM
> E, mapname, udev_flags);

Have you tested this, and have you observed cases where it actually
helped to resume a device after the first DM_DEVICE_RESUME had failed?

Would it increase the chances to resume the device if we wait a few
fractions of a second before retrying? 

Regards
Martin

-- 
Dr. Martin Wilck <mwilck@suse.com>
SUSE Software Solutions Germany GmbH, Frankenstr. 146, 90461 Nürnberg,
Germany
Geschäftsführer: Stefan Gaiser, Jochen Jaser, Abhinav Puri (HRB
36809,AG Nürnberg)

Re: [PATCH 2/2] libmultipath: dm_flush_map__: retry the resume after a failed remove

[Suresh Thelkar]

On Tue, 2026-10-06 at 23:07 +0200, Martin Wilck wrote:
> Have you tested this, and have you observed cases where it actually
> helped to resume a device after the first DM_DEVICE_RESUME had failed?
>
> Would it increase the chances to resume the device if we wait a few
> fractions of a second before retrying?

Thanks Martin, you're right to push on this.

Our fault injector was written to reproduce the exact sequence we
observed in the field: DM_DEVICE_REMOVE returned -EBUSY, and the
following status query then failed. So the injector deliberately forces
just those two -- DM_DEV_REMOVE -> EBUSY and the next DM_TABLE_STATUS ->
error -- which is what drives dm_is_mpath() to return -1. It does not
force DM_DEVICE_RESUME to fail, so the retry branch added in 2/2 was
never actually exercised.

As a result I can't claim to have observed the second resume help. The
extra retry was purely defensive, in case the resume ioctl itself
failed, and I agree an immediate identical retry with no delay wouldn't
add much. On this no-table-swap resume the realistic failures (-EINVAL
"not suspended", -ENXIO "gone") already mean the map isn't left
suspended, so neither a retry nor a short delay buys much.

So I'll drop 2/2. Patch 1/2 is the real fix for the problem we hit in
our environment. If it's useful, I can add a condlog(1, ...) warning
when the resume ioctl fails, so a leaked suspend is at least visible in
the logs -- would that address your concern?

If you agree, should I resend patch 1/2 on its own as a v2 (with 2/2
dropped)? Please confirm.

Regards,
Suresh

Re: [PATCH 2/2] libmultipath: dm_flush_map__: retry the resume after a failed remove

[Martin Wilck]

On Wed, 2026-10-07 at 07:16 +0000, Suresh Thelkar wrote:
> On Tue, 2026-10-06 at 23:07 +0200, Martin Wilck wrote:
> > Have you tested this, and have you observed cases where it actually
> > helped to resume a device after the first DM_DEVICE_RESUME had
> > failed?
> > 
> > Would it increase the chances to resume the device if we wait a few
> > fractions of a second before retrying?
> 
> Thanks Martin, you're right to push on this.
> 
> Our fault injector was written to reproduce the exact sequence we
> observed in the field: DM_DEVICE_REMOVE returned -EBUSY, and the
> following status query then failed. So the injector deliberately
> forces
> just those two -- DM_DEV_REMOVE -> EBUSY and the next DM_TABLE_STATUS
> ->
> error -- which is what drives dm_is_mpath() to return -1. It does not
> force DM_DEVICE_RESUME to fail, so the retry branch added in 2/2 was
> never actually exercised.

Have you ever found out what the exact reason for the DM_TABLE_STATUS
failure was? What was the return code of the kernel / device-mapper?
Can we perhaps make dm_is_mpath() smarter?

> As a result I can't claim to have observed the second resume help.
> The
> extra retry was purely defensive, in case the resume ioctl itself
> failed, and I agree an immediate identical retry with no delay
> wouldn't
> add much. On this no-table-swap resume the realistic failures (-
> EINVAL
> "not suspended", -ENXIO "gone") already mean the map isn't left
> suspended, so neither a retry nor a short delay buys much.
> 09:17 multip
> So I'll drop 2/2. Patch 1/2 is the real fix for the problem we hit in
> our environment. If it's useful, I can add a condlog(1, ...) warning
> when the resume ioctl fails, so a leaked suspend is at least visible
> in
> the logs -- would that address your concern?

Yes, that makes sense. We should see this in the logs.

> If you agree, should I resend patch 1/2 on its own as a v2 (with 2/2
> dropped)? Please confirm.

Yes, please resend a final version. It's up to you whether you keep two
commits or squash them into one.

Martin

-- 
Dr. Martin Wilck <mwilck@suse.com>
SUSE Software Solutions Germany GmbH, Frankenstr. 146, 90461 Nürnberg,
Germany
Geschäftsführer: Stefan Gaiser, Jochen Jaser, Abhinav Puri (HRB
36809,AG Nürnberg)

Re: [PATCH 2/2] libmultipath: dm_flush_map__: retry the resume after a failed remove

[Suresh Thelkar]

On Wed, 2026-10-07 at 09:22 +0200, Martin Wilck wrote:
> Have you ever found out what the exact reason for the DM_TABLE_STATUS
> failure was? What was the return code of the kernel / device-mapper?
> Can we perhaps make dm_is_mpath() smarter?

Honestly, no -- we never captured the exact errno of the failing
DM_TABLE_STATUS ioctl in the field. The diagnosis came from the vmcore
and journal, which showed the leaked-suspend symptom together with the
LUN-recycling + ALUA storm, but not the return code of the status ioctl
itself. Our fault injector used EIO as a representative transient
failure rather than a measured one, so I can't give you the kernel's
actual value from the incident.

On making dm_is_mpath() smarter: on current master it already
distinguishes the cases via libmp_mapinfo__(). An ENXIO from the ioctl
(and a !dmi.exists result) maps to DMP_NOT_FOUND -> DM_IS_MPATH_NO
("genuinely gone"), and a target-type mismatch to DMP_NO_MATCH ->
DM_IS_MPATH_NO. Only a non-ENXIO ioctl failure becomes DMP_ERR ->
DM_IS_MPATH_ERR. So the "device is gone" case is already handled; what
reaches DM_IS_MPATH_ERR is a genuinely ambiguous failure where we can't
tell whether the map is still present and suspended. Without the real
errno I'd rather not reclassify any of those as "removed" -- that is
exactly the misclassification patch 1 removes. If we later capture a
specific, reliably-benign errno there, we can special-case it then.

I'll send v2 shortly (patch 1 unchanged + a condlog(1) on resume
failure).

Thanks,
Suresh

[PATCH v2 0/2] libmultipath: fix leaked device-mapper suspend after a failed remove

[Suresh Thelkar]

Under heavy device-mapper ioctl load (e.g. LUN-number recycling combined
with ALUA transitions), dm_flush_map__() could leave a multipath map
suspended indefinitely after a failed DM_DEVICE_REMOVE. A suspended map
wedges any later task that touches it in uninterruptible D state, and
because sync() walks every block device, this can hang I/O node-wide
until reboot. The storage itself is healthy; the defect is entirely in
user space.

Patch 1 is the actual fix. Query errors on the failed-remove path were
misclassified as external removal: anything that was not DM_IS_MPATH_YES
was treated as "removed externally" and returned DM_FLUSH_OK without
resuming. But dm_is_mpath() also returns DM_IS_MPATH_ERR (-1) when the
status query itself fails transiently under ioctl pressure, so a
still-suspended map was abandoned. Only DM_IS_MPATH_NO now counts as
external removal; a query error is logged and falls through to the
resume path.

Patch 2 checks the DM_DEVICE_RESUME return value on the failed-remove
path and logs at condlog(1) if it fails, so a leaked suspend is at least
visible in the logs instead of being silent.

Only libmultipath/devmapper.c changes. There is no change to the success
path or to behavior outside the failed-remove branch.

Changes since v1:
 - Dropped the v1 "retry the resume" patch. As discussed on the list,
   the fault injector never exercised a resume failure, and on this
   no-table-swap resume the realistic failures (-EINVAL "not suspended",
   -ENXIO "gone") already mean the map is not left suspended, so an
   immediate identical retry (with or without a short delay) buys
   little.
 - Replaced it with a patch that simply logs at condlog(1) if the resume
   ioctl fails, so a leaked suspend is visible in the logs.
 - Patch 1 is unchanged from v1 and carries Benjamin Marzinski's
   Reviewed-by.

v1: https://lore.kernel.org/dm-devel/20261005045726.71858-1-suresh.thelkar@yahoo.com/

Suresh Thelkar (2):
  libmultipath: dm_flush_map__: do not treat query error as external
    removal
  libmultipath: dm_flush_map__: log if the resume fails after a failed
    remove

 libmultipath/devmapper.c | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)


base-commit: d53932bdbee02f440518ea384e63d7c4d3f66685
-- 
2.43.0

[PATCH v2 1/2] libmultipath: dm_flush_map__: do not treat query error as external removal

[Suresh Thelkar]

On a failed DM_DEVICE_REMOVE, dm_flush_map__() classified anything that
was not DM_IS_MPATH_YES as "removed externally" and returned DM_FLUSH_OK
without resuming the map it had just suspended. dm_is_mpath() also
returns DM_IS_MPATH_ERR (-1) when the status query itself fails, which
happens transiently under a heavy ioctl load (e.g. LUN-number recycling
plus ALUA transitions). A transient error was thus misread as a
successful external removal, and the map was left suspended
indefinitely, wedging later I/O node-wide via sync().

Only DM_IS_MPATH_NO now counts as "removed externally". A query error is
logged and falls through to the resume path instead of being reported as
success.

Signed-off-by: Suresh Thelkar <suresh.thelkar@yahoo.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/devmapper.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index e3870f59..a5570a85 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -1042,13 +1042,26 @@ int dm_flush_map__ (const char *mapname, int flags, int retries)
 			}
 			condlog(4, "multipath map %s removed", mapname);
 			return DM_FLUSH_OK;
-		} else if (dm_is_mpath(mapname) != DM_IS_MPATH_YES) {
-			condlog(4, "multipath map %s removed externally",
-				mapname);
-			return DM_FLUSH_OK; /* raced. someone else removed it */
 		} else {
-			condlog(2, "failed to remove multipath map %s",
-				mapname);
+			int is_mpath = dm_is_mpath(mapname);
+
+			if (is_mpath == DM_IS_MPATH_NO) {
+				condlog(4, "multipath map %s removed externally",
+					mapname);
+				return DM_FLUSH_OK; /* raced. someone else removed it */
+			}
+			/*
+			 * A query error (DM_IS_MPATH_ERR) must not be treated
+			 * as "removed externally": under an ioctl storm the
+			 * status query can transiently fail, and returning
+			 * DM_FLUSH_OK here would leave the map suspended.
+			 */
+			if (is_mpath == DM_IS_MPATH_ERR)
+				condlog(1, "%s: unable to verify map state after failed remove",
+					mapname);
+			else
+				condlog(2, "failed to remove multipath map %s",
+					mapname);
 			if ((flags & DMFL_SUSPEND) && queue_if_no_path != -1) {
 				dm_simplecmd_noflush(DM_DEVICE_RESUME,
 						     mapname, udev_flags);
-- 
2.43.0

[PATCH v2 2/2] libmultipath: dm_flush_map__: log if the resume fails after a failed remove

[Suresh Thelkar]

After a failed DM_DEVICE_REMOVE the map was suspended earlier in
dm_flush_map__(), and the recovery path issued a single fire-and-forget
DM_DEVICE_RESUME whose result was ignored. If that resume ioctl failed,
the map could be left suspended with no trace in the logs, which wedges
any later task in uninterruptible D state and, because sync() walks
every block device, hangs I/O node-wide until reboot.

Check the DM_DEVICE_RESUME return value and log at condlog(1) when it
fails, so a leaked suspend is at least visible in the logs. The map is
known to have been suspended at the start of the loop, so the ioctl
return value is authoritative.

Signed-off-by: Suresh Thelkar <suresh.thelkar@yahoo.com>
---
 libmultipath/devmapper.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index a5570a85..85c552cf 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -1062,10 +1062,12 @@ int dm_flush_map__ (const char *mapname, int flags, int retries)
 			else
 				condlog(2, "failed to remove multipath map %s",
 					mapname);
-			if ((flags & DMFL_SUSPEND) && queue_if_no_path != -1) {
-				dm_simplecmd_noflush(DM_DEVICE_RESUME,
-						     mapname, udev_flags);
-			}
+			/* Resume the map suspended above; log if the resume
+			 * ioctl fails, so a leaked suspend is visible. */
+			if ((flags & DMFL_SUSPEND) && queue_if_no_path != -1 &&
+			    !dm_simplecmd_noflush(DM_DEVICE_RESUME, mapname, udev_flags))
+				condlog(1, "%s: failed to resume map after failed remove",
+					mapname);
 		}
 		if (retries)
 			sleep(1);
-- 
2.43.0

Re: [PATCH v2 2/2] libmultipath: dm_flush_map__: log if the resume fails after a failed remove

[Benjamin Marzinski]

On Wed, Oct 07, 2026 at 09:22:36AM +0000, Suresh Thelkar wrote:
> After a failed DM_DEVICE_REMOVE the map was suspended earlier in
> dm_flush_map__(), and the recovery path issued a single fire-and-forget
> DM_DEVICE_RESUME whose result was ignored. If that resume ioctl failed,
> the map could be left suspended with no trace in the logs, which wedges
> any later task in uninterruptible D state and, because sync() walks
> every block device, hangs I/O node-wide until reboot.
> 
> Check the DM_DEVICE_RESUME return value and log at condlog(1) when it
> fails, so a leaked suspend is at least visible in the logs. The map is
> known to have been suspended at the start of the loop, so the ioctl
> return value is authoritative.
> 
> Signed-off-by: Suresh Thelkar <suresh.thelkar@yahoo.com>

I want to discuss this change from v1 a bit. The issue that the second
resume is solving is the same one that lead us to add a second resume to
dm_addmap_reload(). If the device has an invalid inactive table, and you
do a resume, dm will attempt to swap the table, fail, and leave the
device suspended with the invalid table removed. Immediately issuing a
second resume will cause dm to actually resume with the old table. If we
don't manage to resume the device, it seems much less likely that
waiting a second and re-attempting the remove will succeed (I believe
the most common case is us racing with something that temporarily opens
the device, like udev. It may need the device to be unsuspended to
do a quick read and close it again). 

Now, I admit that this is WAY more likely to happen when we are actually
reloading the table, since we just added an inactive table, and dm might
not be able to swap it correctly for any number of reasons. To hit this
issue when removing the device there would need to be an inactive table
(which is unlikely) and dm would have to fail when trying to swap tables
(which is also unlikely). Also, we resume the table in other places in
the code, without caring about this corner case. For consistency's sake,
we should probably either always do it, or only do it when we've loaded
a new table.

Thoughts? Also, I'm fine with this change going in as-is and dealing
with the resume question as a separate issue.

-Ben 

> ---
>  libmultipath/devmapper.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
> index a5570a85..85c552cf 100644
> --- a/libmultipath/devmapper.c
> +++ b/libmultipath/devmapper.c
> @@ -1062,10 +1062,12 @@ int dm_flush_map__ (const char *mapname, int flags, int retries)
>  			else
>  				condlog(2, "failed to remove multipath map %s",
>  					mapname);
> -			if ((flags & DMFL_SUSPEND) && queue_if_no_path != -1) {
> -				dm_simplecmd_noflush(DM_DEVICE_RESUME,
> -						     mapname, udev_flags);
> -			}
> +			/* Resume the map suspended above; log if the resume
> +			 * ioctl fails, so a leaked suspend is visible. */
> +			if ((flags & DMFL_SUSPEND) && queue_if_no_path != -1 &&
> +			    !dm_simplecmd_noflush(DM_DEVICE_RESUME, mapname, udev_flags))
> +				condlog(1, "%s: failed to resume map after failed remove",
> +					mapname);
>  		}
>  		if (retries)
>  			sleep(1);
> -- 
> 2.43.0

Re: [PATCH v2 2/2] libmultipath: dm_flush_map__: log if the resume fails after a failed remove

[Martin Wilck]

On Wed, 2026-10-07 at 12:42 -0400, Benjamin Marzinski wrote:
> On Wed, Oct 07, 2026 at 09:22:36AM +0000, Suresh Thelkar wrote:
> > After a failed DM_DEVICE_REMOVE the map was suspended earlier in
> > dm_flush_map__(), and the recovery path issued a single fire-and-
> > forget
> > DM_DEVICE_RESUME whose result was ignored. If that resume ioctl
> > failed,
> > the map could be left suspended with no trace in the logs, which
> > wedges
> > any later task in uninterruptible D state and, because sync() walks
> > every block device, hangs I/O node-wide until reboot.
> > 
> > Check the DM_DEVICE_RESUME return value and log at condlog(1) when
> > it
> > fails, so a leaked suspend is at least visible in the logs. The map
> > is
> > known to have been suspended at the start of the loop, so the ioctl
> > return value is authoritative.
> > 
> > Signed-off-by: Suresh Thelkar <suresh.thelkar@yahoo.com>
> 
> I want to discuss this change from v1 a bit. The issue that the
> second
> resume is solving is the same one that lead us to add a second resume
> to
> dm_addmap_reload(). If the device has an invalid inactive table, and
> you
> do a resume, dm will attempt to swap the table, fail, and leave the
> device suspended with the invalid table removed. Immediately issuing
> a
> second resume will cause dm to actually resume with the old table. If
> we
> don't manage to resume the device, it seems much less likely that
> waiting a second and re-attempting the remove will succeed (I believe
> the most common case is us racing with something that temporarily
> opens
> the device, like udev. It may need the device to be unsuspended to
> do a quick read and close it again). 
> 
> Now, I admit that this is WAY more likely to happen when we are
> actually
> reloading the table, since we just added an inactive table, and dm
> might
> not be able to swap it correctly for any number of reasons. To hit
> this
> issue when removing the device there would need to be an inactive
> table
> (which is unlikely) and dm would have to fail when trying to swap
> tables
> (which is also unlikely). Also, we resume the table in other places
> in
> the code, without caring about this corner case. For consistency's
> sake,
> we should probably either always do it, or only do it when we've
> loaded
> a new table.
> 
> Thoughts? Also, I'm fine with this change going in as-is and dealing
> with the resume question as a separate issue.

Right, thanks for explaining it. I'd forgotten about this failure
scenario. With this in mind, it actually makes sense to retry the
resume. You're right, there should be no inactive table, but retrying
won't hurt. [*]

I'd appreciate a short comment explaining the problem scenario that
the retry attempts to avoid (i.e. a short version of Ben's explanation
here).

Regards
Martin

[*] I wonder what would happen if there was a _valid_ inactive table.
In that case, instead of removing the map, we'd reload it with a
different table, which seems wrong. But again, there should  be none.

-- 
Dr. Martin Wilck <mwilck@suse.com>
SUSE Software Solutions Germany GmbH, Frankenstr. 146, 90461 Nürnberg,
Germany
Geschäftsführer: Stefan Gaiser, Jochen Jaser, Abhinav Puri (HRB
36809,AG Nürnberg)

Re: [PATCH v2 2/2] libmultipath: dm_flush_map__: log if the resume fails after a failed remove

[Suresh Thelkar]

On Wed, 2026-10-07 at 21:21 +0200, Martin Wilck wrote:
> > [Ben wrote:] The issue that the second resume is solving is the same
> > one that lead us to add a second resume to dm_addmap_reload(). If the
> > device has an invalid inactive table, and you do a resume, dm will
> > attempt to swap the table, fail, and leave the device suspended with
> > the invalid table removed. Immediately issuing a second resume will
> > cause dm to actually resume with the old table.
>
> Right, thanks for explaining it. [...] it actually makes sense to
> retry the resume. [...] I'd appreciate a short comment explaining the
> problem scenario that the retry attempts to avoid (i.e. a short
> version of Ben's explanation here).

Thanks both, that explains it well. I'll restore the retry in v3 (it is
what v1 did) and add a short comment describing the scenario:

	/*
	 * Resume the map suspended above. If it has an inactive table,
	 * the resume makes dm try to swap to it; if that swap fails dm
	 * leaves the map suspended with the invalid table dropped, and a
	 * second resume reactivates it with the old table (as
	 * dm_addmap_reload() does). Retry the resume once, and log if it
	 * is still not resumed so a leaked suspend is visible.
	 */

On Ben's consistency point (always retry vs only after loading a new
table): the flush/remove path never loads an inactive table, so in
practice there should be none to swap here. But the retry is cheap and
harmless and mirrors dm_addmap_reload(), so v3 simply retries on this
path too. I kept the condlog(1) from v2 so a map still suspended after
the retry remains visible in the logs.

Re Martin's valid-inactive-table footnote: agreed -- if a valid inactive
table somehow existed, the second resume would swap to it rather than
remove, which would be wrong. But dm_flush_map__() loads no inactive
table, so that state should not arise on this path; it would be a
pre-existing anomaly outside this fix, so I've left it out of scope here.

Sending v3 now with the above.

Thanks,
Suresh

[PATCH v3 0/2] libmultipath: fix leaked device-mapper suspend after a failed remove

[Suresh Thelkar]

Under heavy device-mapper ioctl load (e.g. LUN-number recycling combined
with ALUA transitions), dm_flush_map__() could leave a multipath map
suspended indefinitely after a failed DM_DEVICE_REMOVE. A suspended map
wedges any later task that touches it in uninterruptible D state, and
because sync() walks every block device, this can hang I/O node-wide
until reboot. The storage itself is healthy; the defect is entirely in
user space.

Patch 1 is the primary fix. Query errors on the failed-remove path were
misclassified as external removal: anything that was not DM_IS_MPATH_YES
was treated as "removed externally" and returned DM_FLUSH_OK without
resuming. But dm_is_mpath() also returns DM_IS_MPATH_ERR (-1) when the
status query itself fails transiently under ioctl pressure, so a
still-suspended map was abandoned. Only DM_IS_MPATH_NO now counts as
external removal; a query error is logged and falls through to the
resume path.

Patch 2 makes the resume on the failed-remove path robust: it checks the
DM_DEVICE_RESUME result, retries once if it fails, and logs at condlog(1)
if the map is still not resumed. The retry handles the inactive-table
case (dm fails the table swap and leaves the map suspended with the
invalid table dropped; a second resume reactivates it with the old
table) - the same scenario dm_addmap_reload() already guards against.

Only libmultipath/devmapper.c changes. There is no change to the success
path or to behavior outside the failed-remove branch.

Changes since v2:
 - Patch 2: restored the single resume retry (dropped in v2) and added a
   comment explaining why, as requested on the list. Per Ben Marzinski's
   explanation, a resume can fail the table swap on a map with an
   inactive table and leave it suspended; a second resume recovers it
   (as dm_addmap_reload() does). Martin Wilck agreed the retry is
   worthwhile and asked for the explanatory comment. Kept the v2
   condlog(1) so a still-unresumed map stays visible.
 - Patch 1 unchanged; carries Benjamin Marzinski's Reviewed-by.

Changes since v1:
 - v2 had replaced the retry with a log-only change; v3 restores the
   retry together with the explanatory comment.

v1: https://lore.kernel.org/dm-devel/20261005045726.71858-1-suresh.thelkar@yahoo.com/
v2: https://lore.kernel.org/dm-devel/20261007092236.165908-1-suresh.thelkar@yahoo.com/

Suresh Thelkar (2):
  libmultipath: dm_flush_map__: do not treat query error as external
    removal
  libmultipath: dm_flush_map__: retry the resume after a failed remove

 libmultipath/devmapper.c | 41 +++++++++++++++++++++++++++++++---------
 1 file changed, 32 insertions(+), 9 deletions(-)


base-commit: d53932bdbee02f440518ea384e63d7c4d3f66685
-- 
2.43.0

[PATCH v3 1/2] libmultipath: dm_flush_map__: do not treat query error as external removal

[Suresh Thelkar]

On a failed DM_DEVICE_REMOVE, dm_flush_map__() classified anything that
was not DM_IS_MPATH_YES as "removed externally" and returned DM_FLUSH_OK
without resuming the map it had just suspended. dm_is_mpath() also
returns DM_IS_MPATH_ERR (-1) when the status query itself fails, which
happens transiently under a heavy ioctl load (e.g. LUN-number recycling
plus ALUA transitions). A transient error was thus misread as a
successful external removal, and the map was left suspended
indefinitely, wedging later I/O node-wide via sync().

Only DM_IS_MPATH_NO now counts as "removed externally". A query error is
logged and falls through to the resume path instead of being reported as
success.

Signed-off-by: Suresh Thelkar <suresh.thelkar@yahoo.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/devmapper.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index e3870f59..a5570a85 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -1042,13 +1042,26 @@ int dm_flush_map__ (const char *mapname, int flags, int retries)
 			}
 			condlog(4, "multipath map %s removed", mapname);
 			return DM_FLUSH_OK;
-		} else if (dm_is_mpath(mapname) != DM_IS_MPATH_YES) {
-			condlog(4, "multipath map %s removed externally",
-				mapname);
-			return DM_FLUSH_OK; /* raced. someone else removed it */
 		} else {
-			condlog(2, "failed to remove multipath map %s",
-				mapname);
+			int is_mpath = dm_is_mpath(mapname);
+
+			if (is_mpath == DM_IS_MPATH_NO) {
+				condlog(4, "multipath map %s removed externally",
+					mapname);
+				return DM_FLUSH_OK; /* raced. someone else removed it */
+			}
+			/*
+			 * A query error (DM_IS_MPATH_ERR) must not be treated
+			 * as "removed externally": under an ioctl storm the
+			 * status query can transiently fail, and returning
+			 * DM_FLUSH_OK here would leave the map suspended.
+			 */
+			if (is_mpath == DM_IS_MPATH_ERR)
+				condlog(1, "%s: unable to verify map state after failed remove",
+					mapname);
+			else
+				condlog(2, "failed to remove multipath map %s",
+					mapname);
 			if ((flags & DMFL_SUSPEND) && queue_if_no_path != -1) {
 				dm_simplecmd_noflush(DM_DEVICE_RESUME,
 						     mapname, udev_flags);
-- 
2.43.0

[PATCH v3 2/2] libmultipath: dm_flush_map__: retry the resume after a failed remove

[Suresh Thelkar]

After a failed DM_DEVICE_REMOVE the map was suspended earlier in
dm_flush_map__(), and the recovery path issued a single fire-and-forget
DM_DEVICE_RESUME whose result was ignored. If that resume failed, the
map could be left suspended with no trace in the logs, which wedges any
later task in uninterruptible D state and, because sync() walks every
block device, hangs I/O node-wide until reboot.

A resume can fail if the map has an inactive table: dm tries to swap to
it and, if the swap fails, leaves the map suspended with the invalid
table dropped. An immediate second resume then reactivates it with the
old table. This is the same scenario dm_addmap_reload() already guards
against. It is far less likely on the remove path (there should be no
inactive table here), but retrying is cheap and harmless.

Check the DM_DEVICE_RESUME return value, retry the resume once if it
fails, and log at condlog(1) if the map is still not resumed so a leaked
suspend is at least visible in the logs.

Signed-off-by: Suresh Thelkar <suresh.thelkar@yahoo.com>
---
 libmultipath/devmapper.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index a5570a85..e3f71bc7 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -1062,10 +1062,20 @@ int dm_flush_map__ (const char *mapname, int flags, int retries)
 			else
 				condlog(2, "failed to remove multipath map %s",
 					mapname);
-			if ((flags & DMFL_SUSPEND) && queue_if_no_path != -1) {
-				dm_simplecmd_noflush(DM_DEVICE_RESUME,
-						     mapname, udev_flags);
-			}
+			/*
+			 * Resume the map suspended above. If it has an inactive
+			 * table, the resume makes dm try to swap to it; if that
+			 * swap fails dm leaves the map suspended with the
+			 * invalid table dropped, and a second resume reactivates
+			 * it with the old table (as dm_addmap_reload() does).
+			 * Retry the resume once, and log if it is still not
+			 * resumed so a leaked suspend is visible.
+			 */
+			if ((flags & DMFL_SUSPEND) && queue_if_no_path != -1 &&
+			    !dm_simplecmd_noflush(DM_DEVICE_RESUME, mapname, udev_flags) &&
+			    !dm_simplecmd_noflush(DM_DEVICE_RESUME, mapname, udev_flags))
+				condlog(1, "%s: failed to resume map after failed remove",
+					mapname);
 		}
 		if (retries)
 			sleep(1);
-- 
2.43.0

Re: [PATCH v3 0/2] libmultipath: fix leaked device-mapper suspend after a failed remove

[mwilck]

On Thu, 08 Oct 2026 04:37:47 +0000, Suresh Thelkar <suresh.thelkar@yahoo.com> wrote:
> libmultipath: fix leaked device-mapper suspend after a failed remove
> 
> Under heavy device-mapper ioctl load (e.g. LUN-number recycling combined
> with ALUA transitions), dm_flush_map__() could leave a multipath map
> suspended indefinitely after a failed DM_DEVICE_REMOVE. A suspended map
> wedges any later task that touches it in uninterruptible D state, and
> because sync() walks every block device, this can hang I/O node-wide
> until reboot. The storage itself is healthy; the defect is entirely in
> user space.
> 
> [...]

Reviewed-by: Martin Wilck <mwilck@suse.com>

-- 
Martin Wilck <mwilck@suse.com>