Linux Device Mapper development
 help / color / mirror / Atom feed
* [PATCH 0/4] multipath-tools: unit test fixes
@ 2026-06-25 16:01 Martin Wilck
  2026-06-25 16:01 ` [PATCH 1/4] multipath-tools tests: add wrapper for fstat() and fstat64() Martin Wilck
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Martin Wilck @ 2026-06-25 16:01 UTC (permalink / raw)
  To: Christophe Varoqui, Benjamin Marzinski; +Cc: dm-devel, Martin Wilck

This series contains a few fixes for issues recently encountered
in the multipath-tools GitHub CI.

A couple of additional patches, related only to the CI itself and the
GitHub workflows, has been pushed to my "tip" branch [1].

[1] https://github.com/openSUSE/multipath-tools/commits/refs/heads/tip/

Martin Wilck (4):
  multipath-tools tests: add wrapper for fstat() and fstat64()
  libmpathutil: runner: avoid segmentation violation in pthread_cancel()
  libmpathutil: runner: pause in cleanup handler if rctx is NULL
  libmpathutil: runner: use pthread_cleanup_push()

 libmpathutil/runner.c | 43 ++++++++++++++++++++++++++++++++++---------
 tests/wrap64.h        | 26 ++++++++++++++++++++++++++
 2 files changed, 60 insertions(+), 9 deletions(-)

-- 
2.54.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/4] multipath-tools tests: add wrapper for fstat() and fstat64()
  2026-06-25 16:01 [PATCH 0/4] multipath-tools: unit test fixes Martin Wilck
@ 2026-06-25 16:01 ` Martin Wilck
  2026-06-25 16:01 ` [PATCH 2/4] libmpathutil: runner: avoid segmentation violation in pthread_cancel() Martin Wilck
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Martin Wilck @ 2026-06-25 16:01 UTC (permalink / raw)
  To: Christophe Varoqui, Benjamin Marzinski; +Cc: dm-devel, Martin Wilck

On some Debian distributions, fstat() and fstat64() are wrappers around
__fxstat() and __fxstat64(), respectively. These functions take an extra
argument. Create macros to make sure we substitute the correct functions
with wrappers.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 tests/wrap64.h | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/tests/wrap64.h b/tests/wrap64.h
index fe985a7..3ea4b0a 100644
--- a/tests/wrap64.h
+++ b/tests/wrap64.h
@@ -4,6 +4,7 @@
 #include <linux/types.h>
 /* The following include is required for LIBAIO_REDIRECT */
 #include <libaio.h>
+#include <sys/stat.h>
 #include "util.h"
 
 /*
@@ -87,4 +88,29 @@
  * (see https://gcc.gnu.org/onlinedocs/cpp/Stringizing.html).
  */
 #define wrap_will_return(x, y) will_return(x, y)
+
+/*
+ * On some glibc versions on older Debian distros (up to bullseye), fstat()
+ * is a wrapper around __fxstat(), which takes an additional argument.
+ * Also, MUSL libc redirects fstat to __fstat_time64 on some architectures.
+ */
+#if defined(__GLIBC__) && defined(_STAT_VER)
+#define WRAP_FSTAT_NAME WRAP_NAME(__fxstat)
+#define WRAP_FSTAT_FUNC(v, fd, buf) WRAP_FSTAT(v, fd, buf)
+#define REAL_FSTAT_FUNC(v, fd, buf) REAL_FSTAT(v, fd, buf)
+#else
+#if !defined(__GLIBC__) && defined(_REDIR_TIME64) && _REDIR_TIME64
+#define WRAP_FSTAT_NAME WRAP_NAME(__fstat_time64)
+#elif defined(__GLIBC__) && __GLIBC_PREREQ(2, 34) && __BITS_PER_LONG == 32 && \
+	defined(_TIME_BITS) && _TIME_BITS == 64
+#define WRAP_FSTAT_NAME WRAP_NAME(__fstat64_time)
+#else
+#define WRAP_FSTAT_NAME WRAP_NAME(fstat)
+#endif
+#define WRAP_FSTAT_FUNC(v, fd, buf) WRAP_FSTAT(fd, buf)
+#define REAL_FSTAT_FUNC(v, fd, buf) REAL_FSTAT(fd, buf)
+#endif
+#define WRAP_FSTAT CONCAT2(__wrap_, WRAP_FSTAT_NAME)
+#define REAL_FSTAT CONCAT2(__real_, WRAP_FSTAT_NAME)
+
 #endif
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/4] libmpathutil: runner: avoid segmentation violation in pthread_cancel()
  2026-06-25 16:01 [PATCH 0/4] multipath-tools: unit test fixes Martin Wilck
  2026-06-25 16:01 ` [PATCH 1/4] multipath-tools tests: add wrapper for fstat() and fstat64() Martin Wilck
@ 2026-06-25 16:01 ` Martin Wilck
  2026-06-25 16:01 ` [PATCH 3/4] libmpathutil: runner: pause in cleanup handler if rctx is NULL Martin Wilck
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Martin Wilck @ 2026-06-25 16:01 UTC (permalink / raw)
  To: Christophe Varoqui, Benjamin Marzinski; +Cc: dm-devel, Martin Wilck

SISEGV has been observed in the runner test in our Gitlab CI with
MUSL libc. It can happen that a thread terminates before pthread_cancel()
is called, causing the error. Avoid it by waiting until the thread has
actually been cancelled.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmpathutil/runner.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/libmpathutil/runner.c b/libmpathutil/runner.c
index 56abd03..459af13 100644
--- a/libmpathutil/runner.c
+++ b/libmpathutil/runner.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 // Copyright (c) 2026 SUSE LLC
 #include <assert.h>
+#include <sched.h>
 #include <time.h>
 #include <pthread.h>
 #include <urcu/uatomic.h>
@@ -47,9 +48,24 @@ static void cleanup_context(struct runner_context **prctx)
 		return;
 
 	st = uatomic_cmpxchg(&rctx->status, RUNNER_RUNNING, RUNNER_DONE);
+	/*
+	 * If it finds the thread in RUNNER_RUNNING state, cancel_runner() sets
+	 * the state to RUNNER_CANCELLED before actually cancelling it.
+	 * If the thread terminates between these two points in time,
+	 * pthread_cancel() may access a pthread_t for an already cleaned-up
+	 * thread. Therefore wait here until the thread has actually been
+	 * cancelled, after which cancel_runner() will set the state to
+	 * RUNNER_DEAD. Whether the thread will actually see this value is
+	 * implementation-dependent.
+	 */
+	if (st == RUNNER_CANCELLED) {
+		do
+			sched_yield();
+		while (uatomic_read(&rctx->status) == RUNNER_CANCELLED);
+	}
 	if (st != RUNNER_RUNNING) {
 		uatomic_cmpxchg(&rctx->status, st, RUNNER_DEAD);
-		condlog(st == RUNNER_CANCELLED ? 3 : 2,
+		condlog(st == RUNNER_DEAD || st == RUNNER_CANCELLED ? 3 : 2,
 			"%s: runner %p finished in state '%s'", __func__, rctx,
 			runner_state_name(st));
 	}
@@ -116,6 +132,8 @@ repeat:
 		break;
 	case RUNNER_RUNNING:
 		pthread_cancel(rctx->thr);
+		assert(uatomic_cmpxchg(&rctx->status, RUNNER_CANCELLED,
+				       RUNNER_DEAD) == RUNNER_CANCELLED);
 		st_new = RUNNER_CANCELLED;
 		/* fallthrough */
 	case RUNNER_CANCELLED:
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/4] libmpathutil: runner: pause in cleanup handler if rctx is NULL
  2026-06-25 16:01 [PATCH 0/4] multipath-tools: unit test fixes Martin Wilck
  2026-06-25 16:01 ` [PATCH 1/4] multipath-tools tests: add wrapper for fstat() and fstat64() Martin Wilck
  2026-06-25 16:01 ` [PATCH 2/4] libmpathutil: runner: avoid segmentation violation in pthread_cancel() Martin Wilck
@ 2026-06-25 16:01 ` Martin Wilck
  2026-06-25 16:02 ` [PATCH 4/4] libmpathutil: runner: use pthread_cleanup_push() Martin Wilck
  2026-06-25 20:09 ` [PATCH 0/4] multipath-tools: unit test fixes Benjamin Marzinski
  4 siblings, 0 replies; 6+ messages in thread
From: Martin Wilck @ 2026-06-25 16:01 UTC (permalink / raw)
  To: Christophe Varoqui, Benjamin Marzinski; +Cc: dm-devel, Martin Wilck

In this (more or less impossible) case, make sure that pthread_cancel()
can't access invalid memory because the thread has exited unexpectedly.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmpathutil/runner.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/libmpathutil/runner.c b/libmpathutil/runner.c
index 459af13..c7adc20 100644
--- a/libmpathutil/runner.c
+++ b/libmpathutil/runner.c
@@ -4,6 +4,7 @@
 #include <sched.h>
 #include <time.h>
 #include <pthread.h>
+#include <unistd.h>
 #include <urcu/uatomic.h>
 #include "util.h"
 #include "debug.h"
@@ -44,8 +45,10 @@ static void cleanup_context(struct runner_context **prctx)
 	struct runner_context *rctx = *prctx;
 	int st;
 
-	if (!rctx)
-		return;
+	if (!rctx) {
+		condlog(0, "ERROR: %s: rctx is NULL", __func__);
+		pause();
+	}
 
 	st = uatomic_cmpxchg(&rctx->status, RUNNER_RUNNING, RUNNER_DONE);
 	/*
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 4/4] libmpathutil: runner: use pthread_cleanup_push()
  2026-06-25 16:01 [PATCH 0/4] multipath-tools: unit test fixes Martin Wilck
                   ` (2 preceding siblings ...)
  2026-06-25 16:01 ` [PATCH 3/4] libmpathutil: runner: pause in cleanup handler if rctx is NULL Martin Wilck
@ 2026-06-25 16:02 ` Martin Wilck
  2026-06-25 20:09 ` [PATCH 0/4] multipath-tools: unit test fixes Benjamin Marzinski
  4 siblings, 0 replies; 6+ messages in thread
From: Martin Wilck @ 2026-06-25 16:02 UTC (permalink / raw)
  To: Christophe Varoqui, Benjamin Marzinski; +Cc: dm-devel, Martin Wilck

... instead of relying on -fexceptions and __attribute__((cleanup)).
The latter causes sporadic occurences of SIGSEGV on MUSL libc.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmpathutil/runner.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/libmpathutil/runner.c b/libmpathutil/runner.c
index c7adc20..694481e 100644
--- a/libmpathutil/runner.c
+++ b/libmpathutil/runner.c
@@ -40,10 +40,10 @@ struct runner_context {
 	char __attribute__((aligned(sizeof(void *)))) data[];
 };
 
-static void cleanup_context(struct runner_context **prctx)
+static void cleanup_context(void *arg)
 {
-	struct runner_context *rctx = *prctx;
 	int st;
+	struct runner_context *rctx = arg;
 
 	if (!rctx) {
 		condlog(0, "ERROR: %s: rctx is NULL", __func__);
@@ -82,7 +82,9 @@ static void *runner_thread(void *arg)
 	 * The cleanup function makes sure memory is freed if the thread is
 	 * cancelled (-fexceptions).
 	 */
-	struct runner_context *rctx __attribute__((cleanup(cleanup_context))) = arg;
+	struct runner_context *rctx = arg;
+
+	pthread_cleanup_push(cleanup_context, arg);
 
 #ifdef RUNNER_START_DELAY_US
 	/*
@@ -98,10 +100,12 @@ static void *runner_thread(void *arg)
 #endif
 
 	st = uatomic_cmpxchg(&rctx->status, RUNNER_IDLE, RUNNER_RUNNING);
-	if (st != RUNNER_IDLE)
-		return NULL;
 
-	(*rctx->func)(rctx->data);
+	/* Only run the function if we haven't been cancelled */
+	if (st == RUNNER_IDLE)
+		(*rctx->func)(rctx->data);
+
+	pthread_cleanup_pop(1);
 	return NULL;
 }
 
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/4] multipath-tools: unit test fixes
  2026-06-25 16:01 [PATCH 0/4] multipath-tools: unit test fixes Martin Wilck
                   ` (3 preceding siblings ...)
  2026-06-25 16:02 ` [PATCH 4/4] libmpathutil: runner: use pthread_cleanup_push() Martin Wilck
@ 2026-06-25 20:09 ` Benjamin Marzinski
  4 siblings, 0 replies; 6+ messages in thread
From: Benjamin Marzinski @ 2026-06-25 20:09 UTC (permalink / raw)
  To: Martin Wilck; +Cc: Christophe Varoqui, dm-devel, Martin Wilck

On Thu, Jun 25, 2026 at 06:01:56PM +0200, Martin Wilck wrote:
> This series contains a few fixes for issues recently encountered
> in the multipath-tools GitHub CI.

For the set:

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-06-25 20:09 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-25 16:01 [PATCH 0/4] multipath-tools: unit test fixes Martin Wilck
2026-06-25 16:01 ` [PATCH 1/4] multipath-tools tests: add wrapper for fstat() and fstat64() Martin Wilck
2026-06-25 16:01 ` [PATCH 2/4] libmpathutil: runner: avoid segmentation violation in pthread_cancel() Martin Wilck
2026-06-25 16:01 ` [PATCH 3/4] libmpathutil: runner: pause in cleanup handler if rctx is NULL Martin Wilck
2026-06-25 16:02 ` [PATCH 4/4] libmpathutil: runner: use pthread_cleanup_push() Martin Wilck
2026-06-25 20:09 ` [PATCH 0/4] multipath-tools: unit test fixes Benjamin Marzinski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox