[PATCH 00/18] multipath-tools: asynchronous checker framework

[PATCH 00/18] multipath-tools: asynchronous checker framework

[Martin Wilck]

Based on my previous patch set [1], which introduced the "runner" thread
handling code as an abstract implementation of the asynchronous checker
logic in the TUR checker, this set introduces a generic "async_checker"
type and converts all existing checkers except directio to this framework.
With this set applied, all checkers run asyncrhonously unless explicitly
forbidden to do so with the "force_sync" option.

Patch 1-3 are minor preparations. Patch 4-8 introduce a general concept of a
refcounted "shared" pointer and use it to replace multiple instances
of hard-coded refcounts on objects. Patch 9 converts the current TUR code
into a generic asynchronous checker type by separating the actual TUR
code from the handling of the async runners. Patch 10 adds support for
this async checker type in the checkers code by searching for
"libcheck_async_func" with dlsym() in checker DSOs. Patch 11-15 convert
all checkers except emc_clariion to the async_checker framework.
emc_clariion is special because it needs extra context both for the checker
itself and for multipath-related information. Patch 16 and 17 add the
respective additional fields to the async_checker framework. The mpcontext
handling is rewritten, because the current apporach of handling void **
pointers would be disastrous in scenarious with truly asynchronous checker
threads that may hang forever. Patch 18 finally converts emc_clariion,
using the concepts from patch 16 and 17.

Comments and reviews welcome.

Regards,
Martin

[1] https://lore.kernel.org/dm-devel/20260428114942.292307-1-mwilck@suse.com/T/#m9369993a81c82b10ef42896505d4a354b6be3f79

Martin Wilck (18):
  libmpathutil: runner: reduce a message loglevel
  libmultipath: checkers: add two generic checker messages
  libmultipath: checkers: move checker_class definition to checkers.h
  libmpathutil: add implementation of generic shared pointer
  multipath-tools tests: add tests for shared pointer code
  libmultipath: use shared_ptr for checker classes
  libmultipath: use shared_ptr for prioritizers
  libmpathutil: runner: use shared_ptr
  libmultipath: add generic async path checker code
  libmultipath: checkers: add support for async checker
  libmultipath: convert TUR checker to an async checker instance
  libmultipath: convert RDAC checker to async checker
  libmultipath: convert cciss_tur checker to async checker
  libmultipath: convert readsector0 checker to async checker
  libmultipath: convert hp_sw checker to async checker
  libmultipath: async_checker: add context_size and init symbols
  libmultipath: checkers: rework mpcontext passing
  libmultipath: convert emc_clariion to async_checker

 libmpathutil/libmpathutil.version    |   7 +
 libmpathutil/runner.c                |  32 ++--
 libmpathutil/util.c                  |  45 ++++-
 libmpathutil/util.h                  |   5 +
 libmultipath/Makefile                |   2 +-
 libmultipath/async_checker.c         | 251 +++++++++++++++++++++++++++
 libmultipath/async_checker.h         |  36 ++++
 libmultipath/checkers.c              | 236 ++++++++++---------------
 libmultipath/checkers.h              |  54 ++++--
 libmultipath/checkers/cciss_tur.c    |  41 ++---
 libmultipath/checkers/directio.c     |   6 +-
 libmultipath/checkers/emc_clariion.c | 104 +++++------
 libmultipath/checkers/hp_sw.c        |  32 +---
 libmultipath/checkers/rdac.c         |  30 ++--
 libmultipath/checkers/readsector0.c  |  28 +--
 libmultipath/checkers/tur.c          | 251 ++-------------------------
 libmultipath/discovery.c             |   8 +-
 libmultipath/libmultipath.version    |   2 +-
 libmultipath/prio.c                  |  47 +++--
 libmultipath/prio.h                  |   1 -
 libmultipath/structs.c               |   3 +-
 libmultipath/structs.h               |   3 +-
 tests/Makefile                       |   4 +-
 tests/directio.c                     |   2 +-
 tests/shared_ptr.c                   | 111 ++++++++++++
 25 files changed, 716 insertions(+), 625 deletions(-)
 create mode 100644 libmultipath/async_checker.c
 create mode 100644 libmultipath/async_checker.h
 create mode 100644 tests/shared_ptr.c

-- 
2.54.0

[PATCH 01/18] libmpathutil: runner: reduce a message loglevel

[Martin Wilck]

Don't "log runner 0x7b724a07a480 cancelled in state 'done'" at
level 3. It's a fairly normal thing to happen.

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

diff --git a/libmpathutil/runner.c b/libmpathutil/runner.c
index da57ff0..8c6d6b9 100644
--- a/libmpathutil/runner.c
+++ b/libmpathutil/runner.c
@@ -134,7 +134,7 @@ repeat:
 		break;
 	case RUNNER_DONE:
 		st_new = RUNNER_DEAD;
-		/* fallthrough */
+		break;
 	case RUNNER_DEAD:
 		level = 3;
 		break;
-- 
2.54.0

[PATCH 02/18] libmultipath: checkers: add two generic checker messages

[Martin Wilck]

Add generic messages for "timed out" and "still running".
They will be needed by follow-up patches.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/checkers.c | 2 ++
 libmultipath/checkers.h | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/libmultipath/checkers.c b/libmultipath/checkers.c
index bb6ad1e..8a90ade 100644
--- a/libmultipath/checkers.c
+++ b/libmultipath/checkers.c
@@ -365,6 +365,8 @@ static const char *generic_msg[CHECKER_GENERIC_MSGTABLE_SIZE] = {
 	[CHECKER_MSGID_GHOST] = " reports path is ghost",
 	[CHECKER_MSGID_UNSUPPORTED] = " doesn't support this device",
 	[CHECKER_MSGID_DISCONNECTED] = " no access to this device",
+	[CHECKER_MSGID_TIMEOUT] = " timed out",
+	[CHECKER_MSGID_RUNNING] = " still running",
 };
 
 const char *checker_message(const struct checker *c)
diff --git a/libmultipath/checkers.h b/libmultipath/checkers.h
index a969e7d..2317cd1 100644
--- a/libmultipath/checkers.h
+++ b/libmultipath/checkers.h
@@ -124,6 +124,8 @@ enum {
 	CHECKER_MSGID_GHOST,
 	CHECKER_MSGID_UNSUPPORTED,
 	CHECKER_MSGID_DISCONNECTED,
+	CHECKER_MSGID_TIMEOUT,
+	CHECKER_MSGID_RUNNING,
 	CHECKER_GENERIC_MSGTABLE_SIZE,
 	CHECKER_FIRST_MSGID = 100,   /* lowest msgid for checkers */
 	CHECKER_MSGTABLE_SIZE = 100, /* max msg table size for checkers */
-- 
2.54.0

[PATCH 03/18] libmultipath: checkers: move checker_class definition to checkers.h

[Martin Wilck]

Make the contents of struct checker_class accessible to other code.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/checkers.c | 18 ------------------
 libmultipath/checkers.h | 20 +++++++++++++++++++-
 2 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/libmultipath/checkers.c b/libmultipath/checkers.c
index 8a90ade..8f18ca6 100644
--- a/libmultipath/checkers.c
+++ b/libmultipath/checkers.c
@@ -14,24 +14,6 @@
 
 static const char * const checker_dir = MULTIPATH_DIR;
 
-struct checker_class {
-	struct list_head node;
-	void *handle;
-	int refcount;
-	int sync;
-	char name[CHECKER_NAME_LEN];
-	int (*check)(struct checker *);
-	int (*init)(struct checker *);       /* to allocate the context */
-	int (*mp_init)(struct checker *);    /* to allocate the mpcontext */
-	void (*free)(struct checker *);      /* to free the context */
-	void (*reset)(void);		     /* to reset the global variables */
-	void *(*thread)(void *);	     /* async thread entry point */
-	int (*pending)(struct checker *);    /* to recheck pending paths */
-	bool (*need_wait)(struct checker *); /* checker needs waiting for */
-	const char **msgtable;
-	short msgtable_size;
-};
-
 static const char *checker_state_names[PATH_MAX_STATE] = {
 	[PATH_WILD] = "wild",
 	[PATH_UNCHECKED] = "unchecked",
diff --git a/libmultipath/checkers.h b/libmultipath/checkers.h
index 2317cd1..694dfa3 100644
--- a/libmultipath/checkers.h
+++ b/libmultipath/checkers.h
@@ -131,7 +131,25 @@ enum {
 	CHECKER_MSGTABLE_SIZE = 100, /* max msg table size for checkers */
 };
 
-struct checker_class;
+struct checker;
+struct checker_class {
+	struct list_head node;
+	void *handle;
+	int refcount;
+	int sync;
+	char name[CHECKER_NAME_LEN];
+	int (*check)(struct checker *);
+	int (*init)(struct checker *);	  /* to allocate the context */
+	int (*mp_init)(struct checker *); /* to allocate the mpcontext */
+	void (*free)(struct checker *);	  /* to free the context */
+	void (*reset)(void);		  /* to reset the global variables */
+	void *(*thread)(void *);	  /* async thread entry point */
+	int (*pending)(struct checker *); /* to recheck pending paths */
+	bool (*need_wait)(struct checker *); /* checker needs waiting for */
+	const char **msgtable;
+	short msgtable_size;
+};
+
 struct checker {
 	struct checker_class *cls;
 	int fd;
-- 
2.54.0

[PATCH 04/18] libmpathutil: add implementation of generic shared pointer

[Martin Wilck]

Add a set of simple functions to handle refcounted pointers.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmpathutil/libmpathutil.version |  7 +++++
 libmpathutil/util.c               | 45 ++++++++++++++++++++++++++++---
 libmpathutil/util.h               |  5 ++++
 3 files changed, 53 insertions(+), 4 deletions(-)

diff --git a/libmpathutil/libmpathutil.version b/libmpathutil/libmpathutil.version
index c69120d..51594ad 100644
--- a/libmpathutil/libmpathutil.version
+++ b/libmpathutil/libmpathutil.version
@@ -194,3 +194,10 @@ global:
 local:
 	*;
 };
+
+LIBMPATHUTIL_6.1 {
+global:
+	alloc_shared_ptr;
+	get_shared_ptr;
+	put_shared_ptr;
+} LIBMPATHUTIL_6.0;
diff --git a/libmpathutil/util.c b/libmpathutil/util.c
index 23a9797..3c623ec 100644
--- a/libmpathutil/util.c
+++ b/libmpathutil/util.c
@@ -12,15 +12,13 @@
 #include <dirent.h>
 #include <unistd.h>
 #include <errno.h>
+#include <urcu/uatomic.h>
 #include "mt-udev-wrap.h"
 
 #include "util.h"
 #include "debug.h"
-#include "checkers.h"
 #include "vector.h"
-#include "structs.h"
-#include "config.h"
-#include "log.h"
+#include "list.h"
 
 size_t
 strchop(char *str)
@@ -384,3 +382,42 @@ void cleanup_udev_device(struct udev_device **udd)
 	if (*udd)
 		udev_device_unref(*udd);
 }
+
+struct shared_ptr {
+	long refcnt;
+	void (*destructor)(void *);
+	char __attribute__((aligned(sizeof(void *)))) ptr[];
+};
+
+void *alloc_shared_ptr(size_t size, void (*destructor)(void *))
+{
+	struct shared_ptr *sp = malloc(sizeof(*sp) + size);
+
+	if (!sp)
+		return NULL;
+	uatomic_set(&sp->refcnt, 1);
+	sp->destructor = destructor;
+	return sp->ptr;
+}
+
+void get_shared_ptr(void *ptr)
+{
+	struct shared_ptr *sp = container_of(ptr, struct shared_ptr, ptr);
+
+	if (uatomic_add_return(&sp->refcnt, 1) < 0)
+		condlog(0, "%s: refcount overflow", __func__);
+}
+
+void put_shared_ptr(void *ptr)
+{
+	struct shared_ptr *sp;
+
+	if (!ptr)
+		return;
+	sp = container_of(ptr, struct shared_ptr, ptr);
+	if (uatomic_sub_return(&sp->refcnt, 1) == 0) {
+		if (sp->destructor)
+			sp->destructor(ptr);
+		free(sp);
+	}
+}
diff --git a/libmpathutil/util.h b/libmpathutil/util.h
index aed1bc1..ffbb182 100644
--- a/libmpathutil/util.h
+++ b/libmpathutil/util.h
@@ -160,4 +160,9 @@ void cleanup_charp(char **p);
 void cleanup_ucharp(unsigned char **p);
 void cleanup_udev_device(struct udev_device **udd);
 void cleanup_bitfield(union bitfield **p);
+
+void *alloc_shared_ptr(size_t size, void (*destructor)(void *));
+void get_shared_ptr(void *ptr);
+void put_shared_ptr(void *ptr);
+
 #endif /* UTIL_H_INCLUDED */
-- 
2.54.0

[PATCH 05/18] multipath-tools tests: add tests for shared pointer code

[Martin Wilck]

Add a few tests (mostly meaningful with valgrind or ASAN) to test the
shared pointer code.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 tests/Makefile     |   4 +-
 tests/shared_ptr.c | 111 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 114 insertions(+), 1 deletion(-)
 create mode 100644 tests/shared_ptr.c

diff --git a/tests/Makefile b/tests/Makefile
index f0e5dd3..fc2a7ed 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -9,7 +9,8 @@ CFLAGS += $(BIN_CFLAGS) -Wno-unused-parameter -Wno-unused-function $(W_MISSING_I
 LIBDEPS += -L. -L $(mpathutildir) -L$(mpathcmddir) -lmultipath -lmpathutil -lmpathcmd -lcmocka
 
 TESTS := uevent parser util dmevents hwtable blacklist unaligned vpd pgpolicy \
-	 alias directio valid devt mpathvalid strbuf sysfs features cli mapinfo runner
+	 alias directio valid devt mpathvalid strbuf sysfs features cli mapinfo runner \
+	 shared_ptr
 HELPERS := test-lib.o test-log.o
 
 .PRECIOUS: $(TESTS:%=%-test)
@@ -66,6 +67,7 @@ features-test_OBJDEPS := $(mpathutildir)/mt-libudev.o
 cli-test_OBJDEPS := $(daemondir)/cli.o
 mapinfo-test_LIBDEPS = -lpthread -ldevmapper
 runner-test_LIBDEPS = -lpthread
+shared_ptr-test_LIBDEPS = -lpthread
 
 %.o: %.c
 	@echo building $@ because of $?
diff --git a/tests/shared_ptr.c b/tests/shared_ptr.c
new file mode 100644
index 0000000..ca5aa1a
--- /dev/null
+++ b/tests/shared_ptr.c
@@ -0,0 +1,111 @@
+#include <inttypes.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <stdarg.h>
+#include <stddef.h>
+#include <setjmp.h>
+#include <pthread.h>
+#include "cmocka-compat.h"
+#include "util.h"
+
+#include "globals.c"
+
+struct s_8 {
+	uint64_t val;
+};
+
+struct s_4 {
+	uint32_t val;
+};
+
+struct s_2 {
+	uint16_t val;
+};
+
+// clang-format off
+#define make_destruct(n, size)	     \
+static void destruct_ ## n ## _ ## size(void *ptr) \
+{						  \
+	struct s_##size *s = ptr;		  \
+						  \
+	assert_ptr_not_equal(ptr, NULL);	  \
+	assert_int_equal(s->val, n);		  \
+}
+
+#define make_fn(size) \
+static void *thread_##size(void *arg)		\
+{						\
+	struct s_##size *s = arg;		\
+						\
+	get_shared_ptr(s);			\
+	s->val++;				\
+	put_shared_ptr(s);			\
+	return NULL;				\
+}
+
+make_fn(2)
+make_fn(4)
+make_fn(8)
+
+#define make_test(n, size)					\
+make_destruct(n, size)						\
+								\
+static void test_ ## n ## _ ## size(void **state)		\
+{								\
+	pthread_t threads[n];					\
+	struct s_##size *s;					\
+	int i, rc;						\
+								\
+	s = alloc_shared_ptr(size, destruct_ ## n## _ ## size);	\
+	assert_ptr_not_equal(s, NULL);				\
+	s->val = 0;						\
+	for (i = 0; i < n; i++) {				\
+		rc = pthread_create(&threads[i], NULL,		\
+				    thread_##size, s);		\
+		assert_int_equal(rc, 0);			\
+	}							\
+	for (i = 0; i < n; i++) {				\
+		rc = pthread_join(threads[i], NULL);		\
+		assert_int_equal(rc, 0);			\
+	}							\
+	assert_int_equal(s->val, n);				\
+	put_shared_ptr(s);					\
+}
+
+make_test(0, 2)
+make_test(0, 4)
+make_test(0, 8)
+make_test(100, 2)
+make_test(100, 4)
+make_test(100, 8)
+make_test(1000, 2)
+make_test(1000, 4)
+make_test(1000, 8)
+	// clang-format on
+
+	int test_shared_ptr(void)
+{
+	// clang-format off
+	const struct CMUnitTest tests[] = {
+		cmocka_unit_test(test_0_2),
+		cmocka_unit_test(test_0_4),
+		cmocka_unit_test(test_0_8),
+		cmocka_unit_test(test_100_2),
+		cmocka_unit_test(test_100_4),
+		cmocka_unit_test(test_100_8),
+		cmocka_unit_test(test_1000_2),
+		cmocka_unit_test(test_1000_4),
+		cmocka_unit_test(test_1000_8),
+	};
+	// clang-format on
+	return cmocka_run_group_tests(tests, NULL, NULL);
+}
+
+int main(void)
+{
+	int ret = 0;
+
+	init_test_verbosity(-1);
+	ret += test_shared_ptr();
+	return ret;
+}
-- 
2.54.0

[PATCH 06/18] libmultipath: use shared_ptr for checker classes

[Martin Wilck]

Utilize the share_ptr code for tracking the refcount of checker classes.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/checkers.c | 98 +++++++++--------------------------------
 libmultipath/checkers.h |  2 -
 2 files changed, 20 insertions(+), 80 deletions(-)

diff --git a/libmultipath/checkers.c b/libmultipath/checkers.c
index 8f18ca6..a3b9cc8 100644
--- a/libmultipath/checkers.c
+++ b/libmultipath/checkers.c
@@ -39,41 +39,11 @@ const char *checker_state_name(int i)
 	return checker_state_names[i];
 }
 
-static struct checker_class *alloc_checker_class(void)
+static void free_checker_class(void *ptr)
 {
-	struct checker_class *c;
-
-	c = calloc(1, sizeof(struct checker_class));
-	if (c) {
-		INIT_LIST_HEAD(&c->node);
-		uatomic_set(&c->refcount, 1);
-	}
-	return c;
-}
-
-/* Use uatomic_{sub,add}_return() to ensure proper memory barriers */
-static int checker_class_ref(struct checker_class *cls)
-{
-	return uatomic_add_return(&cls->refcount, 1);
-}
-
-static int checker_class_unref(struct checker_class *cls)
-{
-	return uatomic_sub_return(&cls->refcount, 1);
-}
-
-void free_checker_class(struct checker_class *c)
-{
-	int cnt;
-
+	struct checker_class *c = ptr;
 	if (!c)
 		return;
-	cnt = checker_class_unref(c);
-	if (cnt != 0) {
-		condlog(cnt < 0 ? 1 : 4, "%s checker refcount %d",
-			c->name, cnt);
-		return;
-	}
 	condlog(3, "unloading %s checker", c->name);
 	list_del(&c->node);
 	if (c->reset)
@@ -84,7 +54,18 @@ void free_checker_class(struct checker_class *c)
 				c->name, dlerror());
 		}
 	}
-	free(c);
+}
+
+static struct checker_class *alloc_checker_class(void)
+{
+	struct checker_class *c;
+
+	c = alloc_shared_ptr(sizeof(*c), free_checker_class);
+	if (c) {
+		memset(c, 0, sizeof(*c));
+		INIT_LIST_HEAD(&c->node);
+	}
+	return c;
 }
 
 void cleanup_checkers (void)
@@ -92,9 +73,8 @@ void cleanup_checkers (void)
 	struct checker_class *checker_loop;
 	struct checker_class *checker_temp;
 
-	list_for_each_entry_safe(checker_loop, checker_temp, &checkers, node) {
-		free_checker_class(checker_loop);
-	}
+	list_for_each_entry_safe(checker_loop, checker_temp, &checkers, node)
+		put_shared_ptr(checker_loop);
 }
 
 static struct checker_class *checker_class_lookup(const char *name)
@@ -163,8 +143,7 @@ static struct checker_class *add_checker_class(const char *name)
 		goto out;
 
 	c->mp_init = (int (*)(struct checker *)) dlsym(c->handle, "libcheck_mp_init");
-	c->reset = (void (*)(void)) dlsym(c->handle, "libcheck_reset");
-	c->thread = (void *(*)(void*)) dlsym(c->handle, "libcheck_thread");
+	c->reset = (void (*)(void))dlsym(c->handle, "libcheck_reset");
 	c->pending = (int (*)(struct checker *)) dlsym(c->handle, "libcheck_pending");
 	c->need_wait = (bool (*)(struct checker *)) dlsym(c->handle, "libcheck_need_wait");
 	/* These 5 functions can be NULL. call dlerror() to clear out any
@@ -199,7 +178,7 @@ done:
 	list_add(&c->node, &checkers);
 	return c;
 out:
-	free_checker_class(c);
+	put_shared_ptr(c);
 	return NULL;
 }
 
@@ -285,7 +264,7 @@ void checker_put (struct checker * dst)
 	if (src && src->free)
 		src->free(dst);
 	checker_clear(dst);
-	free_checker_class(src);
+	put_shared_ptr(src);
 }
 
 int checker_get_state(struct checker *c)
@@ -371,43 +350,6 @@ bad_id:
 	return generic_msg[CHECKER_MSGID_NONE];
 }
 
-static void checker_cleanup_thread(void *arg)
-{
-	struct checker_class *cls = arg;
-
-	free_checker_class(cls);
-	rcu_unregister_thread();
-}
-
-static void *checker_thread_entry(void *arg)
-{
-	struct checker_context *ctx = arg;
-	void *rv;
-
-	rcu_register_thread();
-	pthread_cleanup_push(checker_cleanup_thread, ctx->cls);
-	rv = ctx->cls->thread(ctx);
-	pthread_cleanup_pop(1);
-	return rv;
-}
-
-int start_checker_thread(pthread_t *thread, const pthread_attr_t *attr,
-			 struct checker_context *ctx)
-{
-	int rv;
-
-	assert(ctx && ctx->cls && ctx->cls->thread);
-	/* Take a ref here, lest the class be freed before the thread starts */
-	(void)checker_class_ref(ctx->cls);
-	rv = pthread_create(thread, attr, checker_thread_entry, ctx);
-	if (rv != 0) {
-		condlog(1, "failed to start checker thread for %s: %m",
-			ctx->cls->name);
-		checker_class_unref(ctx->cls);
-	}
-	return rv;
-}
-
 void checker_clear_message (struct checker *c)
 {
 	if (!c)
@@ -431,7 +373,7 @@ void checker_get(struct checker *dst, const char *name)
 	if (!src)
 		return;
 
-	(void)checker_class_ref(dst->cls);
+	get_shared_ptr(dst->cls);
 }
 
 int init_checkers(void)
diff --git a/libmultipath/checkers.h b/libmultipath/checkers.h
index 694dfa3..630e987 100644
--- a/libmultipath/checkers.h
+++ b/libmultipath/checkers.h
@@ -135,7 +135,6 @@ struct checker;
 struct checker_class {
 	struct list_head node;
 	void *handle;
-	int refcount;
 	int sync;
 	char name[CHECKER_NAME_LEN];
 	int (*check)(struct checker *);
@@ -143,7 +142,6 @@ struct checker_class {
 	int (*mp_init)(struct checker *); /* to allocate the mpcontext */
 	void (*free)(struct checker *);	  /* to free the context */
 	void (*reset)(void);		  /* to reset the global variables */
-	void *(*thread)(void *);	  /* async thread entry point */
 	int (*pending)(struct checker *); /* to recheck pending paths */
 	bool (*need_wait)(struct checker *); /* checker needs waiting for */
 	const char **msgtable;
-- 
2.54.0

[PATCH 07/18] libmultipath: use shared_ptr for prioritizers

[Martin Wilck]

Replace the hardcoded refcount handling in prio.c with shared_ptr.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/prio.c | 47 +++++++++++++++++++--------------------------
 libmultipath/prio.h |  1 -
 2 files changed, 20 insertions(+), 28 deletions(-)

diff --git a/libmultipath/prio.c b/libmultipath/prio.c
index 24f825b..bf388e9 100644
--- a/libmultipath/prio.c
+++ b/libmultipath/prio.c
@@ -53,28 +53,11 @@ int init_prio(void)
 	return 0;
 }
 
-static struct prio * alloc_prio (void)
-{
-	struct prio *p;
-
-	p = calloc(1, sizeof(struct prio));
-	if (p) {
-		INIT_LIST_HEAD(&p->node);
-		p->refcount = 1;
-	}
-	return p;
-}
-
-void free_prio (struct prio * p)
+void free_prio(void *ptr)
 {
+	struct prio *p = ptr;
 	if (!p)
 		return;
-	p->refcount--;
-	if (p->refcount) {
-		condlog(4, "%s prioritizer refcount %d",
-			p->name, p->refcount);
-		return;
-	}
 	condlog(3, "unloading %s prioritizer", p->name);
 	list_del(&p->node);
 	if (p->handle) {
@@ -83,7 +66,18 @@ void free_prio (struct prio * p)
 				p->name, dlerror());
 		}
 	}
-	free(p);
+}
+
+static struct prio *alloc_prio(void)
+{
+	struct prio *p;
+
+	p = alloc_shared_ptr(sizeof(*p), free_prio);
+	if (p) {
+		memset(p, 0, sizeof(*p));
+		INIT_LIST_HEAD(&p->node);
+	}
+	return p;
 }
 
 void cleanup_prio(void)
@@ -91,9 +85,8 @@ void cleanup_prio(void)
 	struct prio * prio_loop;
 	struct prio * prio_temp;
 
-	list_for_each_entry_safe(prio_loop, prio_temp, &prioritizers, node) {
-		free_prio(prio_loop);
-	}
+	list_for_each_entry_safe(prio_loop, prio_temp, &prioritizers, node)
+		put_shared_ptr(prio_loop);
 }
 
 static struct prio *prio_lookup(const char *name)
@@ -150,7 +143,7 @@ struct prio *add_prio (const char *name)
 	list_add(&p->node, &prioritizers);
 	return p;
 out:
-	free_prio(p);
+	put_shared_ptr(p);
 	return NULL;
 }
 
@@ -199,17 +192,17 @@ void prio_get(struct prio *dst, const char *name, const char *args)
 	dst->getprio = src->getprio;
 	dst->handle = NULL;
 
-	src->refcount++;
+	get_shared_ptr(src);
 }
 
 void prio_put (struct prio * dst)
 {
-	struct prio * src;
+	struct prio *src;
 
 	if (!dst || !dst->getprio)
 		return;
 
 	src = prio_lookup(dst->name);
 	memset(dst, 0x0, sizeof(struct prio));
-	free_prio(src);
+	put_shared_ptr(src);
 }
diff --git a/libmultipath/prio.h b/libmultipath/prio.h
index 119b75f..7ff1a24 100644
--- a/libmultipath/prio.h
+++ b/libmultipath/prio.h
@@ -45,7 +45,6 @@ struct path;
 
 struct prio {
 	void *handle;
-	int refcount;
 	struct list_head node;
 	char name[PRIO_NAME_LEN];
 	char args[PRIO_ARGS_LEN];
-- 
2.54.0

[PATCH 08/18] libmpathutil: runner: use shared_ptr

[Martin Wilck]

Use shared_ptr to track the runner_context refcount.

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

diff --git a/libmpathutil/runner.c b/libmpathutil/runner.c
index 8c6d6b9..56abd03 100644
--- a/libmpathutil/runner.c
+++ b/libmpathutil/runner.c
@@ -30,7 +30,6 @@ const char *runner_state_name(int state)
 }
 
 struct runner_context {
-	int refcount;
 	int status;
 	struct timespec deadline;
 	pthread_t thr;
@@ -39,17 +38,6 @@ struct runner_context {
 	char __attribute__((aligned(sizeof(void *)))) data[];
 };
 
-static void release_context(struct runner_context *rctx)
-{
-	int n;
-
-	n = uatomic_sub_return(&rctx->refcount, 1);
-	assert(n >= 0);
-
-	if (n == 0)
-		free(rctx);
-}
-
 static void cleanup_context(struct runner_context **prctx)
 {
 	struct runner_context *rctx = *prctx;
@@ -65,7 +53,7 @@ static void cleanup_context(struct runner_context **prctx)
 			"%s: runner %p finished in state '%s'", __func__, rctx,
 			runner_state_name(st));
 	}
-	release_context(rctx);
+	put_shared_ptr(rctx);
 }
 
 static void *runner_thread(void *arg)
@@ -147,7 +135,7 @@ repeat:
 void release_runner(struct runner_context *rctx)
 {
 	cancel_runner(rctx);
-	release_context(rctx);
+	put_shared_ptr(rctx);
 }
 
 int check_runner(struct runner_context *rctx, void *data, unsigned int size)
@@ -195,17 +183,17 @@ struct runner_context *get_runner(runner_func func, void *data,
 		return NULL;
 	}
 
-	rctx = malloc(sizeof(*rctx) + size);
+	rctx = alloc_shared_ptr(sizeof(*rctx) + size, NULL);
 	if (!rctx)
 		return NULL;
 
 	rctx->func = func;
 	/*
-	 * We have to set the refcount to 2 here. The runner thread may be
-	 * cancelled before it even had the chance to increase the refcount,
-	 * which could result in a use-after-free in cleanup_context().
+	 * Take an additional reference here. The runner thread may be
+	 * cancelled before it even had the chance to take a reference, which
+	 * could result in a use-after-free in cleanup_context().
 	 */
-	uatomic_set(&rctx->refcount, 2);
+	get_shared_ptr(rctx);
 	uatomic_set(&rctx->status, RUNNER_IDLE);
 	memcpy(rctx->data, data, size);
 
@@ -222,8 +210,8 @@ struct runner_context *get_runner(runner_func func, void *data,
 
 	if (rc) {
 		condlog(1, "%s: pthread_create(): %s", __func__, strerror(rc));
-		uatomic_dec(&rctx->refcount);
-		release_context(rctx);
+		put_shared_ptr(rctx);
+		put_shared_ptr(rctx);
 		return NULL;
 	}
 	return rctx;
-- 
2.54.0

[PATCH 09/18] libmultipath: add generic async path checker code

[Martin Wilck]

Add a framework for an asynchronous path checker utilizing the
recently added runner code for thread handling.

This code has been derived from the TUR checker code by removing
all references to the actual sending of TUR ioctls.

Follow-up patches will convert the current TUR checker into an
instance of this async checker class.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/Makefile        |   2 +-
 libmultipath/async_checker.c | 216 +++++++++++++++++++++++++++++++++++
 libmultipath/async_checker.h |  35 ++++++
 libmultipath/checkers.c      |   1 +
 libmultipath/checkers.h      |   2 +
 5 files changed, 255 insertions(+), 1 deletion(-)
 create mode 100644 libmultipath/async_checker.c
 create mode 100644 libmultipath/async_checker.h

diff --git a/libmultipath/Makefile b/libmultipath/Makefile
index 85767ab..d71a835 100644
--- a/libmultipath/Makefile
+++ b/libmultipath/Makefile
@@ -22,7 +22,7 @@ OBJS-O := devmapper.o hwtable.o blacklist.o dmparser.o \
 	configure.o structs_vec.o sysfs.o \
 	lock.o file.o wwids.o prioritizers/alua_rtpg.o prkey.o \
 	io_err_stat.o dm-generic.o generic.o nvme-lib.o \
-	libsg.o valid.o
+	libsg.o valid.o async_checker.o
 
 OBJS := $(OBJS-O) $(OBJS-U)
 
diff --git a/libmultipath/async_checker.c b/libmultipath/async_checker.c
new file mode 100644
index 0000000..06159dd
--- /dev/null
+++ b/libmultipath/async_checker.c
@@ -0,0 +1,216 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+// Copyright (c) 2026 SUSE LLC
+
+#include <sys/types.h>
+#include <sys/sysmacros.h>
+#include <sys/stat.h>
+#include <stdlib.h>
+#include <stdbool.h>
+#include "async_checker.h"
+#include "checkers.h"
+#include "debug.h"
+#include "runner.h"
+
+#define MAX_NR_TIMEOUTS 1
+
+struct async_checker_context {
+	struct checker_context chkr;
+	int last_runner_state;
+	unsigned int nr_timeouts;
+	struct runner_context *rtx;
+	struct runner_data rdata;
+};
+
+#define rdata_size(acc) (sizeof(acc->rdata))
+
+int async_check_init(struct checker *c)
+{
+	struct async_checker_context *acc;
+	struct stat sb;
+
+	acc = calloc(1, sizeof(*acc));
+	if (!acc)
+		return -1;
+	acc->rdata.state = PATH_UNCHECKED;
+	acc->rdata.fd = -1;
+	if (fstat(c->fd, &sb) == 0)
+		acc->rdata.devt = sb.st_rdev;
+	acc->chkr.cls = c->cls;
+	c->context = acc;
+	return 0;
+}
+
+void async_check_free(struct checker *c)
+{
+	struct async_checker_context *acc = c->context;
+
+	if (!acc)
+		return;
+	c->context = NULL;
+	if (acc->rtx)
+		release_runner(acc->rtx);
+	free(acc);
+}
+
+static void runner_callback(void *arg)
+{
+	struct runner_data *rdata = arg;
+	int state;
+
+	condlog(4, "%d:%d : async checker starting up", major(rdata->devt),
+		minor(rdata->devt));
+
+	async_deep_sleep(rdata);
+	state = rdata->afunc(rdata);
+	rdata->state = state;
+	pthread_testcancel();
+	condlog(4, "%d:%d : async checker finished, state %s", major(rdata->devt),
+		minor(rdata->devt), checker_state_name(state));
+}
+
+static int check_runner_state(struct async_checker_context *acc)
+{
+	struct runner_context *rtx = acc->rtx;
+	int rc;
+
+	rc = check_runner(rtx, &acc->rdata, rdata_size(acc));
+	switch (rc) {
+	case RUNNER_DEAD:
+		acc->rdata.state = PATH_TIMEOUT;
+		acc->rdata.msgid = CHECKER_MSGID_TIMEOUT;
+		/* fallthrough */
+	case RUNNER_DONE:
+		release_runner(acc->rtx);
+		acc->rtx = NULL;
+		acc->last_runner_state = rc;
+		acc->nr_timeouts = 0;
+		condlog(rc == RUNNER_DONE ? 4 : 3,
+			"%d:%d : async checker finished, state %s, runner state %s",
+			major(acc->rdata.devt), minor(acc->rdata.devt),
+			checker_state_name(acc->rdata.state),
+			runner_state_name(rc));
+		break;
+	case RUNNER_CANCELLED:
+		acc->last_runner_state = rc;
+		acc->rdata.state = PATH_TIMEOUT;
+		acc->rdata.msgid = CHECKER_MSGID_TIMEOUT;
+		if (acc->nr_timeouts < MAX_NR_TIMEOUTS) {
+			condlog(3, "%d:%d : async checker timed out, releasing it",
+				major(acc->rdata.devt), minor(acc->rdata.devt));
+			acc->nr_timeouts++;
+			release_runner(acc->rtx);
+			acc->rtx = NULL;
+		} else if (acc->nr_timeouts == MAX_NR_TIMEOUTS) {
+			acc->nr_timeouts++;
+			condlog(3, "%d:%d : async checker timed out, waiting for it",
+				major(acc->rdata.devt), minor(acc->rdata.devt));
+		}
+		break;
+	default:
+		condlog(4, "%d:%d : async checker still running",
+			major(acc->rdata.devt), minor(acc->rdata.devt));
+		acc->rdata.msgid = CHECKER_MSGID_RUNNING;
+		break;
+	}
+	return rc;
+}
+
+bool async_check_need_wait(struct checker *c)
+{
+	struct async_checker_context *acc = c->context;
+
+	return acc && acc->rtx;
+}
+
+int async_check_pending(struct checker *c)
+{
+	struct async_checker_context *acc = c->context;
+	/* The if path checker isn't running, just return the exiting value. */
+	if (!acc || !acc->rtx)
+		return c->path_state;
+
+	/* This may nullify ct->rtx */
+	check_runner_state(acc);
+	c->msgid = acc->rdata.msgid;
+	return acc->rdata.state;
+}
+
+int async_check_check(struct checker *c)
+{
+	struct async_checker_context *acc = c->context;
+
+	if (!acc)
+		return PATH_UNCHECKED;
+
+	if (checker_is_sync(c))
+		return acc->rdata.afunc(&acc->rdata);
+
+	/* Handle the case that the checker just completed */
+	if (acc->rtx) {
+		check_runner_state(acc);
+		c->msgid = acc->rdata.msgid;
+		return acc->rdata.state;
+	}
+
+	/* create new checker thread */
+	acc->rdata.fd = c->fd;
+	acc->rdata.timeout = c->timeout;
+
+	acc->rdata.state = PATH_PENDING;
+	acc->rdata.msgid = CHECKER_MSGID_RUNNING;
+	acc->rdata.afunc = c->cls->async_func;
+	condlog(4, "%d:%d : starting checker", major(acc->rdata.devt),
+		minor(acc->rdata.devt));
+	acc->rtx = get_runner(runner_callback, &acc->rdata, rdata_size(acc),
+			      1000000 * c->timeout);
+
+	if (acc->rtx) {
+		c->msgid = acc->rdata.msgid;
+		return acc->rdata.state;
+	} else {
+		condlog(3, "%d:%d : failed to start async thread, using sync mode",
+			major(acc->rdata.devt), minor(acc->rdata.devt));
+		return acc->rdata.afunc(&acc->rdata);
+	}
+}
+
+/*
+ * Test code for "zombie tur thread" handling.
+ * Compile e.g. with CFLAGS=-DASYNC_TEST_MAJOR=8
+ * Additional parameters can be configure with the macros below.
+ *
+ * Everty nth started thread will hang in non-cancellable state
+ * for given number of seconds, for device given by major/minor.
+ */
+#ifdef ASYNC_TEST_MAJOR
+#ifndef ASYNC_TEST_MINOR
+#define ASYNC_TEST_MINOR 0
+#endif
+#ifndef ASYNC_SLEEP_INTERVAL
+#define ASYNC_SLEEP_INTERVAL 3
+#endif
+#ifndef ASYNC_SLEEP_SECS
+#define ASYNC_SLEEP_SECS 60
+#endif
+
+static void async_deep_sleep(const struct runner_data *rdata)
+{
+	static int sleep_cnt;
+	const struct timespec ts = {.tv_sec = ASYNC_SLEEP_SECS, .tv_nsec = 0};
+	int oldstate;
+
+	if (rdata->devt != makedev(ASYNC_TEST_MAJOR, ASYNC_TEST_MINOR) ||
+	    ++sleep_cnt % ASYNC_SLEEP_INTERVAL == 0)
+		return;
+
+	condlog(3, "async thread going to sleep for %ld seconds", ts.tv_sec);
+	if (pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &oldstate) != 0)
+		condlog(0, "pthread_setcancelstate: %m");
+	if (nanosleep(&ts, NULL) != 0)
+		condlog(0, "nanosleep: %m");
+	condlog(3, "async zombie thread woke up");
+	if (pthread_setcancelstate(oldstate, NULL) != 0)
+		condlog(0, "pthread_setcancelstate (2): %m");
+	pthread_testcancel();
+}
+#endif /* ASYNC_TEST_MAJOR */
diff --git a/libmultipath/async_checker.h b/libmultipath/async_checker.h
new file mode 100644
index 0000000..835de2a
--- /dev/null
+++ b/libmultipath/async_checker.h
@@ -0,0 +1,35 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+// Copyright (c) 2026 SUSE LLC
+#ifndef ASYNC_CHECKER_H_INCLUDED
+#define ASYNC_CHECKER_H_INCLUDED
+
+struct runner_data;
+struct checker;
+typedef int (*async_checker_func)(struct runner_data *);
+
+struct runner_data {
+	int fd;
+	dev_t devt;
+	async_checker_func afunc;
+	unsigned int timeout;
+	int state;
+	short msgid;
+	char checker_ctx[];
+};
+
+int async_check_init(struct checker *c);
+void async_check_free(struct checker *c);
+bool async_check_need_wait(struct checker *c);
+int async_check_pending(struct checker *c);
+int async_check_check(struct checker *c);
+
+#define CHECKER_MAX_CONTEXT_SIZE 1024
+
+/* For testing handling of async checker timeouts */
+#ifdef ASYNC_TEST_MAJOR
+static void async_deep_sleep(const struct runner_data *rdata);
+#else
+#define async_deep_sleep(x) do {} while (0)
+#endif
+
+#endif
diff --git a/libmultipath/checkers.c b/libmultipath/checkers.c
index a3b9cc8..3a1663f 100644
--- a/libmultipath/checkers.c
+++ b/libmultipath/checkers.c
@@ -9,6 +9,7 @@
 
 #include "debug.h"
 #include "checkers.h"
+#include "async_checker.h"
 #include "vector.h"
 #include "util.h"
 
diff --git a/libmultipath/checkers.h b/libmultipath/checkers.h
index 630e987..744be54 100644
--- a/libmultipath/checkers.h
+++ b/libmultipath/checkers.h
@@ -132,6 +132,7 @@ enum {
 };
 
 struct checker;
+struct runner_data;
 struct checker_class {
 	struct list_head node;
 	void *handle;
@@ -144,6 +145,7 @@ struct checker_class {
 	void (*reset)(void);		  /* to reset the global variables */
 	int (*pending)(struct checker *); /* to recheck pending paths */
 	bool (*need_wait)(struct checker *); /* checker needs waiting for */
+	int (*async_func)(struct runner_data *); /* callback for async_checker */
 	const char **msgtable;
 	short msgtable_size;
 };
-- 
2.54.0

[PATCH 10/18] libmultipath: checkers: add support for async checker

[Martin Wilck]

If a checker shared library contains a symbol "libcheck_async_func",
use this as a callback for the async checker functions, and do not
read any other symbols from the .so file except the message table.

Checkers can provide a symbol async_context_size that will cause
the async_checker code to reserve extra space in runner_data for
the checker to use, and a symbol async_init() for initializing
this private context.

Only emc_clariion will use the latter functionality.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/checkers.c           | 51 +++++++++++++++++++++----------
 libmultipath/libmultipath.version |  2 +-
 2 files changed, 36 insertions(+), 17 deletions(-)

diff --git a/libmultipath/checkers.c b/libmultipath/checkers.c
index 3a1663f..2f20b25 100644
--- a/libmultipath/checkers.c
+++ b/libmultipath/checkers.c
@@ -101,6 +101,18 @@ void reset_checker_classes(void)
 	}
 }
 
+static struct checker_class *add_async_checker_class(struct checker_class *c)
+{
+	c->init = async_check_init;
+	c->check = async_check_check;
+	c->need_wait = async_check_need_wait;
+	c->pending = async_check_pending;
+	c->free = async_check_free;
+
+	list_add(&c->node, &checkers);
+	return c;
+}
+
 static struct checker_class *add_checker_class(const char *name)
 {
 	char libname[LIB_CHECKER_NAMELEN];
@@ -129,6 +141,29 @@ static struct checker_class *add_checker_class(const char *name)
 				errstr);
 		goto out;
 	}
+
+	c->msgtable_size = 0;
+	c->msgtable = dlsym(c->handle, "libcheck_msgtable");
+
+	if (c->msgtable != NULL) {
+		const char **p;
+
+		for (p = c->msgtable;
+		     *p && (p - c->msgtable) < CHECKER_MSGTABLE_SIZE; p++)
+			/* nothing */;
+
+		c->msgtable_size = p - c->msgtable;
+	} else
+		c->msgtable_size = 0;
+	condlog(3, "checker %s: message table size = %d", c->name,
+		c->msgtable_size);
+
+	c->async_func = (int (*)(struct runner_data *))
+		dlsym(c->handle, "libcheck_async_func");
+	errstr = dlerror();
+	if (c->async_func)
+		return add_async_checker_class(c);
+
 	c->check = (int (*)(struct checker *)) dlsym(c->handle, "libcheck_check");
 	errstr = dlerror();
 	if (errstr != NULL)
@@ -158,22 +193,6 @@ static struct checker_class *add_checker_class(const char *name)
 	if (!c->free)
 		goto out;
 
-	c->msgtable_size = 0;
-	c->msgtable = dlsym(c->handle, "libcheck_msgtable");
-
-	if (c->msgtable != NULL) {
-		const char **p;
-
-		for (p = c->msgtable;
-		     *p && (p - c->msgtable) < CHECKER_MSGTABLE_SIZE; p++)
-			/* nothing */;
-
-		c->msgtable_size = p - c->msgtable;
-	} else
-		c->msgtable_size = 0;
-	condlog(3, "checker %s: message table size = %d",
-		c->name, c->msgtable_size);
-
 done:
 	c->sync = 1;
 	list_add(&c->node, &checkers);
diff --git a/libmultipath/libmultipath.version b/libmultipath/libmultipath.version
index 78fa2d4..5e8dbda 100644
--- a/libmultipath/libmultipath.version
+++ b/libmultipath/libmultipath.version
@@ -43,7 +43,7 @@ LIBMPATHCOMMON_1.0.0 {
 	put_multipath_config;
 };
 
-LIBMULTIPATH_32.0.0 {
+LIBMULTIPATH_33.0.0 {
 global:
 	/* symbols referenced by multipath and multipathd */
 	add_foreign;
-- 
2.54.0

[PATCH 11/18] libmultipath: convert TUR checker to an async checker instance

[Martin Wilck]

The TUR checker now just exports the message table and the
"libcheck_async_func" symbol. This converts it into an instance of
the generic async checker model.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/checkers/tur.c | 251 +++---------------------------------
 1 file changed, 15 insertions(+), 236 deletions(-)

diff --git a/libmultipath/checkers/tur.c b/libmultipath/checkers/tur.c
index 6791172..757767a 100644
--- a/libmultipath/checkers/tur.c
+++ b/libmultipath/checkers/tur.c
@@ -3,87 +3,31 @@
  *
  * Copyright (c) 2004 Christophe Varoqui
  */
-#include <stdlib.h>
-#include <string.h>
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <unistd.h>
-#include <fcntl.h>
 #include <sys/ioctl.h>
-#include <sys/sysmacros.h>
+#include <sys/types.h>
+
 #include <errno.h>
-#include <sys/time.h>
 
 #include "checkers.h"
-#include "debug.h"
+#include "async_checker.h"
 #include "sg_include.h"
-#include "runner.h"
 
 #define TUR_CMD_LEN 6
 #define HEAVY_CHECK_COUNT       10
-#define MAX_NR_TIMEOUTS 1
 
 enum {
-	MSG_TUR_RUNNING = CHECKER_FIRST_MSGID,
-	MSG_TUR_TIMEOUT,
-	MSG_TUR_FAILED,
+	MSG_TUR_FAILED = CHECKER_FIRST_MSGID,
 	MSG_TUR_TRANSITIONING,
 };
 
 #define IDX_(x) (MSG_ ## x - CHECKER_FIRST_MSGID)
 const char *libcheck_msgtable[] = {
-	[IDX_(TUR_RUNNING)] = " still running",
-	[IDX_(TUR_TIMEOUT)] = " timed out",
 	[IDX_(TUR_FAILED)] = " failed to initialize",
 	[IDX_(TUR_TRANSITIONING)] = " reports path is transitioning",
 	NULL,
 };
 
-struct tur_data {
-	int fd;
-	dev_t devt;
-	unsigned int timeout;
-	int state;
-	short msgid;
-};
-
-struct tur_checker_context {
-	struct checker_context chkr;
-	int last_runner_state;
-	unsigned int nr_timeouts;
-	struct runner_context *rtx;
-	struct tur_data tdata;
-};
-
-int libcheck_init(struct checker *c)
-{
-	struct tur_checker_context *tcc;
-	struct stat sb;
-
-	tcc = calloc(1, sizeof(*tcc));
-	tcc->tdata.state = PATH_UNCHECKED;
-	tcc->tdata.fd = -1;
-	if (fstat(c->fd, &sb) == 0)
-		tcc->tdata.devt = sb.st_rdev;
-	tcc->chkr.cls = c->cls;
-	c->context = tcc;
-	return 0;
-}
-
-void libcheck_free (struct checker * c)
-{
-	struct tur_checker_context *tcc = c->context;
-
-	if (!tcc)
-		return;
-	c->context = NULL;
-	if (tcc->rtx)
-		release_runner(tcc->rtx);
-	free(tcc);
-}
-
-static int
-tur_check(int fd, unsigned int timeout, short *msgid)
+int libcheck_async_func(struct runner_data *rdata)
 {
 	struct sg_io_hdr io_hdr;
 	unsigned char turCmdBlk[TUR_CMD_LEN] = { 0x00, 0, 0, 0, 0, 0 };
@@ -99,14 +43,14 @@ retry:
 	io_hdr.dxfer_direction = SG_DXFER_NONE;
 	io_hdr.cmdp = turCmdBlk;
 	io_hdr.sbp = sense_buffer;
-	io_hdr.timeout = timeout * 1000;
+	io_hdr.timeout = rdata->timeout * 1000;
 	io_hdr.pack_id = 0;
-	if (ioctl(fd, SG_IO, &io_hdr) < 0) {
+	if (ioctl(rdata->fd, SG_IO, &io_hdr) < 0) {
 		if (errno == ENOTTY) {
-			*msgid = CHECKER_MSGID_UNSUPPORTED;
+			rdata->msgid = CHECKER_MSGID_UNSUPPORTED;
 			return PATH_WILD;
 		}
-		*msgid = CHECKER_MSGID_DOWN;
+		rdata->msgid = CHECKER_MSGID_DOWN;
 		return PATH_DOWN;
 	}
 	if ((io_hdr.status & 0x7e) == 0x18) {
@@ -114,7 +58,7 @@ retry:
 		 * SCSI-3 arrays might return
 		 * reservation conflict on TUR
 		 */
-		*msgid = CHECKER_MSGID_UP;
+		rdata->msgid = CHECKER_MSGID_UP;
 		return PATH_UP;
 	}
 	if (io_hdr.info & SG_INFO_OK_MASK) {
@@ -159,14 +103,14 @@ retry:
 				 * LOGICAL UNIT NOT ACCESSIBLE,
 				 * TARGET PORT IN STANDBY STATE
 				 */
-				*msgid = CHECKER_MSGID_GHOST;
+				rdata->msgid = CHECKER_MSGID_GHOST;
 				return PATH_GHOST;
 			} else if (asc == 0x04 && ascq == 0x0a) {
 				/*
 				 * LOGICAL UNIT NOT ACCESSIBLE,
 				 * ASYMMETRIC ACCESS STATE TRANSITION
 				 */
-				*msgid = MSG_TUR_TRANSITIONING;
+				rdata->msgid = MSG_TUR_TRANSITIONING;
 				return PATH_PENDING;
 			}
 		} else if (key == 0x5) {
@@ -176,178 +120,13 @@ retry:
 				 * LUN NOT SUPPORTED: unmapped at target.
 				 * Signals pp->disconnected, becomes PATH_DOWN.
 				 */
-				*msgid = CHECKER_MSGID_DISCONNECTED;
+				rdata->msgid = CHECKER_MSGID_DISCONNECTED;
 				return PATH_DISCONNECTED;
 			}
 		}
-		*msgid = CHECKER_MSGID_DOWN;
+		rdata->msgid = CHECKER_MSGID_DOWN;
 		return PATH_DOWN;
 	}
-	*msgid = CHECKER_MSGID_UP;
+	rdata->msgid = CHECKER_MSGID_UP;
 	return PATH_UP;
 }
-
-/*
- * Test code for "zombie tur thread" handling.
- * Compile e.g. with CFLAGS=-DTUR_TEST_MAJOR=8
- * Additional parameters can be configure with the macros below.
- *
- * Everty nth started TUR thread will hang in non-cancellable state
- * for given number of seconds, for device given by major/minor.
- */
-#ifdef TUR_TEST_MAJOR
-
-#ifndef TUR_TEST_MINOR
-#define TUR_TEST_MINOR 0
-#endif
-#ifndef TUR_SLEEP_INTERVAL
-#define TUR_SLEEP_INTERVAL 3
-#endif
-#ifndef TUR_SLEEP_SECS
-#define TUR_SLEEP_SECS 60
-#endif
-
-static void tur_deep_sleep(const struct tur_data *tdata)
-{
-	static int sleep_cnt;
-	const struct timespec ts = { .tv_sec = TUR_SLEEP_SECS, .tv_nsec = 0 };
-	int oldstate;
-
-	if (tdata->devt != makedev(TUR_TEST_MAJOR, TUR_TEST_MINOR) ||
-	    ++sleep_cnt % TUR_SLEEP_INTERVAL == 0)
-		return;
-
-	condlog(3, "tur thread going to sleep for %ld seconds", ts.tv_sec);
-	if (pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &oldstate) != 0)
-		condlog(0, "pthread_setcancelstate: %m");
-	if (nanosleep(&ts, NULL) != 0)
-		condlog(0, "nanosleep: %m");
-	condlog(3, "tur zombie thread woke up");
-	if (pthread_setcancelstate(oldstate, NULL) != 0)
-		condlog(0, "pthread_setcancelstate (2): %m");
-	pthread_testcancel();
-}
-#else
-#define tur_deep_sleep(x) do {} while (0)
-#endif /* TUR_TEST_MAJOR */
-
-void runner_callback(void *arg)
-{
-	struct tur_data *tdata = arg;
-	int state;
-
-	condlog(4, "%d:%d : tur checker starting up", major(tdata->devt),
-		minor(tdata->devt));
-
-	tur_deep_sleep(tdata);
-	state = tur_check(tdata->fd, tdata->timeout, &tdata->msgid);
-	tdata->state = state;
-	pthread_testcancel();
-	condlog(4, "%d:%d : tur checker finished, state %s", major(tdata->devt),
-		minor(tdata->devt), checker_state_name(state));
-}
-
-static int check_runner_state(struct tur_checker_context *tcc)
-{
-	struct runner_context *rtx = tcc->rtx;
-	int rc;
-
-	rc = check_runner(rtx, &tcc->tdata, sizeof(tcc->tdata));
-	switch (rc) {
-	case RUNNER_DEAD:
-		tcc->tdata.state = PATH_TIMEOUT;
-		tcc->tdata.msgid = MSG_TUR_TIMEOUT;
-		/* fallthrough */
-	case RUNNER_DONE:
-		release_runner(tcc->rtx);
-		tcc->rtx = NULL;
-		tcc->last_runner_state = rc;
-		tcc->nr_timeouts = 0;
-		condlog(rc == RUNNER_DONE ? 4 : 3,
-			"%d:%d : tur checker finished, state %s, runner state %s",
-			major(tcc->tdata.devt), minor(tcc->tdata.devt),
-			checker_state_name(tcc->tdata.state),
-			runner_state_name(rc));
-		break;
-	case RUNNER_CANCELLED:
-		tcc->last_runner_state = rc;
-		tcc->tdata.state = PATH_TIMEOUT;
-		tcc->tdata.msgid = MSG_TUR_TIMEOUT;
-		if (tcc->nr_timeouts < MAX_NR_TIMEOUTS) {
-			condlog(3, "%d:%d : tur checker timed out, releasing it",
-				major(tcc->tdata.devt), minor(tcc->tdata.devt));
-			tcc->nr_timeouts++;
-			release_runner(tcc->rtx);
-			tcc->rtx = NULL;
-		} else if (tcc->nr_timeouts == MAX_NR_TIMEOUTS) {
-			tcc->nr_timeouts++;
-			condlog(3, "%d:%d : tur checker timed out, waiting for it",
-				major(tcc->tdata.devt), minor(tcc->tdata.devt));
-		}
-		break;
-	default:
-		condlog(4, "%d:%d : tur checker still running",
-			major(tcc->tdata.devt), minor(tcc->tdata.devt));
-		tcc->tdata.msgid = MSG_TUR_RUNNING;
-		break;
-	}
-	return rc;
-}
-
-bool libcheck_need_wait(struct checker *c)
-{
-	struct tur_checker_context *ct = c->context;
-
-	return ct && ct->rtx;
-}
-
-int libcheck_pending(struct checker *c)
-{
-	struct tur_checker_context *ct = c->context;
-	/* The if path checker isn't running, just return the exiting value. */
-	if (!ct || !ct->rtx)
-		return c->path_state;
-
-	/* This may nullify ct->rtx */
-	check_runner_state(ct);
-	c->msgid = ct->tdata.msgid;
-	return ct->tdata.state;
-}
-
-int libcheck_check(struct checker * c)
-{
-	struct tur_checker_context *ct = c->context;
-
-	if (!ct)
-		return PATH_UNCHECKED;
-
-	if (checker_is_sync(c))
-		return tur_check(c->fd, c->timeout, &c->msgid);
-
-	/* Handle the case that the checker just completed */
-	if (ct->rtx) {
-		check_runner_state(ct);
-		c->msgid = ct->tdata.msgid;
-		return ct->tdata.state;
-	}
-
-	/* create new checker thread */
-	ct->tdata.fd = c->fd;
-	ct->tdata.timeout = c->timeout;
-
-	ct->tdata.state = PATH_PENDING;
-	ct->tdata.msgid = MSG_TUR_RUNNING;
-	condlog(3, "%d:%d : starting checker", major(ct->tdata.devt),
-		minor(ct->tdata.devt));
-	ct->rtx = get_runner(runner_callback, &ct->tdata, sizeof(ct->tdata),
-			     1000000 * c->timeout);
-
-	if (ct->rtx) {
-		c->msgid = ct->tdata.msgid;
-		return ct->tdata.state;
-	} else {
-		condlog(3, "%d:%d : failed to start tur thread, using sync mode",
-			major(ct->tdata.devt), minor(ct->tdata.devt));
-		return tur_check(c->fd, c->timeout, &c->msgid);
-	}
-}
-- 
2.54.0

[PATCH 12/18] libmultipath: convert RDAC checker to async checker

[Martin Wilck]

Make the RDAC checker use the async checker framework.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/checkers/rdac.c | 30 ++++++++++--------------------
 1 file changed, 10 insertions(+), 20 deletions(-)

diff --git a/libmultipath/checkers/rdac.c b/libmultipath/checkers/rdac.c
index 87b8872..a477fb5 100644
--- a/libmultipath/checkers/rdac.c
+++ b/libmultipath/checkers/rdac.c
@@ -1,8 +1,6 @@
 /*
  * Copyright (c) 2005 Christophe Varoqui
  */
-#include <stdio.h>
-#include <stdlib.h>
 #include <string.h>
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -15,6 +13,7 @@
 #include "debug.h"
 
 #include "sg_include.h"
+#include "async_checker.h"
 
 #define INQUIRY_CMDLEN		6
 #define INQUIRY_CMD		0x12
@@ -55,11 +54,7 @@ struct control_mode_page {
 	unsigned char dontcare1[6];
 };
 
-struct rdac_checker_context {
-	void * dummy;
-};
-
-int libcheck_init (struct checker * c)
+int libcheck_init(struct checker *c)
 {
 	unsigned char cmd[MODE_SEN_SEL_CMDLEN];
 	unsigned char sense_b[SENSE_BUFF_LEN];
@@ -133,11 +128,6 @@ out:
 	return 0;
 }
 
-void libcheck_free(__attribute__((unused)) struct checker *c)
-{
-	return;
-}
-
 static int
 do_inq(int sg_fd, unsigned int pg_op, void *resp, int mx_resp_len,
        unsigned int timeout)
@@ -288,15 +278,15 @@ checker_msg_string(const struct volume_access_inq *inq)
 	}
 }
 
-int libcheck_check(struct checker * c)
+int libcheck_async_func(struct runner_data *rdata)
 {
 	struct volume_access_inq inq;
 	int ret, inqfail;
 
 	inqfail = 0;
 	memset(&inq, 0, sizeof(struct volume_access_inq));
-	ret = do_inq(c->fd, 0xC9, &inq, sizeof(struct volume_access_inq),
-		     c->timeout);
+	ret = do_inq(rdata->fd, 0xC9, &inq, sizeof(struct volume_access_inq),
+		     rdata->timeout);
 	if (ret != PATH_UP) {
 		inqfail = 1;
 		goto done;
@@ -340,17 +330,17 @@ int libcheck_check(struct checker * c)
 done:
 	switch (ret) {
 	case PATH_WILD:
-		c->msgid = CHECKER_MSGID_UNSUPPORTED;
+		rdata->msgid = CHECKER_MSGID_UNSUPPORTED;
 		break;
 	case PATH_DOWN:
-		c->msgid = (inqfail ? RDAC_MSGID_INQUIRY_FAILED :
-			    checker_msg_string(&inq));
+		rdata->msgid = (inqfail ? RDAC_MSGID_INQUIRY_FAILED
+					: checker_msg_string(&inq));
 		break;
 	case PATH_UP:
-		c->msgid = CHECKER_MSGID_UP;
+		rdata->msgid = CHECKER_MSGID_UP;
 		break;
 	case PATH_GHOST:
-		c->msgid = CHECKER_MSGID_GHOST;
+		rdata->msgid = CHECKER_MSGID_GHOST;
 		break;
 	}
 
-- 
2.54.0

[PATCH 13/18] libmultipath: convert cciss_tur checker to async checker

[Martin Wilck]

Make the cciss_tur checker use the async checker framework.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/checkers/cciss_tur.c | 41 ++++++++-----------------------
 1 file changed, 10 insertions(+), 31 deletions(-)

diff --git a/libmultipath/checkers/cciss_tur.c b/libmultipath/checkers/cciss_tur.c
index c89fe5e..b859138 100644
--- a/libmultipath/checkers/cciss_tur.c
+++ b/libmultipath/checkers/cciss_tur.c
@@ -24,47 +24,33 @@
 #include <errno.h>
 
 #include "checkers.h"
+#include "async_checker.h"
 
 #include "cciss.h"
 
 #define TUR_CMD_LEN 6
 #define HEAVY_CHECK_COUNT       10
 
-struct cciss_tur_checker_context {
-	void * dummy;
-};
-
-int libcheck_init (__attribute__((unused)) struct checker * c)
-{
-	return 0;
-}
-
-void libcheck_free (__attribute__((unused)) struct checker * c)
-{
-	return;
-}
-
-int libcheck_check(struct checker * c)
+int libcheck_async_func(struct runner_data *rdata)
 {
 	int rc;
 	int ret;
 	unsigned int lun = 0;
-	struct cciss_tur_checker_context * ctxt = NULL;
 	LogvolInfo_struct    lvi;       // logical "volume" info
 	IOCTL_Command_struct cic;       // cciss ioctl command
 
-	if ((c->fd) < 0) {
-		c->msgid = CHECKER_MSGID_NO_FD;
+	if ((rdata->fd) < 0) {
+		rdata->msgid = CHECKER_MSGID_NO_FD;
 		ret = -1;
 		goto out;
 	}
 
-	rc = ioctl(c->fd, CCISS_GETLUNINFO, &lvi);
+	rc = ioctl(rdata->fd, CCISS_GETLUNINFO, &lvi);
 	if ( rc != 0) {
 		perror("Error: ");
 		fprintf(stderr, "cciss TUR  failed in CCISS_GETLUNINFO: %s\n",
 			strerror(errno));
-		c->msgid = CHECKER_MSGID_DOWN;
+		rdata->msgid = CHECKER_MSGID_DOWN;
 		ret = PATH_DOWN;
 		goto out;
 	} else {
@@ -87,31 +73,24 @@ int libcheck_check(struct checker * c)
 	cic.Request.CDB[4] = 0;
 	cic.Request.CDB[5] = 0;
 
-	rc = ioctl(c->fd, CCISS_PASSTHRU, &cic);
+	rc = ioctl(rdata->fd, CCISS_PASSTHRU, &cic);
 	if (rc < 0) {
 		fprintf(stderr, "cciss TUR  failed: %s\n",
 			strerror(errno));
-		c->msgid = CHECKER_MSGID_DOWN;
+		rdata->msgid = CHECKER_MSGID_DOWN;
 		ret = PATH_DOWN;
 		goto out;
 	}
 
 	if ((cic.error_info.CommandStatus | cic.error_info.ScsiStatus )) {
-		c->msgid = CHECKER_MSGID_DOWN;
+		rdata->msgid = CHECKER_MSGID_DOWN;
 		ret = PATH_DOWN;
 		goto out;
 	}
 
-	c->msgid = CHECKER_MSGID_UP;
+	rdata->msgid = CHECKER_MSGID_UP;
 
 	ret = PATH_UP;
 out:
-	/*
-	 * caller told us he doesn't want to keep the context :
-	 * free it
-	 */
-	if (!c->context)
-		free(ctxt);
-
 	return(ret);
 }
-- 
2.54.0

[PATCH 14/18] libmultipath: convert readsector0 checker to async checker

[Martin Wilck]

Make the readsector0 checker use the async checker framework.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/checkers/readsector0.c | 28 +++++++---------------------
 1 file changed, 7 insertions(+), 21 deletions(-)

diff --git a/libmultipath/checkers/readsector0.c b/libmultipath/checkers/readsector0.c
index b041f11..fcc4ad1 100644
--- a/libmultipath/checkers/readsector0.c
+++ b/libmultipath/checkers/readsector0.c
@@ -1,41 +1,27 @@
 /*
  * Copyright (c) 2004, 2005 Christophe Varoqui
  */
-#include <stdio.h>
-
+#include <sys/types.h>
 #include "checkers.h"
 #include "libsg.h"
+#include "async_checker.h"
 
-struct readsector0_checker_context {
-	void * dummy;
-};
-
-int libcheck_init (__attribute__((unused)) struct checker * c)
-{
-	return 0;
-}
-
-void libcheck_free (__attribute__((unused)) struct checker * c)
-{
-	return;
-}
-
-int libcheck_check (struct checker * c)
+int libcheck_async_func(struct runner_data *rdata)
 {
 	unsigned char buf[4096];
 	unsigned char sbuf[SENSE_BUFF_LEN];
 	int ret;
 
-	ret = sg_read(c->fd, &buf[0], 4096, &sbuf[0],
-		      SENSE_BUFF_LEN, c->timeout);
+	ret = sg_read(rdata->fd, &buf[0], 4096, &sbuf[0], SENSE_BUFF_LEN,
+		      rdata->timeout);
 
 	switch (ret)
 	{
 	case PATH_DOWN:
-		c->msgid = CHECKER_MSGID_DOWN;
+		rdata->msgid = CHECKER_MSGID_DOWN;
 		break;
 	case PATH_UP:
-		c->msgid = CHECKER_MSGID_UP;
+		rdata->msgid = CHECKER_MSGID_UP;
 		break;
 	default:
 		break;
-- 
2.54.0

[PATCH 15/18] libmultipath: convert hp_sw checker to async checker

[Martin Wilck]

Make the hp_sw checker use the async checker framework.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/checkers/hp_sw.c | 32 ++++++++------------------------
 1 file changed, 8 insertions(+), 24 deletions(-)

diff --git a/libmultipath/checkers/hp_sw.c b/libmultipath/checkers/hp_sw.c
index 1ab7909..770221f 100644
--- a/libmultipath/checkers/hp_sw.c
+++ b/libmultipath/checkers/hp_sw.c
@@ -1,8 +1,6 @@
 /*
  * Copyright (c) 2005 Christophe Varoqui
  */
-#include <stdio.h>
-#include <stdlib.h>
 #include <string.h>
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -12,7 +10,7 @@
 #include <errno.h>
 
 #include "checkers.h"
-
+#include "async_checker.h"
 #include "sg_include.h"
 #include "unaligned.h"
 
@@ -28,20 +26,6 @@
 #define MX_ALLOC_LEN		255
 #define HEAVY_CHECK_COUNT       10
 
-struct sw_checker_context {
-	void * dummy;
-};
-
-int libcheck_init (__attribute__((unused)) struct checker * c)
-{
-	return 0;
-}
-
-void libcheck_free (__attribute__((unused)) struct checker * c)
-{
-	return;
-}
-
 static int
 do_inq(int sg_fd, int cmddt, int evpd, unsigned int pg_op,
        void *resp, int mx_resp_len, unsigned int timeout)
@@ -127,24 +111,24 @@ do_tur (int fd, unsigned int timeout)
 	return 0;
 }
 
-int libcheck_check(struct checker * c)
+int libcheck_async_func(struct runner_data *rdata)
 {
 	char buff[MX_ALLOC_LEN];
-	int ret = do_inq(c->fd, 0, 1, 0x80, buff, MX_ALLOC_LEN, c->timeout);
+	int ret = do_inq(rdata->fd, 0, 1, 0x80, buff, MX_ALLOC_LEN, rdata->timeout);
 
 	if (ret == PATH_WILD) {
-		c->msgid = CHECKER_MSGID_UNSUPPORTED;
+		rdata->msgid = CHECKER_MSGID_UNSUPPORTED;
 		return ret;
 	}
 	if (ret != PATH_UP) {
-		c->msgid = CHECKER_MSGID_DOWN;
+		rdata->msgid = CHECKER_MSGID_DOWN;
 		return ret;
 	};
 
-	if (do_tur(c->fd, c->timeout)) {
-		c->msgid = CHECKER_MSGID_GHOST;
+	if (do_tur(rdata->fd, rdata->timeout)) {
+		rdata->msgid = CHECKER_MSGID_GHOST;
 		return PATH_GHOST;
 	}
-	c->msgid = CHECKER_MSGID_UP;
+	rdata->msgid = CHECKER_MSGID_UP;
 	return PATH_UP;
 }
-- 
2.54.0

[PATCH 16/18] libmultipath: async_checker: add context_size and init symbols

[Martin Wilck]

Add two symbols that can be read from checker .so files with dlsym():
"async_context_size" for defining the size of a checker-private context,
and "async_init" for initializing this context.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/async_checker.c | 31 +++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/libmultipath/async_checker.c b/libmultipath/async_checker.c
index 06159dd..52e24f2 100644
--- a/libmultipath/async_checker.c
+++ b/libmultipath/async_checker.c
@@ -6,37 +6,64 @@
 #include <sys/stat.h>
 #include <stdlib.h>
 #include <stdbool.h>
+#include <dlfcn.h>
 #include "async_checker.h"
 #include "checkers.h"
 #include "debug.h"
 #include "runner.h"
 
 #define MAX_NR_TIMEOUTS 1
+typedef int (*async_init_func)(struct runner_data *);
 
 struct async_checker_context {
 	struct checker_context chkr;
 	int last_runner_state;
 	unsigned int nr_timeouts;
 	struct runner_context *rtx;
+	async_init_func async_init;
+	int async_context_size;
 	struct runner_data rdata;
 };
 
-#define rdata_size(acc) (sizeof(acc->rdata))
+#define rdata_size(acc) (sizeof(acc->rdata) + acc->async_context_size)
 
 int async_check_init(struct checker *c)
 {
 	struct async_checker_context *acc;
 	struct stat sb;
+	const int *p_ctx_size;
+	int ctx_size;
+	char *errstr __attribute__((unused));
 
-	acc = calloc(1, sizeof(*acc));
+	p_ctx_size = dlsym(c->cls->handle, "async_context_size");
+	errstr = dlerror();
+	if (p_ctx_size)
+		ctx_size = *p_ctx_size;
+	else
+		ctx_size = 0;
+
+	if (ctx_size < 0 || ctx_size > CHECKER_MAX_CONTEXT_SIZE) {
+		condlog(0, "%s: invalid context size %d", __func__, ctx_size);
+		return -1;
+	}
+	condlog(3, "%s: extra context size: %d", __func__, ctx_size);
+	acc = calloc(1, sizeof(*acc) + ctx_size);
 	if (!acc)
 		return -1;
+
+	acc->async_context_size = ctx_size;
+	acc->async_init =
+		(int (*)(struct runner_data *))dlsym(c->cls->handle, "async_init");
+	errstr = dlerror();
+
 	acc->rdata.state = PATH_UNCHECKED;
 	acc->rdata.fd = -1;
 	if (fstat(c->fd, &sb) == 0)
 		acc->rdata.devt = sb.st_rdev;
 	acc->chkr.cls = c->cls;
 	c->context = acc;
+	if (acc->async_init)
+		return acc->async_init(&acc->rdata);
 	return 0;
 }
 
-- 
2.54.0

[PATCH 17/18] libmultipath: checkers: rework mpcontext passing

[Martin Wilck]

The emc_clariion path checker requires an "mpcontext" that is shared
between the checkers for all paths of a given multipath map. This context
is currently represented in struct multipath as a void *, the pointer to
which is passed to the checker. The checker allocates the actual memory in
the libcheck_mp_init() function, and changes the pointer in struct
multipath. This is a dangerous layering violation as the checker operates
on memory it doesn't own, and not type-safe at all.

This patch changes the mpcontext handling as follows: The void * pointer
is replaced by a dedicated union checker_mpcontext. It only contains
a long integer element, which is sufficient to store the information
needed by emc_clariion.

Instead of initializing this value in mp_init, the address of the union is
passed to libcheck_check() and libcheck_pending() if the path that is being
checked has an associated struct multipath. Passing a union pointer
improves compile-time type safety. In order to check whether the value is
initialized, instead of testing for a NULL pointer like before, the code
defines an INVALID_MPCONTEXT value. This value must obviously be chosen
such that it doesn't represent a valid context value, which is the case for
emc_clariion.

For a synchronous checker (like emc_clariion), libcheck_check() is allowed
to write to this memory, because at the time of the call we know that
pp->mpp is valid. The same holds for libcheck_pending().

This change requires checker_check() and checker_get_state() to be passed
a "struct path *" rather than a "struct checker *". The layer separation
happens in these functions now.

The async_checker code also gets an implementation of mpcontext handling,
in preparation of converting emc_clariion to an async checker.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/async_checker.c         | 32 +++++++++++-------
 libmultipath/async_checker.h         |  5 +--
 libmultipath/checkers.c              | 46 +++++++++++---------------
 libmultipath/checkers.h              | 44 ++++++++++++++++---------
 libmultipath/checkers/directio.c     |  6 ++--
 libmultipath/checkers/emc_clariion.c | 49 ++++++++++------------------
 libmultipath/discovery.c             |  8 ++---
 libmultipath/libmultipath.version    |  2 +-
 libmultipath/structs.c               |  3 +-
 libmultipath/structs.h               |  3 +-
 tests/directio.c                     |  2 +-
 11 files changed, 100 insertions(+), 100 deletions(-)

diff --git a/libmultipath/async_checker.c b/libmultipath/async_checker.c
index 52e24f2..63704aa 100644
--- a/libmultipath/async_checker.c
+++ b/libmultipath/async_checker.c
@@ -7,8 +7,8 @@
 #include <stdlib.h>
 #include <stdbool.h>
 #include <dlfcn.h>
-#include "async_checker.h"
 #include "checkers.h"
+#include "async_checker.h"
 #include "debug.h"
 #include "runner.h"
 
@@ -61,6 +61,7 @@ int async_check_init(struct checker *c)
 	if (fstat(c->fd, &sb) == 0)
 		acc->rdata.devt = sb.st_rdev;
 	acc->chkr.cls = c->cls;
+	SET_INVALID_MPCONTEXT(acc->rdata.mpc);
 	c->context = acc;
 	if (acc->async_init)
 		return acc->async_init(&acc->rdata);
@@ -149,35 +150,42 @@ bool async_check_need_wait(struct checker *c)
 	return acc && acc->rtx;
 }
 
-int async_check_pending(struct checker *c)
+int async_check_pending(struct checker *c, union checker_mpcontext *mpc)
 {
 	struct async_checker_context *acc = c->context;
+	int rc;
 	/* The if path checker isn't running, just return the exiting value. */
 	if (!acc || !acc->rtx)
 		return c->path_state;
 
-	/* This may nullify ct->rtx */
-	check_runner_state(acc);
+	/* This may nullify acc->rtx */
+	rc = check_runner_state(acc);
 	c->msgid = acc->rdata.msgid;
+	if (rc == RUNNER_DONE && mpc && !IS_INVALID_MPCONTEXT(acc->rdata.mpc))
+		*mpc = acc->rdata.mpc;
 	return acc->rdata.state;
 }
 
-int async_check_check(struct checker *c)
+int async_check_check(struct checker *c, union checker_mpcontext *mpc)
 {
 	struct async_checker_context *acc = c->context;
 
 	if (!acc)
 		return PATH_UNCHECKED;
 
-	if (checker_is_sync(c))
-		return acc->rdata.afunc(&acc->rdata);
+	if (mpc && !IS_INVALID_MPCONTEXT(*mpc))
+		acc->rdata.mpc = *mpc;
 
-	/* Handle the case that the checker just completed */
-	if (acc->rtx) {
-		check_runner_state(acc);
-		c->msgid = acc->rdata.msgid;
-		return acc->rdata.state;
+	if (checker_is_sync(c)) {
+		int rc = acc->rdata.afunc(&acc->rdata);
+
+		if (mpc && !IS_INVALID_MPCONTEXT(acc->rdata.mpc))
+			*mpc = acc->rdata.mpc;
+		return rc;
 	}
+	/* Handle the case that the checker just completed */
+	if (acc->rtx)
+		return async_check_pending(c, mpc);
 
 	/* create new checker thread */
 	acc->rdata.fd = c->fd;
diff --git a/libmultipath/async_checker.h b/libmultipath/async_checker.h
index 835de2a..10948d1 100644
--- a/libmultipath/async_checker.h
+++ b/libmultipath/async_checker.h
@@ -14,14 +14,15 @@ struct runner_data {
 	unsigned int timeout;
 	int state;
 	short msgid;
+	union checker_mpcontext mpc;
 	char checker_ctx[];
 };
 
 int async_check_init(struct checker *c);
 void async_check_free(struct checker *c);
 bool async_check_need_wait(struct checker *c);
-int async_check_pending(struct checker *c);
-int async_check_check(struct checker *c);
+int async_check_pending(struct checker *c, union checker_mpcontext *mpctx);
+int async_check_check(struct checker *c, union checker_mpcontext *mpctx);
 
 #define CHECKER_MAX_CONTEXT_SIZE 1024
 
diff --git a/libmultipath/checkers.c b/libmultipath/checkers.c
index 2f20b25..12d928a 100644
--- a/libmultipath/checkers.c
+++ b/libmultipath/checkers.c
@@ -12,6 +12,7 @@
 #include "async_checker.h"
 #include "vector.h"
 #include "util.h"
+#include "structs.h"
 
 static const char * const checker_dir = MULTIPATH_DIR;
 
@@ -164,7 +165,8 @@ static struct checker_class *add_checker_class(const char *name)
 	if (c->async_func)
 		return add_async_checker_class(c);
 
-	c->check = (int (*)(struct checker *)) dlsym(c->handle, "libcheck_check");
+	c->check = (int (*)(struct checker *, union checker_mpcontext *))
+		dlsym(c->handle, "libcheck_check");
 	errstr = dlerror();
 	if (errstr != NULL)
 		condlog(0, "A dynamic linking error occurred: (%s)", errstr);
@@ -178,9 +180,9 @@ static struct checker_class *add_checker_class(const char *name)
 	if (!c->init)
 		goto out;
 
-	c->mp_init = (int (*)(struct checker *)) dlsym(c->handle, "libcheck_mp_init");
 	c->reset = (void (*)(void))dlsym(c->handle, "libcheck_reset");
-	c->pending = (int (*)(struct checker *)) dlsym(c->handle, "libcheck_pending");
+	c->pending = (int (*)(struct checker *, union checker_mpcontext *))
+		dlsym(c->handle, "libcheck_pending");
 	c->need_wait = (bool (*)(struct checker *)) dlsym(c->handle, "libcheck_need_wait");
 	/* These 5 functions can be NULL. call dlerror() to clear out any
 	 * error string */
@@ -239,31 +241,12 @@ void checker_disable (struct checker * c)
 	c->path_state = PATH_UNCHECKED;
 }
 
-int checker_init (struct checker * c, void ** mpctxt_addr)
+int checker_init(struct checker *c)
 {
 	if (!c || !c->cls)
 		return 1;
-	c->mpcontext = mpctxt_addr;
 	if (c->cls->init && c->cls->init(c) != 0)
 		return 1;
-	if (mpctxt_addr && *mpctxt_addr == NULL && c->cls->mp_init &&
-	    c->cls->mp_init(c) != 0) /* continue even if mp_init fails */
-		c->mpcontext = NULL;
-	return 0;
-}
-
-int checker_mp_init(struct checker * c, void ** mpctxt_addr)
-{
-	if (!c || !c->cls)
-		return 1;
-	if (c->mpcontext || !mpctxt_addr)
-		return 0;
-	c->mpcontext = mpctxt_addr;
-	if (*mpctxt_addr == NULL && c->cls->mp_init &&
-	    c->cls->mp_init(c) != 0) {
-		c->mpcontext = NULL;
-		return 1;
-	}
 	return 0;
 }
 
@@ -287,13 +270,17 @@ void checker_put (struct checker * dst)
 	put_shared_ptr(src);
 }
 
-int checker_get_state(struct checker *c)
+int checker_get_state(struct path *pp)
 {
+	struct checker *c = &pp->checker;
+	union checker_mpcontext *mpc;
+
 	if (!c || !c->cls)
 		return PATH_UNCHECKED;
 	if (c->path_state != PATH_PENDING || !c->cls->pending)
 		return c->path_state;
-	c->path_state = c->cls->pending(c);
+	mpc = pp->mpp ? &pp->mpp->mpcontext : NULL;
+	c->path_state = c->cls->pending(c, mpc);
 	return c->path_state;
 }
 
@@ -305,8 +292,10 @@ bool checker_need_wait(struct checker *c)
 	return c->cls->need_wait(c);
 }
 
-void checker_check (struct checker * c, int path_state)
+void checker_check(struct path *pp, int path_state)
 {
+	struct checker *c = &pp->checker;
+
 	if (!c)
 		return;
 
@@ -320,7 +309,10 @@ void checker_check (struct checker * c, int path_state)
 		c->msgid = CHECKER_MSGID_NO_FD;
 		c->path_state = PATH_WILD;
 	} else {
-		c->path_state = c->cls->check(c);
+		union checker_mpcontext *mpc;
+
+		mpc = pp->mpp ? &pp->mpp->mpcontext : NULL;
+		c->path_state = c->cls->check(c, mpc);
 	}
 }
 
diff --git a/libmultipath/checkers.h b/libmultipath/checkers.h
index 744be54..eb3455f 100644
--- a/libmultipath/checkers.h
+++ b/libmultipath/checkers.h
@@ -131,6 +131,23 @@ enum {
 	CHECKER_MSGTABLE_SIZE = 100, /* max msg table size for checkers */
 };
 
+/*
+ * Data shared between checkers for a struct multipath.
+ * Should be large enough to hold the data for all checker types.
+ * Currently only used by emc_clariion.
+ */
+union checker_mpcontext {
+	long long_val;
+};
+
+#define INVALID_MPCONTEXT__ -1L
+#define IS_INVALID_MPCONTEXT(mpc) ((mpc).long_val == INVALID_MPCONTEXT__)
+#define SET_INVALID_MPCONTEXT(mpc)			\
+	do {						\
+		(mpc).long_val = INVALID_MPCONTEXT__;	\
+	} while (0)
+
+struct path;
 struct checker;
 struct runner_data;
 struct checker_class {
@@ -138,12 +155,12 @@ struct checker_class {
 	void *handle;
 	int sync;
 	char name[CHECKER_NAME_LEN];
-	int (*check)(struct checker *);
-	int (*init)(struct checker *);	  /* to allocate the context */
-	int (*mp_init)(struct checker *); /* to allocate the mpcontext */
-	void (*free)(struct checker *);	  /* to free the context */
-	void (*reset)(void);		  /* to reset the global variables */
-	int (*pending)(struct checker *); /* to recheck pending paths */
+	int (*check)(struct checker *, union checker_mpcontext *);
+	int (*init)(struct checker *);	/* to allocate the context */
+	void (*free)(struct checker *); /* to free the context */
+	void (*reset)(void);		/* to reset the global variables */
+	int (*pending)(struct checker *,
+		       union checker_mpcontext *); /* to recheck pending paths */
 	bool (*need_wait)(struct checker *); /* checker needs waiting for */
 	int (*async_func)(struct runner_data *); /* callback for async_checker */
 	const char **msgtable;
@@ -157,9 +174,7 @@ struct checker {
 	int disable;
 	int path_state;
 	short msgid;		             /* checker-internal extra status */
-	void * context;                      /* store for persistent data */
-	void ** mpcontext;                   /* store for persistent data shared
-						multipath-wide. */
+	void *context;			     /* store for persistent data */
 };
 
 static inline int checker_selected(const struct checker *c)
@@ -170,8 +185,7 @@ static inline int checker_selected(const struct checker *c)
 const char *checker_state_name(int);
 int init_checkers(void);
 void cleanup_checkers (void);
-int checker_init (struct checker *, void **);
-int checker_mp_init(struct checker *, void **);
+int checker_init(struct checker *);
 void checker_clear (struct checker *);
 void checker_put (struct checker *);
 void checker_reset (struct checker *);
@@ -202,9 +216,9 @@ struct checker_context {
 };
 int start_checker_thread (pthread_t *thread, const pthread_attr_t *attr,
 			  struct checker_context *ctx);
-int checker_get_state(struct checker *c);
+int checker_get_state(struct path *pp);
 bool checker_need_wait(struct checker *c);
-void checker_check (struct checker *, int);
+void checker_check(struct path *, int);
 int checker_is_sync(const struct checker *);
 const char *checker_name (const struct checker *);
 void reset_checker_classes(void);
@@ -217,13 +231,13 @@ void checker_clear_message (struct checker *c);
 void checker_get(struct checker *, const char *);
 
 /* Prototypes for symbols exported by path checker dynamic libraries (.so) */
-int libcheck_check(struct checker *);
+int libcheck_check(struct checker *, union checker_mpcontext *);
 int libcheck_init(struct checker *);
 void libcheck_free(struct checker *);
 void *libcheck_thread(struct checker_context *ctx);
 void libcheck_reset(void);
 int libcheck_mp_init(struct checker *);
-int libcheck_pending(struct checker *c);
+int libcheck_pending(struct checker *c, union checker_mpcontext *);
 bool libcheck_need_wait(struct checker *c);
 
 /*
diff --git a/libmultipath/checkers/directio.c b/libmultipath/checkers/directio.c
index a7422a8..82b2041 100644
--- a/libmultipath/checkers/directio.c
+++ b/libmultipath/checkers/directio.c
@@ -405,7 +405,8 @@ bool libcheck_need_wait(struct checker *c)
 		!ct->checked_state);
 }
 
-int libcheck_pending(struct checker *c)
+int libcheck_pending(struct checker *c,
+		     union checker_mpcontext *mpc __attribute__((unused)))
 {
 	int rc;
 	struct io_event event;
@@ -441,7 +442,8 @@ out:
 	return rc;
 }
 
-int libcheck_check (struct checker * c)
+int libcheck_check(struct checker *c,
+		   union checker_mpcontext *mpctx __attribute__((unused)))
 {
 	int ret;
 	struct directio_context * ct = (struct directio_context *)c->context;
diff --git a/libmultipath/checkers/emc_clariion.c b/libmultipath/checkers/emc_clariion.c
index bf0baab..24a386c 100644
--- a/libmultipath/checkers/emc_clariion.c
+++ b/libmultipath/checkers/emc_clariion.c
@@ -15,6 +15,7 @@
 #include "libsg.h"
 #include "checkers.h"
 #include "debug.h"
+#include "util.h"
 
 #define INQUIRY_CMD     0x12
 #define INQUIRY_CMDLEN  6
@@ -32,18 +33,20 @@
  * simple read test would return 02/04/03 instead
  * of 05/25/01 sensekey/ASC/ASCQ data.
  */
-#define	IS_INACTIVE_SNAP(c)   (c->mpcontext ?				   \
-			       ((struct emc_clariion_checker_LU_context *) \
-					(*c->mpcontext))->inactive_snap	   \
-					    : 0)
+#define IS_INACTIVE_SNAP(mpctx)				\
+	(!mpctx || IS_INVALID_MPCONTEXT(*mpctx) ? 0 : mpctx->long_val)
 
-#define	SET_INACTIVE_SNAP(c)  if (c->mpcontext)				   \
-				((struct emc_clariion_checker_LU_context *)\
-					(*c->mpcontext))->inactive_snap = 1
+#define SET_INACTIVE_SNAP(mpctx)		\
+	do {					\
+		if (mpctx)			\
+			mpctx->long_val = 1;	\
+	} while (0);
 
-#define	CLR_INACTIVE_SNAP(c)  if (c->mpcontext)				   \
-				((struct emc_clariion_checker_LU_context *)\
-					(*c->mpcontext))->inactive_snap = 0
+#define CLR_INACTIVE_SNAP(mpctx)		\
+	do {					\
+		if (mpctx)			\
+			mpctx->long_val = 0;	\
+	} while (0);
 
 enum {
 	MSG_CLARIION_QUERY_FAILED = CHECKER_FIRST_MSGID,
@@ -109,28 +112,12 @@ int libcheck_init (struct checker * c)
 	return 0;
 }
 
-int libcheck_mp_init (struct checker * c)
-{
-	/*
-	 * Allocate and initialize the multi-path global context.
-	 */
-	if (c->mpcontext && *c->mpcontext == NULL) {
-		void * mpctxt = malloc(sizeof(int));
-		if (!mpctxt)
-			return 1;
-		*c->mpcontext = mpctxt;
-		CLR_INACTIVE_SNAP(c);
-	}
-
-	return 0;
-}
-
-void libcheck_free (struct checker * c)
+void libcheck_free(struct checker *c)
 {
 	free(c->context);
 }
 
-int libcheck_check (struct checker * c)
+int libcheck_check(struct checker *c, union checker_mpcontext *mpctx)
 {
 	unsigned char sense_buffer[128] = { 0, };
 	unsigned char sb[SENSE_BUFF_LEN] = { 0, }, *sbb;
@@ -282,7 +269,7 @@ retry:
 				 * passive paths which will return
 				 * 02/04/03 not 05/25/01 on read.
 				 */
-				SET_INACTIVE_SNAP(c);
+				SET_INACTIVE_SNAP(mpctx);
 				condlog(3, "emc_clariion_checker: Active "
 					"path to inactive snapshot WWN %s.",
 					wwnstr);
@@ -300,10 +287,10 @@ retry:
 			 * snapshot LUs if it was in this list since the
 			 * snapshot is no longer inactive.
 			 */
-			CLR_INACTIVE_SNAP(c);
+			CLR_INACTIVE_SNAP(mpctx);
 		}
 	} else {
-		if (IS_INACTIVE_SNAP(c)) {
+		if (IS_INACTIVE_SNAP(mpctx)) {
 			hexadecimal_to_ascii(ct->wwn, wwnstr);
 			condlog(3, "emc_clariion_checker: Passive "
 				"path to inactive snapshot WWN %s.",
diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index f59fa6d..ceca9fd 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -1998,20 +1998,18 @@ start_checker (struct path * pp, struct config *conf, int daemon, int oldstate)
 			return -1;
 		}
 		checker_set_fd(c, pp->fd);
-		if (checker_init(c, pp->mpp?&pp->mpp->mpcontext:NULL)) {
+		if (checker_init(c)) {
 			checker_clear(c);
 			condlog(3, "%s: checker init failed", pp->dev);
 			return -1;
 		}
 	}
-	if (pp->mpp && !c->mpcontext)
-		checker_mp_init(c, &pp->mpp->mpcontext);
 	checker_clear_message(c);
 	if (conf->force_sync == 0)
 		checker_set_async(c);
 	else
 		checker_set_sync(c);
-	checker_check(c, oldstate);
+	checker_check(pp, oldstate);
 	return 0;
 }
 
@@ -2021,7 +2019,7 @@ get_state (struct path * pp)
 	struct checker * c = &pp->checker;
 	int state, lvl;
 
-	state = checker_get_state(c);
+	state = checker_get_state(pp);
 
 	lvl = state == pp->oldstate || state == PATH_PENDING ? 4 : 3;
 	condlog(lvl, "%s: %s state = %s", pp->dev,
diff --git a/libmultipath/libmultipath.version b/libmultipath/libmultipath.version
index 5e8dbda..d2876c7 100644
--- a/libmultipath/libmultipath.version
+++ b/libmultipath/libmultipath.version
@@ -43,7 +43,7 @@ LIBMPATHCOMMON_1.0.0 {
 	put_multipath_config;
 };
 
-LIBMULTIPATH_33.0.0 {
+LIBMULTIPATH_34.0.0 {
 global:
 	/* symbols referenced by multipath and multipathd */
 	add_foreign;
diff --git a/libmultipath/structs.c b/libmultipath/structs.c
index f441312..c627b2c 100644
--- a/libmultipath/structs.c
+++ b/libmultipath/structs.c
@@ -261,7 +261,7 @@ alloc_multipath (void)
 
 	if (mpp) {
 		mpp->bestpg = 1;
-		mpp->mpcontext = NULL;
+		SET_INVALID_MPCONTEXT(mpp->mpcontext);
 		mpp->no_path_retry = NO_PATH_RETRY_UNDEF;
 		dm_multipath_to_gen(mpp)->ops = &dm_gen_multipath_ops;
 	}
@@ -330,7 +330,6 @@ void free_multipath(struct multipath *mpp)
 		vector_free(mpp->hwe);
 		mpp->hwe = NULL;
 	}
-	free(mpp->mpcontext);
 	free(mpp);
 }
 
diff --git a/libmultipath/structs.h b/libmultipath/structs.h
index 9adedde..8b71aa2 100644
--- a/libmultipath/structs.h
+++ b/libmultipath/structs.h
@@ -540,8 +540,7 @@ struct multipath {
 	unsigned int stat_queueing_timeouts;
 	unsigned int stat_map_failures;
 
-	/* checkers shared data */
-	void * mpcontext;
+	union checker_mpcontext mpcontext;
 
 	/* persistent management data*/
 	int prkey_source;
diff --git a/tests/directio.c b/tests/directio.c
index fa1e553..addb7cf 100644
--- a/tests/directio.c
+++ b/tests/directio.c
@@ -227,7 +227,7 @@ void do_check_state(struct checker *c, int sync, int chk_state)
 
 void do_libcheck_pending(struct checker *c, int chk_state)
 {
-	assert_int_equal(libcheck_pending(c), chk_state);
+	assert_int_equal(libcheck_pending(c, NULL), chk_state);
 	assert_int_equal(ev_off, 0);
 	memset(mock_events, 0, sizeof(mock_events));
 }
-- 
2.54.0

[PATCH 18/18] libmultipath: convert emc_clariion to async_checker

[Martin Wilck]

---
 libmultipath/checkers/emc_clariion.c | 79 ++++++++++++----------------
 1 file changed, 33 insertions(+), 46 deletions(-)

diff --git a/libmultipath/checkers/emc_clariion.c b/libmultipath/checkers/emc_clariion.c
index 24a386c..2c9d4fd 100644
--- a/libmultipath/checkers/emc_clariion.c
+++ b/libmultipath/checkers/emc_clariion.c
@@ -1,8 +1,6 @@
 /*
  * Copyright (c) 2004, 2005 Lars Marowsky-Bree
  */
-#include <stdio.h>
-#include <stdlib.h>
 #include <string.h>
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -14,8 +12,8 @@
 #include "sg_include.h"
 #include "libsg.h"
 #include "checkers.h"
+#include "async_checker.h"
 #include "debug.h"
-#include "util.h"
 
 #define INQUIRY_CMD     0x12
 #define INQUIRY_CMDLEN  6
@@ -33,19 +31,17 @@
  * simple read test would return 02/04/03 instead
  * of 05/25/01 sensekey/ASC/ASCQ data.
  */
-#define IS_INACTIVE_SNAP(mpctx)				\
-	(!mpctx || IS_INVALID_MPCONTEXT(*mpctx) ? 0 : mpctx->long_val)
+#define IS_INACTIVE_SNAP(mpctx)					\
+	(IS_INVALID_MPCONTEXT(mpctx) ? 0 : mpctx.long_val)
 
 #define SET_INACTIVE_SNAP(mpctx)		\
 	do {					\
-		if (mpctx)			\
-			mpctx->long_val = 1;	\
+		mpctx.long_val = 1;		\
 	} while (0);
 
 #define CLR_INACTIVE_SNAP(mpctx)		\
 	do {					\
-		if (mpctx)			\
-			mpctx->long_val = 0;	\
+		mpctx.long_val = 0;		\
 	} while (0);
 
 enum {
@@ -86,7 +82,9 @@ struct emc_clariion_checker_LU_context {
 	int inactive_snap;
 };
 
-void hexadecimal_to_ascii(char * wwn, char *wwnstr)
+int async_context_size = sizeof(struct emc_clariion_checker_path_context);
+
+static void hexadecimal_to_ascii(char *wwn, char *wwnstr)
 {
 	int i,j, nbl;
 
@@ -99,33 +97,22 @@ void hexadecimal_to_ascii(char * wwn, char *wwnstr)
 	wwnstr[32]=0;
 }
 
-int libcheck_init (struct checker * c)
+int async_init(struct runner_data *rdata)
 {
-	/*
-	 * Allocate and initialize the path specific context.
-	 */
-	c->context = calloc(1, sizeof(struct emc_clariion_checker_path_context));
-	if (!c->context)
-		return 1;
-	((struct emc_clariion_checker_path_context *)c->context)->wwn_set = 0;
+	((struct emc_clariion_checker_path_context *)rdata->checker_ctx)->wwn_set = 0;
 
 	return 0;
 }
 
-void libcheck_free(struct checker *c)
-{
-	free(c->context);
-}
-
-int libcheck_check(struct checker *c, union checker_mpcontext *mpctx)
+int libcheck_async_func(struct runner_data *rdata)
 {
 	unsigned char sense_buffer[128] = { 0, };
 	unsigned char sb[SENSE_BUFF_LEN] = { 0, }, *sbb;
 	unsigned char inqCmdBlk[INQUIRY_CMDLEN] = {INQUIRY_CMD, 1, 0xC0, 0,
 						sizeof(sense_buffer), 0};
 	struct sg_io_hdr io_hdr;
-	struct emc_clariion_checker_path_context * ct =
-		(struct emc_clariion_checker_path_context *)c->context;
+	struct emc_clariion_checker_path_context *ct =
+		(struct emc_clariion_checker_path_context *)rdata->checker_ctx;
 	char wwnstr[33];
 	int ret;
 	int retry_emc = 5;
@@ -142,14 +129,14 @@ retry:
 	io_hdr.dxferp = sense_buffer;
 	io_hdr.cmdp = inqCmdBlk;
 	io_hdr.sbp = sb;
-	io_hdr.timeout = c->timeout * 1000;
+	io_hdr.timeout = rdata->timeout * 1000;
 	io_hdr.pack_id = 0;
-	if (ioctl(c->fd, SG_IO, &io_hdr) < 0) {
+	if (ioctl(rdata->fd, SG_IO, &io_hdr) < 0) {
 		if (errno == ENOTTY) {
-			c->msgid = CHECKER_MSGID_UNSUPPORTED;
+			rdata->msgid = CHECKER_MSGID_UNSUPPORTED;
 			return PATH_WILD;
 		}
-		c->msgid = MSG_CLARIION_QUERY_FAILED;
+		rdata->msgid = MSG_CLARIION_QUERY_FAILED;
 		return PATH_DOWN;
 	}
 
@@ -181,12 +168,12 @@ retry:
 				sense_key = sbp[2] & 0xf;
 
 			if (sense_key == ILLEGAL_REQUEST) {
-				c->msgid = CHECKER_MSGID_UNSUPPORTED;
+				rdata->msgid = CHECKER_MSGID_UNSUPPORTED;
 				return PATH_WILD;
 			} else if (sense_key != RECOVERED_ERROR) {
 				condlog(1, "emc_clariion_checker: INQUIRY failed with sense key %02x",
 					sense_key);
-				c->msgid = MSG_CLARIION_QUERY_ERROR;
+				rdata->msgid = MSG_CLARIION_QUERY_ERROR;
 				return PATH_DOWN;
 			}
 		}
@@ -195,13 +182,13 @@ retry:
 	if (io_hdr.info & SG_INFO_OK_MASK) {
 		condlog(1, "emc_clariion_checker: INQUIRY failed without sense, status %02x",
 			io_hdr.status);
-		c->msgid = MSG_CLARIION_QUERY_ERROR;
+		rdata->msgid = MSG_CLARIION_QUERY_ERROR;
 		return PATH_DOWN;
 	}
 
 	if (/* Verify the code page - right page & revision */
 	    sense_buffer[1] != 0xc0 || sense_buffer[9] != 0x00) {
-		c->msgid = MSG_CLARIION_UNIT_REPORT;
+		rdata->msgid = MSG_CLARIION_UNIT_REPORT;
 		return PATH_DOWN;
 	}
 
@@ -215,19 +202,19 @@ retry:
 		    ((sense_buffer[28] & 0x07) != 0x06))
 		/* Arraycommpath should be set to 1 */
 		|| (sense_buffer[30] & 0x04) != 0x04) {
-		c->msgid = MSG_CLARIION_PATH_CONFIG;
+		rdata->msgid = MSG_CLARIION_PATH_CONFIG;
 		return PATH_DOWN;
 	}
 
 	if ( /* LUN operations should indicate normal operations */
 		sense_buffer[48] != 0x00) {
-		c->msgid = MSG_CLARIION_PATH_NOT_AVAIL;
+		rdata->msgid = MSG_CLARIION_PATH_NOT_AVAIL;
 		return PATH_SHAKY;
 	}
 
 	if ( /* LUN should at least be bound somewhere and not be LUNZ */
 		sense_buffer[4] == 0x00) {
-		c->msgid = MSG_CLARIION_LUN_UNBOUND;
+		rdata->msgid = MSG_CLARIION_LUN_UNBOUND;
 		return PATH_DOWN;
 	}
 
@@ -238,7 +225,7 @@ retry:
 	 */
 	if (ct->wwn_set) {
 		if (memcmp(ct->wwn, &sense_buffer[10], 16) != 0) {
-			c->msgid = MSG_CLARIION_WWN_CHANGED;
+			rdata->msgid = MSG_CLARIION_WWN_CHANGED;
 			return PATH_DOWN;
 		}
 	} else {
@@ -253,8 +240,8 @@ retry:
 		unsigned char buf[4096];
 
 		memset(buf, 0, 4096);
-		ret = sg_read(c->fd, &buf[0], 4096,
-			      sbb = &sb[0], SENSE_BUFF_LEN, c->timeout);
+		ret = sg_read(rdata->fd, &buf[0], 4096, sbb = &sb[0],
+			      SENSE_BUFF_LEN, rdata->timeout);
 		if (ret == PATH_DOWN) {
 			hexadecimal_to_ascii(ct->wwn, wwnstr);
 
@@ -269,7 +256,7 @@ retry:
 				 * passive paths which will return
 				 * 02/04/03 not 05/25/01 on read.
 				 */
-				SET_INACTIVE_SNAP(mpctx);
+				SET_INACTIVE_SNAP(rdata->mpc);
 				condlog(3, "emc_clariion_checker: Active "
 					"path to inactive snapshot WWN %s.",
 					wwnstr);
@@ -278,26 +265,26 @@ retry:
 					"error for WWN %s.  Sense data are "
 					"0x%x/0x%x/0x%x.", wwnstr,
 					sbb[2]&0xf, sbb[12], sbb[13]);
-				c->msgid = MSG_CLARIION_READ_ERROR;
+				rdata->msgid = MSG_CLARIION_READ_ERROR;
 			}
 		} else {
-			c->msgid = MSG_CLARIION_PASSIVE_GOOD;
+			rdata->msgid = MSG_CLARIION_PASSIVE_GOOD;
 			/*
 			 * Remove the path from the set of paths to inactive
 			 * snapshot LUs if it was in this list since the
 			 * snapshot is no longer inactive.
 			 */
-			CLR_INACTIVE_SNAP(mpctx);
+			CLR_INACTIVE_SNAP(rdata->mpc);
 		}
 	} else {
-		if (IS_INACTIVE_SNAP(mpctx)) {
+		if (IS_INACTIVE_SNAP(rdata->mpc)) {
 			hexadecimal_to_ascii(ct->wwn, wwnstr);
 			condlog(3, "emc_clariion_checker: Passive "
 				"path to inactive snapshot WWN %s.",
 				wwnstr);
 			ret = PATH_DOWN;
 		} else {
-			c->msgid = MSG_CLARIION_PASSIVE_GOOD;
+			rdata->msgid = MSG_CLARIION_PASSIVE_GOOD;
 			ret = PATH_UP;	/* not ghost */
 		}
 	}
-- 
2.54.0

Re: [PATCH 05/18] multipath-tools tests: add tests for shared pointer code

[Martin Wilck]

On Tue, 2026-05-05 at 17:43 +0200, Martin Wilck wrote:
> Add a few tests (mostly meaningful with valgrind or ASAN) to test the
> shared pointer code.
> 
> Signed-off-by: Martin Wilck <mwilck@suse.com>

This test code needs rework. Don't review it yet.

Re: [PATCH 01/18] libmpathutil: runner: reduce a message loglevel

[Benjamin Marzinski]

On Tue, May 05, 2026 at 05:43:15PM +0200, Martin Wilck wrote:
> Don't "log runner 0x7b724a07a480 cancelled in state 'done'" at
> level 3. It's a fairly normal thing to happen.
> 
> Signed-off-by: Martin Wilck <mwilck@suse.com>

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

Re: [PATCH 02/18] libmultipath: checkers: add two generic checker messages

[Benjamin Marzinski]

On Tue, May 05, 2026 at 05:43:16PM +0200, Martin Wilck wrote:
> Add generic messages for "timed out" and "still running".
> They will be needed by follow-up patches.
> 
> Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>

Re: [PATCH 03/18] libmultipath: checkers: move checker_class definition to checkers.h

[Benjamin Marzinski]

On Tue, May 05, 2026 at 05:43:17PM +0200, Martin Wilck wrote:
> Make the contents of struct checker_class accessible to other code.
> 
> Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>

Re: [PATCH 04/18] libmpathutil: add implementation of generic shared pointer

[Benjamin Marzinski]

On Tue, May 05, 2026 at 05:43:18PM +0200, Martin Wilck wrote:
> Add a set of simple functions to handle refcounted pointers.
> 
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
> diff --git a/libmpathutil/util.c b/libmpathutil/util.c
> index 23a9797..3c623ec 100644
> --- a/libmpathutil/util.c
> +++ b/libmpathutil/util.c
[...]
> +void get_shared_ptr(void *ptr)
> +{
> +	struct shared_ptr *sp = container_of(ptr, struct shared_ptr, ptr);
> +
> +	if (uatomic_add_return(&sp->refcnt, 1) < 0)

This is fine, but what about using an unsigned refcnt, and checking if
this returns 1? It would catch the cases where wrap-around happened,
but it would also catch if something incremented a freed pointer. Just
a thought. But like I said, this is fine as is, so

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

> +		condlog(0, "%s: refcount overflow", __func__);
> +}

Re: [PATCH 06/18] libmultipath: use shared_ptr for checker classes

[Benjamin Marzinski]

On Tue, May 05, 2026 at 05:43:20PM +0200, Martin Wilck wrote:
> Utilize the share_ptr code for tracking the refcount of checker classes.
> 
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
>  libmultipath/checkers.c | 98 +++++++++--------------------------------
>  libmultipath/checkers.h |  2 -
>  2 files changed, 20 insertions(+), 80 deletions(-)
> 
> diff --git a/libmultipath/checkers.c b/libmultipath/checkers.c
> index 8f18ca6..a3b9cc8 100644
> --- a/libmultipath/checkers.c
> +++ b/libmultipath/checkers.c
[...]
> @@ -163,8 +143,7 @@ static struct checker_class *add_checker_class(const char *name)
>  		goto out;
>  
>  	c->mp_init = (int (*)(struct checker *)) dlsym(c->handle, "libcheck_mp_init");
> -	c->reset = (void (*)(void)) dlsym(c->handle, "libcheck_reset");
> -	c->thread = (void *(*)(void*)) dlsym(c->handle, "libcheck_thread");
> +	c->reset = (void (*)(void))dlsym(c->handle, "libcheck_reset");

Nitpick. Dropped a space here.

>  	c->pending = (int (*)(struct checker *)) dlsym(c->handle, "libcheck_pending");
>  	c->need_wait = (bool (*)(struct checker *)) dlsym(c->handle, "libcheck_need_wait");
>  	/* These 5 functions can be NULL. call dlerror() to clear out any
[...]
> @@ -371,43 +350,6 @@ bad_id:
>  	return generic_msg[CHECKER_MSGID_NONE];
>  }

Removing the checker thread stuff is more of a cleanup from d1ebd977. It
should probably either be it's own cleanup patch, or it should at least
get mentioned in the commit message.
  
> -static void checker_cleanup_thread(void *arg)
> -{
> -	struct checker_class *cls = arg;
> -
> -	free_checker_class(cls);
> -	rcu_unregister_thread();
> -}
> -
> -static void *checker_thread_entry(void *arg)
> -{
> -	struct checker_context *ctx = arg;
> -	void *rv;
> -
> -	rcu_register_thread();
> -	pthread_cleanup_push(checker_cleanup_thread, ctx->cls);
> -	rv = ctx->cls->thread(ctx);
> -	pthread_cleanup_pop(1);
> -	return rv;
> -}
> -

Also, start_checker_thread() is stil declared in libmultipath/checkers.h,
along with a comment.

-Ben

> -int start_checker_thread(pthread_t *thread, const pthread_attr_t *attr,
> -			 struct checker_context *ctx)
> -{
> -	int rv;
> -
> -	assert(ctx && ctx->cls && ctx->cls->thread);
> -	/* Take a ref here, lest the class be freed before the thread starts */
> -	(void)checker_class_ref(ctx->cls);
> -	rv = pthread_create(thread, attr, checker_thread_entry, ctx);
> -	if (rv != 0) {
> -		condlog(1, "failed to start checker thread for %s: %m",
> -			ctx->cls->name);
> -		checker_class_unref(ctx->cls);
> -	}
> -	return rv;
> -}
> -
>  void checker_clear_message (struct checker *c)
>  {
>  	if (!c)

Re: [PATCH 07/18] libmultipath: use shared_ptr for prioritizers

[Benjamin Marzinski]

On Tue, May 05, 2026 at 05:43:21PM +0200, Martin Wilck wrote:
> Replace the hardcoded refcount handling in prio.c with shared_ptr.
> 
> Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>

Re: [PATCH 08/18] libmpathutil: runner: use shared_ptr

[Benjamin Marzinski]

On Tue, May 05, 2026 at 05:43:22PM +0200, Martin Wilck wrote:
> Use shared_ptr to track the runner_context refcount.
> 
> Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>

Re: [PATCH 09/18] libmultipath: add generic async path checker code

[Benjamin Marzinski]

On Tue, May 05, 2026 at 05:43:23PM +0200, Martin Wilck wrote:
> Add a framework for an asynchronous path checker utilizing the
> recently added runner code for thread handling.
> 
> This code has been derived from the TUR checker code by removing
> all references to the actual sending of TUR ioctls.
> 
> Follow-up patches will convert the current TUR checker into an
> instance of this async checker class.
> 
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
>  libmultipath/Makefile        |   2 +-
>  libmultipath/async_checker.c | 216 +++++++++++++++++++++++++++++++++++
>  libmultipath/async_checker.h |  35 ++++++
>  libmultipath/checkers.c      |   1 +
>  libmultipath/checkers.h      |   2 +
>  5 files changed, 255 insertions(+), 1 deletion(-)
>  create mode 100644 libmultipath/async_checker.c
>  create mode 100644 libmultipath/async_checker.h
> 
[...]
> diff --git a/libmultipath/async_checker.h b/libmultipath/async_checker.h
> new file mode 100644
> index 0000000..835de2a
> --- /dev/null
> +++ b/libmultipath/async_checker.h
> @@ -0,0 +1,35 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +// Copyright (c) 2026 SUSE LLC
> +#ifndef ASYNC_CHECKER_H_INCLUDED
> +#define ASYNC_CHECKER_H_INCLUDED
> +
> +struct runner_data;
> +struct checker;
> +typedef int (*async_checker_func)(struct runner_data *);
> +
> +struct runner_data {
> +	int fd;
> +	dev_t devt;
> +	async_checker_func afunc;
> +	unsigned int timeout;
> +	int state;
> +	short msgid;
> +	char checker_ctx[];

checker_ctx should be probably be aligned.

-Ben

> +};
> +
> +int async_check_init(struct checker *c);
> +void async_check_free(struct checker *c);
> +bool async_check_need_wait(struct checker *c);
> +int async_check_pending(struct checker *c);
> +int async_check_check(struct checker *c);
> +
> +#define CHECKER_MAX_CONTEXT_SIZE 1024
> +
> +/* For testing handling of async checker timeouts */
> +#ifdef ASYNC_TEST_MAJOR
> +static void async_deep_sleep(const struct runner_data *rdata);
> +#else
> +#define async_deep_sleep(x) do {} while (0)
> +#endif
> +
> +#endif
> diff --git a/libmultipath/checkers.c b/libmultipath/checkers.c
> index a3b9cc8..3a1663f 100644
> --- a/libmultipath/checkers.c
> +++ b/libmultipath/checkers.c
> @@ -9,6 +9,7 @@
>  
>  #include "debug.h"
>  #include "checkers.h"
> +#include "async_checker.h"
>  #include "vector.h"
>  #include "util.h"
>  
> diff --git a/libmultipath/checkers.h b/libmultipath/checkers.h
> index 630e987..744be54 100644
> --- a/libmultipath/checkers.h
> +++ b/libmultipath/checkers.h
> @@ -132,6 +132,7 @@ enum {
>  };
>  
>  struct checker;
> +struct runner_data;
>  struct checker_class {
>  	struct list_head node;
>  	void *handle;
> @@ -144,6 +145,7 @@ struct checker_class {
>  	void (*reset)(void);		  /* to reset the global variables */
>  	int (*pending)(struct checker *); /* to recheck pending paths */
>  	bool (*need_wait)(struct checker *); /* checker needs waiting for */
> +	int (*async_func)(struct runner_data *); /* callback for async_checker */
>  	const char **msgtable;
>  	short msgtable_size;
>  };
> -- 
> 2.54.0

Re: [PATCH 10/18] libmultipath: checkers: add support for async checker

[Benjamin Marzinski]

On Tue, May 05, 2026 at 05:43:24PM +0200, Martin Wilck wrote:
> If a checker shared library contains a symbol "libcheck_async_func",
> use this as a callback for the async checker functions, and do not
> read any other symbols from the .so file except the message table.
> 
> Checkers can provide a symbol async_context_size that will cause
> the async_checker code to reserve extra space in runner_data for
> the checker to use, and a symbol async_init() for initializing
> this private context.
> 
> Only emc_clariion will use the latter functionality.
> 
> Signed-off-by: Martin Wilck <mwilck@suse.com>
async_context_size and async_init() don't exist yet, but otherwise
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>

Re: [PATCH 11/18] libmultipath: convert TUR checker to an async checker instance

[Benjamin Marzinski]

On Tue, May 05, 2026 at 05:43:25PM +0200, Martin Wilck wrote:
> The TUR checker now just exports the message table and the
> "libcheck_async_func" symbol. This converts it into an instance of
> the generic async checker model.
> 
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
>  libmultipath/checkers/tur.c | 251 +++---------------------------------
>  1 file changed, 15 insertions(+), 236 deletions(-)
> 
> diff --git a/libmultipath/checkers/tur.c b/libmultipath/checkers/tur.c
> index 6791172..757767a 100644
> --- a/libmultipath/checkers/tur.c
> +++ b/libmultipath/checkers/tur.c
> @@ -3,87 +3,31 @@
>   *
>   * Copyright (c) 2004 Christophe Varoqui
>   */
> -#include <stdlib.h>
> -#include <string.h>
> -#include <sys/types.h>
> -#include <sys/stat.h>
> -#include <unistd.h>
> -#include <fcntl.h>
>  #include <sys/ioctl.h>
> -#include <sys/sysmacros.h>
> +#include <sys/types.h>
> +
>  #include <errno.h>
> -#include <sys/time.h>
>  
>  #include "checkers.h"
> -#include "debug.h"
> +#include "async_checker.h"
>  #include "sg_include.h"
> -#include "runner.h"
>  
>  #define TUR_CMD_LEN 6
>  #define HEAVY_CHECK_COUNT       10
> -#define MAX_NR_TIMEOUTS 1
>  
>  enum {
> -	MSG_TUR_RUNNING = CHECKER_FIRST_MSGID,
> -	MSG_TUR_TIMEOUT,
> -	MSG_TUR_FAILED,

MSG_TUR_FAILED isn't actually used anymore, and can get removed.
Otherwise this looks good.

-Ben

Re: [PATCH 12/18] libmultipath: convert RDAC checker to async checker

[Benjamin Marzinski]

On Tue, May 05, 2026 at 05:43:26PM +0200, Martin Wilck wrote:
> Make the RDAC checker use the async checker framework.
> 
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
>  libmultipath/checkers/rdac.c | 30 ++++++++++--------------------
>  1 file changed, 10 insertions(+), 20 deletions(-)
> 
> diff --git a/libmultipath/checkers/rdac.c b/libmultipath/checkers/rdac.c
> index 87b8872..a477fb5 100644
> --- a/libmultipath/checkers/rdac.c
> +++ b/libmultipath/checkers/rdac.c
> @@ -1,8 +1,6 @@
>  /*
>   * Copyright (c) 2005 Christophe Varoqui
>   */
> -#include <stdio.h>
> -#include <stdlib.h>
>  #include <string.h>
>  #include <sys/types.h>
>  #include <sys/stat.h>
> @@ -15,6 +13,7 @@
>  #include "debug.h"
>  
>  #include "sg_include.h"
> +#include "async_checker.h"
>  
>  #define INQUIRY_CMDLEN		6
>  #define INQUIRY_CMD		0x12
> @@ -55,11 +54,7 @@ struct control_mode_page {
>  	unsigned char dontcare1[6];
>  };
>  
> -struct rdac_checker_context {
> -	void * dummy;
> -};
> -
> -int libcheck_init (struct checker * c)
> +int libcheck_init(struct checker *c)

This function doesn't get called anymore. Do we not need to set the
TAS bit for some reason?

-Ben

>  {
>  	unsigned char cmd[MODE_SEN_SEL_CMDLEN];
>  	unsigned char sense_b[SENSE_BUFF_LEN];

Re: [PATCH 13/18] libmultipath: convert cciss_tur checker to async checker

[Benjamin Marzinski]

On Tue, May 05, 2026 at 05:43:27PM +0200, Martin Wilck wrote:
> Make the cciss_tur checker use the async checker framework.
> 
> Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>

Re: [PATCH 14/18] libmultipath: convert readsector0 checker to async checker

[Benjamin Marzinski]

On Tue, May 05, 2026 at 05:43:28PM +0200, Martin Wilck wrote:
> Make the readsector0 checker use the async checker framework.
> 
> Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>

Re: [PATCH 15/18] libmultipath: convert hp_sw checker to async checker

[Benjamin Marzinski]

On Tue, May 05, 2026 at 05:43:29PM +0200, Martin Wilck wrote:
> Make the hp_sw checker use the async checker framework.
> 
> Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>

Re: [PATCH 16/18] libmultipath: async_checker: add context_size and init symbols

[Benjamin Marzinski]

On Tue, May 05, 2026 at 05:43:30PM +0200, Martin Wilck wrote:
> Add two symbols that can be read from checker .so files with dlsym():
> "async_context_size" for defining the size of a checker-private context,
> and "async_init" for initializing this context.

Why not "libcheck_async_context_size" and "libcheck_async_init", to
go with "libcheck_async_func"? Otherwise this looks good.

-Ben

Re: [PATCH 17/18] libmultipath: checkers: rework mpcontext passing

[Benjamin Marzinski]

On Tue, May 05, 2026 at 05:43:31PM +0200, Martin Wilck wrote:
> The emc_clariion path checker requires an "mpcontext" that is shared
> between the checkers for all paths of a given multipath map. This context
> is currently represented in struct multipath as a void *, the pointer to
> which is passed to the checker. The checker allocates the actual memory in
> the libcheck_mp_init() function, and changes the pointer in struct
> multipath. This is a dangerous layering violation as the checker operates
> on memory it doesn't own, and not type-safe at all.
> 
> This patch changes the mpcontext handling as follows: The void * pointer
> is replaced by a dedicated union checker_mpcontext. It only contains
> a long integer element, which is sufficient to store the information
> needed by emc_clariion.
> 
> Instead of initializing this value in mp_init, the address of the union is
> passed to libcheck_check() and libcheck_pending() if the path that is being
> checked has an associated struct multipath. Passing a union pointer
> improves compile-time type safety. In order to check whether the value is
> initialized, instead of testing for a NULL pointer like before, the code
> defines an INVALID_MPCONTEXT value. This value must obviously be chosen
> such that it doesn't represent a valid context value, which is the case for
> emc_clariion.
> 
> For a synchronous checker (like emc_clariion), libcheck_check() is allowed
> to write to this memory, because at the time of the call we know that
> pp->mpp is valid. The same holds for libcheck_pending().
> 
> This change requires checker_check() and checker_get_state() to be passed
> a "struct path *" rather than a "struct checker *". The layer separation
> happens in these functions now.
> 
> The async_checker code also gets an implementation of mpcontext handling,
> in preparation of converting emc_clariion to an async checker.
> 
> Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>

Re: [PATCH 18/18] libmultipath: convert emc_clariion to async_checker

[Benjamin Marzinski]

On Tue, May 05, 2026 at 05:43:32PM +0200, Martin Wilck wrote:
> ---
>  libmultipath/checkers/emc_clariion.c | 79 ++++++++++++----------------
>  1 file changed, 33 insertions(+), 46 deletions(-)
> 

No Signed-off-by. Also, I don't think it really matters much, but
running this checker asynchronously means that the checkers will not
have their mpcontext updated by previous checkers run in the same
checker loop, like they did when the checker was synchronous. If an
earlier path check detects that a snapshot LU is no longer inactive, a
later call to a passive path in the same checker loop will still return
PATH_DOWN instead of PATH_UP, since the mpcontext won't be updated until
update_paths() gets called. Obviously, this will get fixed the next time
the path is checked, so I think it's fine.

-Ben

Re: [PATCH 06/18] libmultipath: use shared_ptr for checker classes

[Martin Wilck]

On Tue, 2026-05-12 at 20:49 -0400, Benjamin Marzinski wrote:
> On Tue, May 05, 2026 at 05:43:20PM +0200, Martin Wilck wrote:
> > Utilize the share_ptr code for tracking the refcount of checker
> > classes.
> > 
> > Signed-off-by: Martin Wilck <mwilck@suse.com>
> > ---
> >  libmultipath/checkers.c | 98 +++++++++----------------------------
> > ----
> >  libmultipath/checkers.h |  2 -
> >  2 files changed, 20 insertions(+), 80 deletions(-)
> > 
> > diff --git a/libmultipath/checkers.c b/libmultipath/checkers.c
> > index 8f18ca6..a3b9cc8 100644
> > --- a/libmultipath/checkers.c
> > +++ b/libmultipath/checkers.c
> [...]
> > @@ -163,8 +143,7 @@ static struct checker_class
> > *add_checker_class(const char *name)
> >  		goto out;
> >  
> >  	c->mp_init = (int (*)(struct checker *)) dlsym(c->handle,
> > "libcheck_mp_init");
> > -	c->reset = (void (*)(void)) dlsym(c->handle,
> > "libcheck_reset");
> > -	c->thread = (void *(*)(void*)) dlsym(c->handle,
> > "libcheck_thread");
> > +	c->reset = (void (*)(void))dlsym(c->handle,
> > "libcheck_reset");
> 
> Nitpick. Dropped a space here.

I didn't do that. clang-format did, as it also reformats context lines
in patches. I think clang-format is correct. We usually don't use space
after cast, do we? (As usual, our code base is inconsistent in this
respect).

While I don't want to enforce formatting style throughout the entire
code base, I tend to keep the changes that clang-format applies to
context lines (be it only to avoid red lights from the GitHub CI).

Do you disagree with this policy?

Regards
Martin

> 
> >  	c->pending = (int (*)(struct checker *)) dlsym(c->handle,
> > "libcheck_pending");
> >  	c->need_wait = (bool (*)(struct checker *)) dlsym(c-
> > >handle, "libcheck_need_wait");
> >  	/* These 5 functions can be NULL. call dlerror() to clear
> > out any
> [...]
> > @@ -371,43 +350,6 @@ bad_id:
> >  	return generic_msg[CHECKER_MSGID_NONE];
> >  }
> 
> Removing the checker thread stuff is more of a cleanup from d1ebd977.
> It
> should probably either be it's own cleanup patch, or it should at
> least
> get mentioned in the commit message.
>   
> > -static void checker_cleanup_thread(void *arg)
> > -{
> > -	struct checker_class *cls = arg;
> > -
> > -	free_checker_class(cls);
> > -	rcu_unregister_thread();
> > -}
> > -
> > -static void *checker_thread_entry(void *arg)
> > -{
> > -	struct checker_context *ctx = arg;
> > -	void *rv;
> > -
> > -	rcu_register_thread();
> > -	pthread_cleanup_push(checker_cleanup_thread, ctx->cls);
> > -	rv = ctx->cls->thread(ctx);
> > -	pthread_cleanup_pop(1);
> > -	return rv;
> > -}
> > -
> 
> Also, start_checker_thread() is stil declared in
> libmultipath/checkers.h,
> along with a comment.
> 
> -Ben
> 
> > -int start_checker_thread(pthread_t *thread, const pthread_attr_t
> > *attr,
> > -			 struct checker_context *ctx)
> > -{
> > -	int rv;
> > -
> > -	assert(ctx && ctx->cls && ctx->cls->thread);
> > -	/* Take a ref here, lest the class be freed before the
> > thread starts */
> > -	(void)checker_class_ref(ctx->cls);
> > -	rv = pthread_create(thread, attr, checker_thread_entry,
> > ctx);
> > -	if (rv != 0) {
> > -		condlog(1, "failed to start checker thread for %s:
> > %m",
> > -			ctx->cls->name);
> > -		checker_class_unref(ctx->cls);
> > -	}
> > -	return rv;
> > -}
> > -
> >  void checker_clear_message (struct checker *c)
> >  {
> >  	if (!c)

Re: [PATCH 06/18] libmultipath: use shared_ptr for checker classes

[Benjamin Marzinski]

On Sun, May 17, 2026 at 11:57:25AM +0200, Martin Wilck wrote:
> On Tue, 2026-05-12 at 20:49 -0400, Benjamin Marzinski wrote:
> > On Tue, May 05, 2026 at 05:43:20PM +0200, Martin Wilck wrote:
> > > Utilize the share_ptr code for tracking the refcount of checker
> > > classes.
> > > 
> > > Signed-off-by: Martin Wilck <mwilck@suse.com>
> > > ---
> > >  libmultipath/checkers.c | 98 +++++++++----------------------------
> > > ----
> > >  libmultipath/checkers.h |  2 -
> > >  2 files changed, 20 insertions(+), 80 deletions(-)
> > > 
> > > diff --git a/libmultipath/checkers.c b/libmultipath/checkers.c
> > > index 8f18ca6..a3b9cc8 100644
> > > --- a/libmultipath/checkers.c
> > > +++ b/libmultipath/checkers.c
> > [...]
> > > @@ -163,8 +143,7 @@ static struct checker_class
> > > *add_checker_class(const char *name)
> > >  		goto out;
> > >  
> > >  	c->mp_init = (int (*)(struct checker *)) dlsym(c->handle,
> > > "libcheck_mp_init");
> > > -	c->reset = (void (*)(void)) dlsym(c->handle,
> > > "libcheck_reset");
> > > -	c->thread = (void *(*)(void*)) dlsym(c->handle,
> > > "libcheck_thread");
> > > +	c->reset = (void (*)(void))dlsym(c->handle,
> > > "libcheck_reset");
> > 
> > Nitpick. Dropped a space here.
> 
> I didn't do that. clang-format did, as it also reformats context lines
> in patches. I think clang-format is correct. We usually don't use space
> after cast, do we? (As usual, our code base is inconsistent in this
> respect).
> 
> While I don't want to enforce formatting style throughout the entire
> code base, I tend to keep the changes that clang-format applies to
> context lines (be it only to avoid red lights from the GitHub CI).
> 
> Do you disagree with this policy?

It's fine. It just doesn't match the dlsym lines around it, which is fine.
-Ben

> 
> Regards
> Martin
> 
> > 
> > >  	c->pending = (int (*)(struct checker *)) dlsym(c->handle,
> > > "libcheck_pending");
> > >  	c->need_wait = (bool (*)(struct checker *)) dlsym(c-
> > > >handle, "libcheck_need_wait");
> > >  	/* These 5 functions can be NULL. call dlerror() to clear
> > > out any
> > [...]
> > > @@ -371,43 +350,6 @@ bad_id:
> > >  	return generic_msg[CHECKER_MSGID_NONE];
> > >  }
> > 
> > Removing the checker thread stuff is more of a cleanup from d1ebd977.
> > It
> > should probably either be it's own cleanup patch, or it should at
> > least
> > get mentioned in the commit message.
> >   
> > > -static void checker_cleanup_thread(void *arg)
> > > -{
> > > -	struct checker_class *cls = arg;
> > > -
> > > -	free_checker_class(cls);
> > > -	rcu_unregister_thread();
> > > -}
> > > -
> > > -static void *checker_thread_entry(void *arg)
> > > -{
> > > -	struct checker_context *ctx = arg;
> > > -	void *rv;
> > > -
> > > -	rcu_register_thread();
> > > -	pthread_cleanup_push(checker_cleanup_thread, ctx->cls);
> > > -	rv = ctx->cls->thread(ctx);
> > > -	pthread_cleanup_pop(1);
> > > -	return rv;
> > > -}
> > > -
> > 
> > Also, start_checker_thread() is stil declared in
> > libmultipath/checkers.h,
> > along with a comment.
> > 
> > -Ben
> > 
> > > -int start_checker_thread(pthread_t *thread, const pthread_attr_t
> > > *attr,
> > > -			 struct checker_context *ctx)
> > > -{
> > > -	int rv;
> > > -
> > > -	assert(ctx && ctx->cls && ctx->cls->thread);
> > > -	/* Take a ref here, lest the class be freed before the
> > > thread starts */
> > > -	(void)checker_class_ref(ctx->cls);
> > > -	rv = pthread_create(thread, attr, checker_thread_entry,
> > > ctx);
> > > -	if (rv != 0) {
> > > -		condlog(1, "failed to start checker thread for %s:
> > > %m",
> > > -			ctx->cls->name);
> > > -		checker_class_unref(ctx->cls);
> > > -	}
> > > -	return rv;
> > > -}
> > > -
> > >  void checker_clear_message (struct checker *c)
> > >  {
> > >  	if (!c)