[PATCH 11/12] multipathd: add new polling dmevents waiter thread

From: "Benjamin Marzinski" <bmarzins@redhat.com>
To: dm-devel@redhat.com
Cc: Martin Wilck <mwilck@suse.com>, christophe.varoqui@free.fr
Subject: [PATCH 11/12] multipathd: add new polling dmevents waiter thread
Date: Wed, 14 Mar 2018 12:46:44 -0500	[thread overview]
Message-ID: <1521049605-22050-12-git-send-email-bmarzins@redhat.com> (raw)
In-Reply-To: <1521049605-22050-1-git-send-email-bmarzins@redhat.com>

The current method of waiting for dmevents on multipath devices involves
creating a seperate thread for each device. This can become very
wasteful when there are large numbers of multipath devices. Also, since
multipathd needs to grab the vecs lock to update the devices, the
additional threads don't actually provide much parallelism.

The patch adds a new method of updating multipath devices on dmevents,
which uses the new device-mapper event polling interface. This means
that there is only one dmevent waiting thread which will wait for events
on all of the multipath devices.  Currently the code to get the event
number from the list of device names and to re-arm the polling interface
is not in libdevmapper, so the patch does that work. Obviously, these
bits need to go into libdevmapper, so that multipathd can use a standard
interface.

I haven't touched any of the existing event waiting code, since event
polling was only added to device-mapper in version 4.37.0.  multipathd
checks this version, and defaults to using the polling code if
device-mapper supports it. This can be overridden by running multipathd
with "-w", to force it to use the old event waiting code.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 Makefile.inc               |   3 +
 libmultipath/structs_vec.c |   8 +
 libmultipath/structs_vec.h |   2 +
 multipathd/Makefile        |   6 +-
 multipathd/dmevents.c      | 392 +++++++++++++++++++++++++++++++++++++++++++++
 multipathd/dmevents.h      |  13 ++
 multipathd/main.c          |  62 ++++++-
 7 files changed, 477 insertions(+), 9 deletions(-)
 create mode 100644 multipathd/dmevents.c
 create mode 100644 multipathd/dmevents.h

diff --git a/Makefile.inc b/Makefile.inc
index 5d6123d..57a1835 100644
--- a/Makefile.inc
+++ b/Makefile.inc
@@ -14,6 +14,9 @@
 #
 # Uncomment to disable libdmmp support
 # ENABLE_LIBDMMP = 0
+#
+# Uncomment to disable dmevents polling support
+# ENABLE_DMEVENTS_POLL = 0
 
 ifeq ($(TOPDIR),)
 	TOPDIR	= ..
diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
index 9a01631..e9a0274 100644
--- a/libmultipath/structs_vec.c
+++ b/libmultipath/structs_vec.c
@@ -130,6 +130,14 @@ remove_map(struct multipath * mpp, struct vectors * vecs, int purge_vec)
 }
 
 void
+remove_map_by_alias(const char *alias, struct vectors * vecs, int purge_vec)
+{
+	struct multipath * mpp = find_mp_by_alias(vecs->mpvec, alias);
+	if (mpp)
+		remove_map(mpp, vecs, purge_vec);
+}
+
+void
 remove_maps(struct vectors * vecs)
 {
 	int i;
diff --git a/libmultipath/structs_vec.h b/libmultipath/structs_vec.h
index ceab6d9..0adba17 100644
--- a/libmultipath/structs_vec.h
+++ b/libmultipath/structs_vec.h
@@ -29,6 +29,8 @@ void extract_hwe_from_path(struct multipath * mpp);
 #define PURGE_VEC 1
 
 void remove_map (struct multipath * mpp, struct vectors * vecs, int purge_vec);
+void remove_map_by_alias(const char *alias, struct vectors * vecs,
+			 int purge_vec);
 void remove_maps (struct vectors * vecs);
 
 void sync_map_state (struct multipath *);
diff --git a/multipathd/Makefile b/multipathd/Makefile
index 7800e47..d1a9863 100644
--- a/multipathd/Makefile
+++ b/multipathd/Makefile
@@ -21,8 +21,12 @@ ifdef SYSTEMD
 		LIBDEPS += -lsystemd-daemon
 	endif
 endif
+ifeq ($(ENABLE_DMEVENTS_POLL),0)
+	CFLAGS += -DNO_DMEVENTS_POLL
+endif
 
-OBJS = main.o pidfile.o uxlsnr.o uxclnt.o cli.o cli_handlers.o waiter.o
+OBJS = main.o pidfile.o uxlsnr.o uxclnt.o cli.o cli_handlers.o waiter.o \
+       dmevents.o
 
 EXEC = multipathd
 
diff --git a/multipathd/dmevents.c b/multipathd/dmevents.c
new file mode 100644
index 0000000..1ef811e
--- /dev/null
+++ b/multipathd/dmevents.c
@@ -0,0 +1,392 @@
+/*
+ * Copyright (c) 2004, 2005 Christophe Varoqui
+ * Copyright (c) 2005 Kiyoshi Ueda, NEC
+ * Copyright (c) 2005 Edward Goggin, EMC
+ * Copyright (c) 2005, 2018 Benjamin Marzinski, Redhat
+ */
+#include <unistd.h>
+#include <libdevmapper.h>
+#include <sys/mman.h>
+#include <pthread.h>
+#include <urcu.h>
+#include <poll.h>
+#include <sys/ioctl.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <linux/dm-ioctl.h>
+#include <errno.h>
+
+#include "vector.h"
+#include "structs.h"
+#include "structs_vec.h"
+#include "devmapper.h"
+#include "debug.h"
+#include "dmevents.h"
+
+#ifndef DM_DEV_ARM_POLL
+#define DM_DEV_ARM_POLL _IOWR(DM_IOCTL, DM_DEV_SET_GEOMETRY_CMD + 1, struct dm_ioctl)
+#endif
+
+enum event_actions {
+	EVENT_NOTHING,
+	EVENT_REMOVE,
+	EVENT_UPDATE,
+};
+
+struct dev_event {
+	char name[WWID_SIZE];
+	uint32_t evt_nr;
+	enum event_actions action;
+};
+
+struct dmevent_waiter {
+	int fd;
+	struct vectors *vecs;
+	vector events;
+	pthread_mutex_t events_lock;
+};
+
+static struct dmevent_waiter *waiter;
+
+int dmevent_poll_supported(void)
+{
+	unsigned int minv[3] = {4, 37, 0};
+	unsigned int v[3];
+
+	if (dm_drv_version(v))
+		return 0;
+
+	if (VERSION_GE(v, minv))
+		return 1;
+	return 0;
+}
+
+
+int init_dmevent_waiter(struct vectors *vecs)
+{
+	if (!vecs) {
+		condlog(0, "can't create waiter structure. invalid vectors");
+		goto fail;
+	}
+	waiter = (struct dmevent_waiter *)malloc(sizeof(struct dmevent_waiter));
+	if (!waiter) {
+		condlog(0, "failed to allocate waiter structure");
+		goto fail;
+	}
+	memset(waiter, 0, sizeof(struct dmevent_waiter));
+	waiter->events = vector_alloc();
+	if (!waiter->events) {
+		condlog(0, "failed to allocate waiter events vector");
+		goto fail_waiter;
+	}
+	waiter->fd = open("/dev/mapper/control", O_RDWR);
+	if (waiter->fd < 0) {
+		condlog(0, "failed to open /dev/mapper/control for waiter");
+		goto fail_events;
+	}
+	pthread_mutex_init(&waiter->events_lock, NULL);
+	waiter->vecs = vecs;
+
+	return 0;
+fail_events:
+	vector_free(waiter->events);
+fail_waiter:
+	free(waiter);
+fail:
+	waiter = NULL;
+	return -1;
+}
+
+void cleanup_dmevent_waiter(void)
+{
+	struct dev_event *dev_evt;
+	int i;
+
+	if (!waiter)
+		return;
+	pthread_mutex_destroy(&waiter->events_lock);
+	close(waiter->fd);
+	vector_foreach_slot(waiter->events, dev_evt, i)
+		free(dev_evt);
+	vector_free(waiter->events);
+	free(waiter);
+	waiter = NULL;
+}
+
+static int arm_dm_event_poll(int fd)
+{
+	struct dm_ioctl dmi;
+	memset(&dmi, 0, sizeof(dmi));
+	dmi.version[0] = DM_VERSION_MAJOR;
+	dmi.version[1] = DM_VERSION_MINOR;
+	dmi.version[2] = DM_VERSION_PATCHLEVEL;
+	dmi.flags = 0x4;
+	dmi.data_start = offsetof(struct dm_ioctl, data);
+	dmi.data_size = sizeof(dmi);
+	return ioctl(fd, DM_DEV_ARM_POLL, &dmi);
+}
+
+/*
+ * As of version 4.37.0 device-mapper stores the event number in the
+ * dm_names structure after the name, when DM_DEVICE_LIST is called
+ */
+static uint32_t dm_event_nr(struct dm_names *n)
+{
+	return *(uint32_t *)(((uintptr_t)(strchr(n->name, 0) + 1) + 7) & ~7);
+}
+
+static int dm_get_events(void)
+{
+	struct dm_task *dmt;
+	struct dm_names *names;
+	struct dev_event *dev_evt;
+	int i;
+
+	if (!(dmt = libmp_dm_task_create(DM_DEVICE_LIST)))
+		return -1;
+
+	dm_task_no_open_count(dmt);
+
+	if (!dm_task_run(dmt))
+		goto fail;
+
+	if (!(names = dm_task_get_names(dmt)))
+		goto fail;
+
+	pthread_mutex_lock(&waiter->events_lock);
+	vector_foreach_slot(waiter->events, dev_evt, i)
+		dev_evt->action = EVENT_REMOVE;
+	while (names->dev) {
+		uint32_t event_nr;
+
+		if (!dm_is_mpath(names->name))
+			goto next;
+
+		event_nr = dm_event_nr(names);
+		vector_foreach_slot(waiter->events, dev_evt, i) {
+			if (!strcmp(dev_evt->name, names->name)) {
+				if (event_nr != dev_evt->evt_nr) {
+					dev_evt->evt_nr = event_nr;
+					dev_evt->action = EVENT_UPDATE;
+				} else
+					dev_evt->action = EVENT_NOTHING;
+				break;
+			}
+		}
+next:
+		if (!names->next)
+			break;
+		names = (void *)names + names->next;
+	}
+	pthread_mutex_unlock(&waiter->events_lock);
+	dm_task_destroy(dmt);
+	return 0;
+
+fail:
+	dm_task_destroy(dmt);
+	return -1;
+}
+
+/* You must call update_multipath() after calling this function, to
+ * deal with any events that came in before the device was added */
+int watch_dmevents(char *name)
+{
+	int event_nr;
+	struct dev_event *dev_evt, *old_dev_evt;
+	int i;
+
+	if (!dm_is_mpath(name)) {
+		condlog(0, "%s: not a multipath device. can't watch events",
+			name);
+		return -1;
+	}
+
+	if ((event_nr = dm_geteventnr(name)) < 0)
+		return -1;
+
+	dev_evt = (struct dev_event *)malloc(sizeof(struct dev_event));
+	if (!dev_evt) {
+		condlog(0, "%s: can't allocate event waiter structure", name);
+		return -1;
+	}
+
+	strncpy(dev_evt->name, name, WWID_SIZE);
+	dev_evt->name[WWID_SIZE - 1] = 0;
+	dev_evt->evt_nr = event_nr;
+	dev_evt->action = EVENT_NOTHING;
+
+	pthread_mutex_lock(&waiter->events_lock);
+	vector_foreach_slot(waiter->events, old_dev_evt, i){
+		if (!strcmp(dev_evt->name, old_dev_evt->name)) {
+			/* caller will be updating this device */
+			old_dev_evt->evt_nr = event_nr;
+			old_dev_evt->action = EVENT_NOTHING;
+			pthread_mutex_unlock(&waiter->events_lock);
+			condlog(2, "%s: already waiting for events on device",
+				name);
+			free(dev_evt);
+			return 0;
+		}
+	}
+	if (!vector_alloc_slot(waiter->events)) {
+		pthread_mutex_unlock(&waiter->events_lock);
+		free(dev_evt);
+		return -1;
+	}
+	vector_set_slot(waiter->events, dev_evt);
+	pthread_mutex_unlock(&waiter->events_lock);
+	return 0;
+}
+
+void unwatch_all_dmevents(void)
+{
+	struct dev_event *dev_evt;
+	int i;
+
+	pthread_mutex_lock(&waiter->events_lock);
+	vector_foreach_slot(waiter->events, dev_evt, i)
+		free(dev_evt);
+	vector_reset(waiter->events);
+	pthread_mutex_unlock(&waiter->events_lock);
+}
+
+static void unwatch_dmevents(char *name)
+{
+	struct dev_event *dev_evt;
+	int i;
+
+	pthread_mutex_lock(&waiter->events_lock);
+	vector_foreach_slot(waiter->events, dev_evt, i) {
+		if (!strcmp(dev_evt->name, name)) {
+			vector_del_slot(waiter->events, i);
+			free(dev_evt);
+			break;
+		}
+	}
+	pthread_mutex_unlock(&waiter->events_lock);
+}
+
+/*
+ * returns the reschedule delay
+ * negative means *stop*
+ */
+
+/* poll, arm, update, return */
+static int dmevent_loop (void)
+{
+	int r, i = 0;
+	struct pollfd pfd;
+	struct dev_event *dev_evt;
+
+	pfd.fd = waiter->fd;
+	pfd.events = POLLIN;
+	r = poll(&pfd, 1, -1);
+	if (r <= 0) {
+		condlog(0, "failed polling for dm events: %s", strerror(errno));
+		/* sleep 1s and hope things get better */
+		return 1;
+	}
+
+	if (arm_dm_event_poll(waiter->fd) != 0) {
+		condlog(0, "Cannot re-arm event polling: %s", strerror(errno));
+		/* sleep 1s and hope things get better */
+		return 1;
+	}
+
+	if (dm_get_events() != 0) {
+		condlog(0, "failed getting dm events: %s", strerror(errno));
+		/* sleep 1s and hope things get better */
+		return 1;
+	}
+
+	/*
+	 * upon event ...
+	 */
+
+	while (1) {
+		int done = 1;
+		struct dev_event curr_dev;
+
+		pthread_mutex_lock(&waiter->events_lock);
+		vector_foreach_slot(waiter->events, dev_evt, i) {
+			if (dev_evt->action != EVENT_NOTHING) {
+				curr_dev = *dev_evt;
+				if (dev_evt->action == EVENT_REMOVE) {
+					vector_del_slot(waiter->events, i);
+					free(dev_evt);
+				} else
+					dev_evt->action = EVENT_NOTHING;
+				done = 0;
+				break;
+			}
+		}
+		pthread_mutex_unlock(&waiter->events_lock);
+		if (done)
+			return 1;
+
+		condlog(3, "%s: devmap event #%i", curr_dev.name,
+			curr_dev.evt_nr);
+
+		/*
+		 * event might be :
+		 *
+		 * 1) a table reload, which means our mpp structure is
+		 *    obsolete : refresh it through update_multipath()
+		 * 2) a path failed by DM : mark as such through
+		 *    update_multipath()
+		 * 3) map has gone away : stop the thread.
+		 * 4) a path reinstate : nothing to do
+		 * 5) a switch group : nothing to do
+		 */
+		pthread_cleanup_push(cleanup_lock, &waiter->vecs->lock);
+		lock(&waiter->vecs->lock);
+		pthread_testcancel();
+		r = 0;
+		if (curr_dev.action == EVENT_REMOVE)
+			remove_map_by_alias(curr_dev.name, waiter->vecs, 1);
+		else
+			r = update_multipath(waiter->vecs, curr_dev.name, 1);
+		lock_cleanup_pop(&waiter->vecs->lock);
+
+		if (r) {
+			condlog(2, "%s: stopped watching dmevents",
+				curr_dev.name);
+			unwatch_dmevents(curr_dev.name);
+		}
+	}
+	condlog(0, "dmevent waiter thread unexpectedly quit");
+	return -1; /* never reach there */
+}
+
+static void rcu_unregister(void *param)
+{
+	rcu_unregister_thread();
+}
+
+void *wait_dmevents (void *unused)
+{
+	int r;
+
+
+	if (!waiter) {
+		condlog(0, "dmevents waiter not intialized");
+		return NULL;
+	}
+
+	pthread_cleanup_push(rcu_unregister, NULL);
+	rcu_register_thread();
+	mlockall(MCL_CURRENT | MCL_FUTURE);
+
+	while (1) {
+		r = dmevent_loop();
+
+		if (r < 0)
+			break;
+
+		sleep(r);
+	}
+
+	pthread_cleanup_pop(1);
+	return NULL;
+}
diff --git a/multipathd/dmevents.h b/multipathd/dmevents.h
new file mode 100644
index 0000000..012fbad
--- /dev/null
+++ b/multipathd/dmevents.h
@@ -0,0 +1,13 @@
+#ifndef _DMEVENTS_H
+#define _DMEVENTS_H
+
+#include "structs_vec.h"
+
+int dmevent_poll_supported(void);
+int init_dmevent_waiter(struct vectors *vecs);
+void cleanup_dmevent_waiter(void);
+int watch_dmevents(char *name);
+void unwatch_all_dmevents(void);
+void *wait_dmevents (void *unused);
+
+#endif /* _DMEVENTS_H */
diff --git a/multipathd/main.c b/multipathd/main.c
index aa86785..e35231e 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -82,6 +82,7 @@ static int use_watchdog;
 #include "cli_handlers.h"
 #include "lock.h"
 #include "waiter.h"
+#include "dmevents.h"
 #include "io_err_stat.h"
 #include "wwids.h"
 #include "foreign.h"
@@ -109,6 +110,11 @@ int uxsock_timeout;
 int verbosity;
 int bindings_read_only;
 int ignore_new_devs;
+#ifdef NO_DMEVENTS_POLL
+int poll_dmevents = 0;
+#else
+int poll_dmevents = 1;
+#endif
 enum daemon_status running_state = DAEMON_INIT;
 pid_t daemon_pid;
 pthread_mutex_t config_lock = PTHREAD_MUTEX_INITIALIZER;
@@ -293,11 +299,23 @@ switch_pathgroup (struct multipath * mpp)
 		 mpp->alias, mpp->bestpg);
 }
 
+static int
+wait_for_events(struct multipath *mpp, struct vectors *vecs)
+{
+	if (poll_dmevents)
+		return watch_dmevents(mpp->alias);
+	else
+		return start_waiter_thread(mpp, vecs);
+}
+
 static void
 remove_map_and_stop_waiter(struct multipath *mpp, struct vectors *vecs,
 			   int purge_vec)
 {
-	stop_waiter_thread(mpp, vecs);
+	/* devices are automatically removed by the dmevent polling code,
+	 * so they don't need to be manually removed here */
+	if (!poll_dmevents)
+		stop_waiter_thread(mpp, vecs);
 	remove_map(mpp, vecs, purge_vec);
 }
 
@@ -310,8 +328,12 @@ remove_maps_and_stop_waiters(struct vectors *vecs)
 	if (!vecs)
 		return;
 
-	vector_foreach_slot(vecs->mpvec, mpp, i)
-		stop_waiter_thread(mpp, vecs);
+	if (!poll_dmevents) {
+		vector_foreach_slot(vecs->mpvec, mpp, i)
+			stop_waiter_thread(mpp, vecs);
+	}
+	else
+		unwatch_all_dmevents();
 
 	remove_maps(vecs);
 }
@@ -356,7 +378,7 @@ retry:
 	dm_lib_release();
 
 fail:
-	if (new_map && (retries < 0 || start_waiter_thread(mpp, vecs))) {
+	if (new_map && (retries < 0 || wait_for_events(mpp, vecs))) {
 		condlog(0, "%s: failed to create new map", mpp->alias);
 		remove_map(mpp, vecs, 1);
 		return 1;
@@ -875,7 +897,7 @@ retry:
 
 	if ((mpp->action == ACT_CREATE ||
 	     (mpp->action == ACT_NOTHING && start_waiter && !mpp->waiter)) &&
-	    start_waiter_thread(mpp, vecs))
+	    wait_for_events(mpp, vecs))
 			goto fail_map;
 
 	/*
@@ -2203,7 +2225,7 @@ configure (struct vectors * vecs)
 	 * start dm event waiter threads for these new maps
 	 */
 	vector_foreach_slot(vecs->mpvec, mpp, i) {
-		if (start_waiter_thread(mpp, vecs)) {
+		if (wait_for_events(mpp, vecs)) {
 			remove_map(mpp, vecs, 1);
 			i--;
 			continue;
@@ -2449,7 +2471,7 @@ set_oom_adj (void)
 static int
 child (void * param)
 {
-	pthread_t check_thr, uevent_thr, uxlsnr_thr, uevq_thr;
+	pthread_t check_thr, uevent_thr, uxlsnr_thr, uevq_thr, dmevent_thr;
 	pthread_attr_t log_attr, misc_attr, uevent_attr;
 	struct vectors * vecs;
 	struct multipath * mpp;
@@ -2514,6 +2536,8 @@ child (void * param)
 
 	init_foreign(conf->multipath_dir);
 
+	if (poll_dmevents)
+		poll_dmevents = dmevent_poll_supported();
 	setlogmask(LOG_UPTO(conf->verbosity + 3));
 
 	envp = getenv("LimitNOFILE");
@@ -2580,6 +2604,19 @@ child (void * param)
 
 	init_path_check_interval(vecs);
 
+	if (poll_dmevents) {
+		if (init_dmevent_waiter(vecs)) {
+			condlog(0, "failed to allocate dmevents waiter info");
+			goto failed;
+		}
+		if ((rc = pthread_create(&dmevent_thr, &misc_attr,
+					 wait_dmevents, NULL))) {
+			condlog(0, "failed to create dmevent waiter thread: %d",
+				rc);
+			goto failed;
+		}
+	}
+
 	/*
 	 * Start uevent listener early to catch events
 	 */
@@ -2649,11 +2686,15 @@ child (void * param)
 	pthread_cancel(uevent_thr);
 	pthread_cancel(uxlsnr_thr);
 	pthread_cancel(uevq_thr);
+	if (poll_dmevents)
+		pthread_cancel(dmevent_thr);
 
 	pthread_join(check_thr, NULL);
 	pthread_join(uevent_thr, NULL);
 	pthread_join(uxlsnr_thr, NULL);
 	pthread_join(uevq_thr, NULL);
+	if (poll_dmevents)
+		pthread_join(dmevent_thr, NULL);
 
 	stop_io_err_stat_thread();
 
@@ -2669,6 +2710,8 @@ child (void * param)
 	cleanup_foreign();
 	cleanup_checkers();
 	cleanup_prio();
+	if (poll_dmevents)
+		cleanup_dmevent_waiter();
 
 	dm_lib_release();
 	dm_lib_exit();
@@ -2800,7 +2843,7 @@ main (int argc, char *argv[])
 	udev = udev_new();
 	libmp_udev_set_sync_support(0);
 
-	while ((arg = getopt(argc, argv, ":dsv:k::Bn")) != EOF ) {
+	while ((arg = getopt(argc, argv, ":dsv:k::Bniw")) != EOF ) {
 		switch(arg) {
 		case 'd':
 			foreground = 1;
@@ -2834,6 +2877,9 @@ main (int argc, char *argv[])
 		case 'n':
 			ignore_new_devs = 1;
 			break;
+		case 'w':
+			poll_dmevents = 0;
+			break;
 		default:
 			fprintf(stderr, "Invalid argument '-%c'\n",
 				optopt);
-- 
2.7.4
