All of lore.kernel.org
 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 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.