* [RFC PATCH 1/5] libmultipath: move remove_map waiter code to multipathd
2018-02-10 5:07 [RFC PATCH 0/5] alternate dmevents waiter method Benjamin Marzinski
@ 2018-02-10 5:07 ` Benjamin Marzinski
2018-02-10 16:15 ` Martin Wilck
2018-02-10 5:07 ` [RFC PATCH 2/5] move waiter code from libmultipath " Benjamin Marzinski
` (4 subsequent siblings)
5 siblings, 1 reply; 19+ messages in thread
From: Benjamin Marzinski @ 2018-02-10 5:07 UTC (permalink / raw)
To: device-mapper development; +Cc: Martin Wilck
Only multipathd needs to worry about the multipath waiter code. There is
no point in having remove_map_and_stop_waiter() or
remove_maps_and_stop_waiters() in libmultipath, since they should never
be use outside of multipathd.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
libmultipath/structs_vec.c | 40 +++++-----------------------------------
libmultipath/structs_vec.h | 2 --
multipathd/main.c | 23 +++++++++++++++++++++++
3 files changed, 28 insertions(+), 37 deletions(-)
diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
index 0de2221..abf5327 100644
--- a/libmultipath/structs_vec.c
+++ b/libmultipath/structs_vec.c
@@ -116,25 +116,16 @@ set_multipath_wwid (struct multipath * mpp)
dm_get_uuid(mpp->alias, mpp->wwid);
}
-#define KEEP_WAITER 0
-#define STOP_WAITER 1
#define PURGE_VEC 1
-static void
-_remove_map (struct multipath * mpp, struct vectors * vecs,
- int stop_waiter, int purge_vec)
+void
+remove_map(struct multipath * mpp, struct vectors * vecs, int purge_vec)
{
int i;
condlog(4, "%s: remove multipath map", mpp->alias);
/*
- * stop the DM event waiter thread
- */
- if (stop_waiter)
- stop_waiter_thread(mpp, vecs);
-
- /*
* clear references to this map
*/
orphan_paths(vecs->pathvec, mpp);
@@ -149,19 +140,8 @@ _remove_map (struct multipath * mpp, struct vectors * vecs,
free_multipath(mpp, KEEP_PATHS);
}
-void remove_map(struct multipath *mpp, struct vectors *vecs, int purge_vec)
-{
- _remove_map(mpp, vecs, KEEP_WAITER, purge_vec);
-}
-
-void remove_map_and_stop_waiter(struct multipath *mpp, struct vectors *vecs,
- int purge_vec)
-{
- _remove_map(mpp, vecs, STOP_WAITER, purge_vec);
-}
-
-static void
-_remove_maps (struct vectors * vecs, int stop_waiter)
+void
+remove_maps(struct vectors * vecs)
{
int i;
struct multipath * mpp;
@@ -170,7 +150,7 @@ _remove_maps (struct vectors * vecs, int stop_waiter)
return;
vector_foreach_slot (vecs->mpvec, mpp, i) {
- _remove_map(mpp, vecs, stop_waiter, 1);
+ remove_map(mpp, vecs, 1);
i--;
}
@@ -178,16 +158,6 @@ _remove_maps (struct vectors * vecs, int stop_waiter)
vecs->mpvec = NULL;
}
-void remove_maps(struct vectors *vecs)
-{
- _remove_maps(vecs, KEEP_WAITER);
-}
-
-void remove_maps_and_stop_waiters(struct vectors *vecs)
-{
- _remove_maps(vecs, STOP_WAITER);
-}
-
void
extract_hwe_from_path(struct multipath * mpp)
{
diff --git a/libmultipath/structs_vec.h b/libmultipath/structs_vec.h
index b81413b..d6e17bb 100644
--- a/libmultipath/structs_vec.h
+++ b/libmultipath/structs_vec.h
@@ -27,9 +27,7 @@ int update_multipath_strings (struct multipath *mpp, vector pathvec,
void extract_hwe_from_path(struct multipath * mpp);
void remove_map (struct multipath * mpp, struct vectors * vecs, int purge_vec);
-void remove_map_and_stop_waiter (struct multipath * mpp, struct vectors * vecs, int purge_vec);
void remove_maps (struct vectors * vecs);
-void remove_maps_and_stop_waiters (struct vectors * vecs);
void sync_map_state (struct multipath *);
int update_map (struct multipath *mpp, struct vectors *vecs);
diff --git a/multipathd/main.c b/multipathd/main.c
index 7ac59d9..72c3c2f 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -288,6 +288,29 @@ switch_pathgroup (struct multipath * mpp)
mpp->alias, mpp->bestpg);
}
+static void
+remove_map_and_stop_waiter(struct multipath *mpp, struct vectors *vecs,
+ int purge_vec)
+{
+ stop_waiter_thread(mpp, vecs);
+ remove_map(mpp, vecs, purge_vec);
+}
+
+static void
+remove_maps_and_stop_waiters(struct vectors *vecs)
+{
+ int i;
+ struct multipath * mpp;
+
+ if (!vecs)
+ return;
+
+ vector_foreach_slot(vecs->mpvec, mpp, i)
+ stop_waiter_thread(mpp, vecs);
+
+ remove_maps(vecs);
+}
+
static int
coalesce_maps(struct vectors *vecs, vector nmpv)
{
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [RFC PATCH 1/5] libmultipath: move remove_map waiter code to multipathd
2018-02-10 5:07 ` [RFC PATCH 1/5] libmultipath: move remove_map waiter code to multipathd Benjamin Marzinski
@ 2018-02-10 16:15 ` Martin Wilck
0 siblings, 0 replies; 19+ messages in thread
From: Martin Wilck @ 2018-02-10 16:15 UTC (permalink / raw)
To: Benjamin Marzinski, device-mapper development
On Fri, 2018-02-09 at 23:07 -0600, Benjamin Marzinski wrote:
> Only multipathd needs to worry about the multipath waiter code. There
> is
> no point in having remove_map_and_stop_waiter() or
> remove_maps_and_stop_waiters() in libmultipath, since they should
> never
> be use outside of multipathd.
>
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
> ---
> libmultipath/structs_vec.c | 40 +++++-------------------------------
> ----
> libmultipath/structs_vec.h | 2 --
> multipathd/main.c | 23 +++++++++++++++++++++++
> 3 files changed, 28 insertions(+), 37 deletions(-)
>
> diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
> index 0de2221..abf5327 100644
> --- a/libmultipath/structs_vec.c
> +++ b/libmultipath/structs_vec.c
> @@ -116,25 +116,16 @@ set_multipath_wwid (struct multipath * mpp)
> dm_get_uuid(mpp->alias, mpp->wwid);
> }
>
> -#define KEEP_WAITER 0
> -#define STOP_WAITER 1
> #define PURGE_VEC 1
>
> -static void
> -_remove_map (struct multipath * mpp, struct vectors * vecs,
> - int stop_waiter, int purge_vec)
> +void
> +remove_map(struct multipath * mpp, struct vectors * vecs, int
> purge_vec)
> {
> int i;
>
> condlog(4, "%s: remove multipath map", mpp->alias);
>
> /*
> - * stop the DM event waiter thread
> - */
> - if (stop_waiter)
> - stop_waiter_thread(mpp, vecs);
> -
> - /*
> * clear references to this map
> */
> orphan_paths(vecs->pathvec, mpp);
> @@ -149,19 +140,8 @@ _remove_map (struct multipath * mpp, struct
> vectors * vecs,
> free_multipath(mpp, KEEP_PATHS);
> }
>
> -void remove_map(struct multipath *mpp, struct vectors *vecs, int
> purge_vec)
> -{
> - _remove_map(mpp, vecs, KEEP_WAITER, purge_vec);
> -}
> -
> -void remove_map_and_stop_waiter(struct multipath *mpp, struct
> vectors *vecs,
> - int purge_vec)
> -{
> - _remove_map(mpp, vecs, STOP_WAITER, purge_vec);
> -}
> -
> -static void
> -_remove_maps (struct vectors * vecs, int stop_waiter)
> +void
> +remove_maps(struct vectors * vecs)
> {
> int i;
> struct multipath * mpp;
> @@ -170,7 +150,7 @@ _remove_maps (struct vectors * vecs, int
> stop_waiter)
> return;
>
> vector_foreach_slot (vecs->mpvec, mpp, i) {
> - _remove_map(mpp, vecs, stop_waiter, 1);
> + remove_map(mpp, vecs, 1);
> i--;
> }
>
> @@ -178,16 +158,6 @@ _remove_maps (struct vectors * vecs, int
> stop_waiter)
> vecs->mpvec = NULL;
> }
>
> -void remove_maps(struct vectors *vecs)
> -{
> - _remove_maps(vecs, KEEP_WAITER);
> -}
> -
> -void remove_maps_and_stop_waiters(struct vectors *vecs)
> -{
> - _remove_maps(vecs, STOP_WAITER);
> -}
> -
> void
> extract_hwe_from_path(struct multipath * mpp)
> {
> diff --git a/libmultipath/structs_vec.h b/libmultipath/structs_vec.h
> index b81413b..d6e17bb 100644
> --- a/libmultipath/structs_vec.h
> +++ b/libmultipath/structs_vec.h
> @@ -27,9 +27,7 @@ int update_multipath_strings (struct multipath
> *mpp, vector pathvec,
> void extract_hwe_from_path(struct multipath * mpp);
>
> void remove_map (struct multipath * mpp, struct vectors * vecs, int
> purge_vec);
> -void remove_map_and_stop_waiter (struct multipath * mpp, struct
> vectors * vecs, int purge_vec);
> void remove_maps (struct vectors * vecs);
> -void remove_maps_and_stop_waiters (struct vectors * vecs);
>
> void sync_map_state (struct multipath *);
> int update_map (struct multipath *mpp, struct vectors *vecs);
> diff --git a/multipathd/main.c b/multipathd/main.c
> index 7ac59d9..72c3c2f 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -288,6 +288,29 @@ switch_pathgroup (struct multipath * mpp)
> mpp->alias, mpp->bestpg);
> }
>
> +static void
> +remove_map_and_stop_waiter(struct multipath *mpp, struct vectors
> *vecs,
> + int purge_vec)
> +{
> + stop_waiter_thread(mpp, vecs);
> + remove_map(mpp, vecs, purge_vec);
> +}
> +
> +static void
> +remove_maps_and_stop_waiters(struct vectors *vecs)
> +{
> + int i;
> + struct multipath * mpp;
> +
> + if (!vecs)
> + return;
> +
> + vector_foreach_slot(vecs->mpvec, mpp, i)
> + stop_waiter_thread(mpp, vecs);
> +
> + remove_maps(vecs);
> +}
> +
> static int
> coalesce_maps(struct vectors *vecs, vector nmpv)
> {
--
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFC PATCH 2/5] move waiter code from libmultipath to multipathd
2018-02-10 5:07 [RFC PATCH 0/5] alternate dmevents waiter method Benjamin Marzinski
2018-02-10 5:07 ` [RFC PATCH 1/5] libmultipath: move remove_map waiter code to multipathd Benjamin Marzinski
@ 2018-02-10 5:07 ` Benjamin Marzinski
2018-02-10 16:16 ` Martin Wilck
2018-02-10 5:07 ` [RFC PATCH 3/5] call start_waiter_thread() before setup_multipath() Benjamin Marzinski
` (3 subsequent siblings)
5 siblings, 1 reply; 19+ messages in thread
From: Benjamin Marzinski @ 2018-02-10 5:07 UTC (permalink / raw)
To: device-mapper development; +Cc: Martin Wilck
Only multipathd uses the code in waiter.[ch] and the functions that call
it directly, so they should all live in the multipathd directory. This
patch is simply moving the waiter.[ch] files and the functions in
structs_vec that use them. None of the moved code has been changed.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
libmultipath/Makefile | 2 +-
libmultipath/structs_vec.c | 98 ---------------------
libmultipath/structs_vec.h | 4 +-
libmultipath/waiter.c | 215 ---------------------------------------------
libmultipath/waiter.h | 17 ----
multipathd/Makefile | 2 +-
multipathd/main.c | 96 ++++++++++++++++++++
multipathd/waiter.c | 215 +++++++++++++++++++++++++++++++++++++++++++++
multipathd/waiter.h | 17 ++++
9 files changed, 332 insertions(+), 334 deletions(-)
delete mode 100644 libmultipath/waiter.c
delete mode 100644 libmultipath/waiter.h
create mode 100644 multipathd/waiter.c
create mode 100644 multipathd/waiter.h
diff --git a/libmultipath/Makefile b/libmultipath/Makefile
index 6447d8d..a1005b2 100644
--- a/libmultipath/Makefile
+++ b/libmultipath/Makefile
@@ -42,7 +42,7 @@ OBJS = memory.o parser.o vector.o devmapper.o callout.o \
pgpolicies.o debug.o defaults.o uevent.o time-util.o \
switchgroup.o uxsock.o print.o alias.o log_pthread.o \
log.o configure.o structs_vec.o sysfs.o prio.o checkers.o \
- lock.o waiter.o file.o wwids.o prioritizers/alua_rtpg.o prkey.o \
+ lock.o file.o wwids.o prioritizers/alua_rtpg.o prkey.o \
io_err_stat.o
all: $(LIBS)
diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
index abf5327..77b045b 100644
--- a/libmultipath/structs_vec.c
+++ b/libmultipath/structs_vec.c
@@ -10,7 +10,6 @@
#include "structs.h"
#include "structs_vec.h"
#include "sysfs.h"
-#include "waiter.h"
#include "devmapper.h"
#include "dmparser.h"
#include "propsel.h"
@@ -107,17 +106,6 @@ void orphan_paths(vector pathvec, struct multipath *mpp)
}
}
-static void
-set_multipath_wwid (struct multipath * mpp)
-{
- if (strlen(mpp->wwid))
- return;
-
- dm_get_uuid(mpp->alias, mpp->wwid);
-}
-
-#define PURGE_VEC 1
-
void
remove_map(struct multipath * mpp, struct vectors * vecs, int purge_vec)
{
@@ -379,92 +367,6 @@ sync_map_state(struct multipath *mpp)
}
}
-int
-update_map (struct multipath *mpp, struct vectors *vecs)
-{
- int retries = 3;
- char params[PARAMS_SIZE] = {0};
-
-retry:
- condlog(4, "%s: updating new map", mpp->alias);
- if (adopt_paths(vecs->pathvec, mpp)) {
- condlog(0, "%s: failed to adopt paths for new map update",
- mpp->alias);
- retries = -1;
- goto fail;
- }
- verify_paths(mpp, vecs);
- mpp->action = ACT_RELOAD;
-
- extract_hwe_from_path(mpp);
- if (setup_map(mpp, params, PARAMS_SIZE)) {
- condlog(0, "%s: failed to setup new map in update", mpp->alias);
- retries = -1;
- goto fail;
- }
- if (domap(mpp, params, 1) <= 0 && retries-- > 0) {
- condlog(0, "%s: map_udate sleep", mpp->alias);
- sleep(1);
- goto retry;
- }
- dm_lib_release();
-
-fail:
- if (setup_multipath(vecs, mpp))
- return 1;
-
- sync_map_state(mpp);
-
- if (retries < 0)
- condlog(0, "%s: failed reload in new map update", mpp->alias);
- return 0;
-}
-
-struct multipath *add_map_without_path (struct vectors *vecs, char *alias)
-{
- struct multipath * mpp = alloc_multipath();
- struct config *conf;
-
- if (!mpp)
- return NULL;
- if (!alias) {
- FREE(mpp);
- return NULL;
- }
-
- mpp->alias = STRDUP(alias);
-
- if (dm_get_info(mpp->alias, &mpp->dmi)) {
- condlog(3, "%s: cannot access table", mpp->alias);
- goto out;
- }
- set_multipath_wwid(mpp);
- conf = get_multipath_config();
- mpp->mpe = find_mpe(conf->mptable, mpp->wwid);
- put_multipath_config(conf);
-
- if (update_multipath_table(mpp, vecs->pathvec, 1))
- goto out;
- if (update_multipath_status(mpp))
- goto out;
-
- if (!vector_alloc_slot(vecs->mpvec))
- goto out;
-
- vector_set_slot(vecs->mpvec, mpp);
-
- if (update_map(mpp, vecs) != 0) /* map removed */
- return NULL;
-
- if (start_waiter_thread(mpp, vecs))
- goto out;
-
- return mpp;
-out:
- remove_map(mpp, vecs, PURGE_VEC);
- return NULL;
-}
-
static void
find_existing_alias (struct multipath * mpp,
struct vectors *vecs)
diff --git a/libmultipath/structs_vec.h b/libmultipath/structs_vec.h
index d6e17bb..ceab6d9 100644
--- a/libmultipath/structs_vec.h
+++ b/libmultipath/structs_vec.h
@@ -26,12 +26,12 @@ int update_multipath_strings (struct multipath *mpp, vector pathvec,
int is_daemon);
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_maps (struct vectors * vecs);
void sync_map_state (struct multipath *);
-int update_map (struct multipath *mpp, struct vectors *vecs);
-struct multipath * add_map_without_path (struct vectors * vecs, char * alias);
struct multipath * add_map_with_path (struct vectors * vecs,
struct path * pp, int add_vec);
int update_multipath (struct vectors *vecs, char *mapname, int reset);
diff --git a/libmultipath/waiter.c b/libmultipath/waiter.c
deleted file mode 100644
index cb9708b..0000000
--- a/libmultipath/waiter.c
+++ /dev/null
@@ -1,215 +0,0 @@
-/*
- * Copyright (c) 2004, 2005 Christophe Varoqui
- * Copyright (c) 2005 Kiyoshi Ueda, NEC
- * Copyright (c) 2005 Benjamin Marzinski, Redhat
- * Copyright (c) 2005 Edward Goggin, EMC
- */
-#include <unistd.h>
-#include <libdevmapper.h>
-#include <sys/mman.h>
-#include <pthread.h>
-#include <signal.h>
-#include <urcu.h>
-
-#include "vector.h"
-#include "memory.h"
-#include "checkers.h"
-#include "config.h"
-#include "structs.h"
-#include "structs_vec.h"
-#include "devmapper.h"
-#include "debug.h"
-#include "lock.h"
-#include "waiter.h"
-
-pthread_attr_t waiter_attr;
-
-static struct event_thread *alloc_waiter (void)
-{
-
- struct event_thread *wp;
-
- wp = (struct event_thread *)MALLOC(sizeof(struct event_thread));
- memset(wp, 0, sizeof(struct event_thread));
-
- return wp;
-}
-
-static void free_waiter (void *data)
-{
- struct event_thread *wp = (struct event_thread *)data;
-
- if (wp->dmt)
- dm_task_destroy(wp->dmt);
-
- rcu_unregister_thread();
- FREE(wp);
-}
-
-void stop_waiter_thread (struct multipath *mpp, struct vectors *vecs)
-{
- pthread_t thread;
-
- if (mpp->waiter == (pthread_t)0) {
- condlog(3, "%s: event checker thread already stopped",
- mpp->alias);
- return;
- }
- condlog(2, "%s: stop event checker thread (%lu)", mpp->alias,
- mpp->waiter);
- thread = mpp->waiter;
- mpp->waiter = (pthread_t)0;
- pthread_cancel(thread);
- pthread_kill(thread, SIGUSR2);
-}
-
-/*
- * returns the reschedule delay
- * negative means *stop*
- */
-static int waiteventloop (struct event_thread *waiter)
-{
- sigset_t set, oldset;
- int event_nr;
- int r;
-
- if (!waiter->event_nr)
- waiter->event_nr = dm_geteventnr(waiter->mapname);
-
- if (!(waiter->dmt = libmp_dm_task_create(DM_DEVICE_WAITEVENT))) {
- condlog(0, "%s: devmap event #%i dm_task_create error",
- waiter->mapname, waiter->event_nr);
- return 1;
- }
-
- if (!dm_task_set_name(waiter->dmt, waiter->mapname)) {
- condlog(0, "%s: devmap event #%i dm_task_set_name error",
- waiter->mapname, waiter->event_nr);
- dm_task_destroy(waiter->dmt);
- waiter->dmt = NULL;
- return 1;
- }
-
- if (waiter->event_nr && !dm_task_set_event_nr(waiter->dmt,
- waiter->event_nr)) {
- condlog(0, "%s: devmap event #%i dm_task_set_event_nr error",
- waiter->mapname, waiter->event_nr);
- dm_task_destroy(waiter->dmt);
- waiter->dmt = NULL;
- return 1;
- }
-
- dm_task_no_open_count(waiter->dmt);
-
- /* wait */
- sigemptyset(&set);
- sigaddset(&set, SIGUSR2);
- pthread_sigmask(SIG_UNBLOCK, &set, &oldset);
-
- pthread_testcancel();
- r = dm_task_run(waiter->dmt);
- pthread_testcancel();
-
- pthread_sigmask(SIG_SETMASK, &oldset, NULL);
- dm_task_destroy(waiter->dmt);
- waiter->dmt = NULL;
-
- if (!r) /* wait interrupted by signal */
- return -1;
-
- waiter->event_nr++;
-
- /*
- * upon event ...
- */
- while (1) {
- condlog(3, "%s: devmap event #%i",
- waiter->mapname, waiter->event_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 = update_multipath(waiter->vecs, waiter->mapname, 1);
- lock_cleanup_pop(waiter->vecs->lock);
-
- if (r) {
- condlog(2, "%s: event checker exit",
- waiter->mapname);
- return -1; /* stop the thread */
- }
-
- event_nr = dm_geteventnr(waiter->mapname);
-
- if (waiter->event_nr == event_nr)
- return 1; /* upon problem reschedule 1s later */
-
- waiter->event_nr = event_nr;
- }
- return -1; /* never reach there */
-}
-
-static void *waitevent (void *et)
-{
- int r;
- struct event_thread *waiter;
-
- mlockall(MCL_CURRENT | MCL_FUTURE);
-
- waiter = (struct event_thread *)et;
- pthread_cleanup_push(free_waiter, et);
-
- rcu_register_thread();
- while (1) {
- r = waiteventloop(waiter);
-
- if (r < 0)
- break;
-
- sleep(r);
- }
-
- pthread_cleanup_pop(1);
- return NULL;
-}
-
-int start_waiter_thread (struct multipath *mpp, struct vectors *vecs)
-{
- struct event_thread *wp;
-
- if (!mpp)
- return 0;
-
- wp = alloc_waiter();
-
- if (!wp)
- goto out;
-
- strncpy(wp->mapname, mpp->alias, WWID_SIZE - 1);
- wp->vecs = vecs;
-
- if (pthread_create(&wp->thread, &waiter_attr, waitevent, wp)) {
- condlog(0, "%s: cannot create event checker", wp->mapname);
- goto out1;
- }
- mpp->waiter = wp->thread;
- condlog(2, "%s: event checker started", wp->mapname);
-
- return 0;
-out1:
- free_waiter(wp);
- mpp->waiter = (pthread_t)0;
-out:
- condlog(0, "failed to start waiter thread");
- return 1;
-}
diff --git a/libmultipath/waiter.h b/libmultipath/waiter.h
deleted file mode 100644
index 0cfae46..0000000
--- a/libmultipath/waiter.h
+++ /dev/null
@@ -1,17 +0,0 @@
-#ifndef _WAITER_H
-#define _WAITER_H
-
-extern pthread_attr_t waiter_attr;
-
-struct event_thread {
- struct dm_task *dmt;
- pthread_t thread;
- int event_nr;
- char mapname[WWID_SIZE];
- struct vectors *vecs;
-};
-
-void stop_waiter_thread (struct multipath *mpp, struct vectors *vecs);
-int start_waiter_thread (struct multipath *mpp, struct vectors *vecs);
-
-#endif /* _WAITER_H */
diff --git a/multipathd/Makefile b/multipathd/Makefile
index e6f140b..85f29a7 100644
--- a/multipathd/Makefile
+++ b/multipathd/Makefile
@@ -22,7 +22,7 @@ ifdef SYSTEMD
endif
endif
-OBJS = main.o pidfile.o uxlsnr.o uxclnt.o cli.o cli_handlers.o
+OBJS = main.o pidfile.o uxlsnr.o uxclnt.o cli.o cli_handlers.o waiter.o
EXEC = multipathd
diff --git a/multipathd/main.c b/multipathd/main.c
index 72c3c2f..94b2406 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -311,6 +311,102 @@ remove_maps_and_stop_waiters(struct vectors *vecs)
remove_maps(vecs);
}
+static void
+set_multipath_wwid (struct multipath * mpp)
+{
+ if (strlen(mpp->wwid))
+ return;
+
+ dm_get_uuid(mpp->alias, mpp->wwid);
+}
+
+static int
+update_map (struct multipath *mpp, struct vectors *vecs)
+{
+ int retries = 3;
+ char params[PARAMS_SIZE] = {0};
+
+retry:
+ condlog(4, "%s: updating new map", mpp->alias);
+ if (adopt_paths(vecs->pathvec, mpp)) {
+ condlog(0, "%s: failed to adopt paths for new map update",
+ mpp->alias);
+ retries = -1;
+ goto fail;
+ }
+ verify_paths(mpp, vecs);
+ mpp->action = ACT_RELOAD;
+
+ extract_hwe_from_path(mpp);
+ if (setup_map(mpp, params, PARAMS_SIZE)) {
+ condlog(0, "%s: failed to setup new map in update", mpp->alias);
+ retries = -1;
+ goto fail;
+ }
+ if (domap(mpp, params, 1) <= 0 && retries-- > 0) {
+ condlog(0, "%s: map_udate sleep", mpp->alias);
+ sleep(1);
+ goto retry;
+ }
+ dm_lib_release();
+
+fail:
+ if (setup_multipath(vecs, mpp))
+ return 1;
+
+ sync_map_state(mpp);
+
+ if (retries < 0)
+ condlog(0, "%s: failed reload in new map update", mpp->alias);
+ return 0;
+}
+
+static struct multipath *
+add_map_without_path (struct vectors *vecs, char *alias)
+{
+ struct multipath * mpp = alloc_multipath();
+ struct config *conf;
+
+ if (!mpp)
+ return NULL;
+ if (!alias) {
+ FREE(mpp);
+ return NULL;
+ }
+
+ mpp->alias = STRDUP(alias);
+
+ if (dm_get_info(mpp->alias, &mpp->dmi)) {
+ condlog(3, "%s: cannot access table", mpp->alias);
+ goto out;
+ }
+ set_multipath_wwid(mpp);
+ conf = get_multipath_config();
+ mpp->mpe = find_mpe(conf->mptable, mpp->wwid);
+ put_multipath_config(conf);
+
+ if (update_multipath_table(mpp, vecs->pathvec, 1))
+ goto out;
+ if (update_multipath_status(mpp))
+ goto out;
+
+ if (!vector_alloc_slot(vecs->mpvec))
+ goto out;
+
+ vector_set_slot(vecs->mpvec, mpp);
+
+ if (update_map(mpp, vecs) != 0) /* map removed */
+ return NULL;
+
+ if (start_waiter_thread(mpp, vecs))
+ goto out;
+
+ return mpp;
+out:
+ remove_map(mpp, vecs, PURGE_VEC);
+ return NULL;
+}
+
static int
coalesce_maps(struct vectors *vecs, vector nmpv)
{
diff --git a/multipathd/waiter.c b/multipathd/waiter.c
new file mode 100644
index 0000000..cb9708b
--- /dev/null
+++ b/multipathd/waiter.c
@@ -0,0 +1,215 @@
+/*
+ * Copyright (c) 2004, 2005 Christophe Varoqui
+ * Copyright (c) 2005 Kiyoshi Ueda, NEC
+ * Copyright (c) 2005 Benjamin Marzinski, Redhat
+ * Copyright (c) 2005 Edward Goggin, EMC
+ */
+#include <unistd.h>
+#include <libdevmapper.h>
+#include <sys/mman.h>
+#include <pthread.h>
+#include <signal.h>
+#include <urcu.h>
+
+#include "vector.h"
+#include "memory.h"
+#include "checkers.h"
+#include "config.h"
+#include "structs.h"
+#include "structs_vec.h"
+#include "devmapper.h"
+#include "debug.h"
+#include "lock.h"
+#include "waiter.h"
+
+pthread_attr_t waiter_attr;
+
+static struct event_thread *alloc_waiter (void)
+{
+
+ struct event_thread *wp;
+
+ wp = (struct event_thread *)MALLOC(sizeof(struct event_thread));
+ memset(wp, 0, sizeof(struct event_thread));
+
+ return wp;
+}
+
+static void free_waiter (void *data)
+{
+ struct event_thread *wp = (struct event_thread *)data;
+
+ if (wp->dmt)
+ dm_task_destroy(wp->dmt);
+
+ rcu_unregister_thread();
+ FREE(wp);
+}
+
+void stop_waiter_thread (struct multipath *mpp, struct vectors *vecs)
+{
+ pthread_t thread;
+
+ if (mpp->waiter == (pthread_t)0) {
+ condlog(3, "%s: event checker thread already stopped",
+ mpp->alias);
+ return;
+ }
+ condlog(2, "%s: stop event checker thread (%lu)", mpp->alias,
+ mpp->waiter);
+ thread = mpp->waiter;
+ mpp->waiter = (pthread_t)0;
+ pthread_cancel(thread);
+ pthread_kill(thread, SIGUSR2);
+}
+
+/*
+ * returns the reschedule delay
+ * negative means *stop*
+ */
+static int waiteventloop (struct event_thread *waiter)
+{
+ sigset_t set, oldset;
+ int event_nr;
+ int r;
+
+ if (!waiter->event_nr)
+ waiter->event_nr = dm_geteventnr(waiter->mapname);
+
+ if (!(waiter->dmt = libmp_dm_task_create(DM_DEVICE_WAITEVENT))) {
+ condlog(0, "%s: devmap event #%i dm_task_create error",
+ waiter->mapname, waiter->event_nr);
+ return 1;
+ }
+
+ if (!dm_task_set_name(waiter->dmt, waiter->mapname)) {
+ condlog(0, "%s: devmap event #%i dm_task_set_name error",
+ waiter->mapname, waiter->event_nr);
+ dm_task_destroy(waiter->dmt);
+ waiter->dmt = NULL;
+ return 1;
+ }
+
+ if (waiter->event_nr && !dm_task_set_event_nr(waiter->dmt,
+ waiter->event_nr)) {
+ condlog(0, "%s: devmap event #%i dm_task_set_event_nr error",
+ waiter->mapname, waiter->event_nr);
+ dm_task_destroy(waiter->dmt);
+ waiter->dmt = NULL;
+ return 1;
+ }
+
+ dm_task_no_open_count(waiter->dmt);
+
+ /* wait */
+ sigemptyset(&set);
+ sigaddset(&set, SIGUSR2);
+ pthread_sigmask(SIG_UNBLOCK, &set, &oldset);
+
+ pthread_testcancel();
+ r = dm_task_run(waiter->dmt);
+ pthread_testcancel();
+
+ pthread_sigmask(SIG_SETMASK, &oldset, NULL);
+ dm_task_destroy(waiter->dmt);
+ waiter->dmt = NULL;
+
+ if (!r) /* wait interrupted by signal */
+ return -1;
+
+ waiter->event_nr++;
+
+ /*
+ * upon event ...
+ */
+ while (1) {
+ condlog(3, "%s: devmap event #%i",
+ waiter->mapname, waiter->event_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 = update_multipath(waiter->vecs, waiter->mapname, 1);
+ lock_cleanup_pop(waiter->vecs->lock);
+
+ if (r) {
+ condlog(2, "%s: event checker exit",
+ waiter->mapname);
+ return -1; /* stop the thread */
+ }
+
+ event_nr = dm_geteventnr(waiter->mapname);
+
+ if (waiter->event_nr == event_nr)
+ return 1; /* upon problem reschedule 1s later */
+
+ waiter->event_nr = event_nr;
+ }
+ return -1; /* never reach there */
+}
+
+static void *waitevent (void *et)
+{
+ int r;
+ struct event_thread *waiter;
+
+ mlockall(MCL_CURRENT | MCL_FUTURE);
+
+ waiter = (struct event_thread *)et;
+ pthread_cleanup_push(free_waiter, et);
+
+ rcu_register_thread();
+ while (1) {
+ r = waiteventloop(waiter);
+
+ if (r < 0)
+ break;
+
+ sleep(r);
+ }
+
+ pthread_cleanup_pop(1);
+ return NULL;
+}
+
+int start_waiter_thread (struct multipath *mpp, struct vectors *vecs)
+{
+ struct event_thread *wp;
+
+ if (!mpp)
+ return 0;
+
+ wp = alloc_waiter();
+
+ if (!wp)
+ goto out;
+
+ strncpy(wp->mapname, mpp->alias, WWID_SIZE - 1);
+ wp->vecs = vecs;
+
+ if (pthread_create(&wp->thread, &waiter_attr, waitevent, wp)) {
+ condlog(0, "%s: cannot create event checker", wp->mapname);
+ goto out1;
+ }
+ mpp->waiter = wp->thread;
+ condlog(2, "%s: event checker started", wp->mapname);
+
+ return 0;
+out1:
+ free_waiter(wp);
+ mpp->waiter = (pthread_t)0;
+out:
+ condlog(0, "failed to start waiter thread");
+ return 1;
+}
diff --git a/multipathd/waiter.h b/multipathd/waiter.h
new file mode 100644
index 0000000..0cfae46
--- /dev/null
+++ b/multipathd/waiter.h
@@ -0,0 +1,17 @@
+#ifndef _WAITER_H
+#define _WAITER_H
+
+extern pthread_attr_t waiter_attr;
+
+struct event_thread {
+ struct dm_task *dmt;
+ pthread_t thread;
+ int event_nr;
+ char mapname[WWID_SIZE];
+ struct vectors *vecs;
+};
+
+void stop_waiter_thread (struct multipath *mpp, struct vectors *vecs);
+int start_waiter_thread (struct multipath *mpp, struct vectors *vecs);
+
+#endif /* _WAITER_H */
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [RFC PATCH 2/5] move waiter code from libmultipath to multipathd
2018-02-10 5:07 ` [RFC PATCH 2/5] move waiter code from libmultipath " Benjamin Marzinski
@ 2018-02-10 16:16 ` Martin Wilck
0 siblings, 0 replies; 19+ messages in thread
From: Martin Wilck @ 2018-02-10 16:16 UTC (permalink / raw)
To: Benjamin Marzinski, device-mapper development
On Fri, 2018-02-09 at 23:07 -0600, Benjamin Marzinski wrote:
> Only multipathd uses the code in waiter.[ch] and the functions that
> call
> it directly, so they should all live in the multipathd
> directory. This
> patch is simply moving the waiter.[ch] files and the functions in
> structs_vec that use them. None of the moved code has been changed.
>
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
> ---
> libmultipath/Makefile | 2 +-
> libmultipath/structs_vec.c | 98 ---------------------
> libmultipath/structs_vec.h | 4 +-
> libmultipath/waiter.c | 215 ---------------------------------
> ------------
> libmultipath/waiter.h | 17 ----
> multipathd/Makefile | 2 +-
> multipathd/main.c | 96 ++++++++++++++++++++
> multipathd/waiter.c | 215
> +++++++++++++++++++++++++++++++++++++++++++++
> multipathd/waiter.h | 17 ++++
> 9 files changed, 332 insertions(+), 334 deletions(-)
> delete mode 100644 libmultipath/waiter.c
> delete mode 100644 libmultipath/waiter.h
> create mode 100644 multipathd/waiter.c
> create mode 100644 multipathd/waiter.h
>
> diff --git a/libmultipath/Makefile b/libmultipath/Makefile
> index 6447d8d..a1005b2 100644
> --- a/libmultipath/Makefile
> +++ b/libmultipath/Makefile
> @@ -42,7 +42,7 @@ OBJS = memory.o parser.o vector.o devmapper.o
> callout.o \
> pgpolicies.o debug.o defaults.o uevent.o time-util.o \
> switchgroup.o uxsock.o print.o alias.o log_pthread.o \
> log.o configure.o structs_vec.o sysfs.o prio.o checkers.o \
> - lock.o waiter.o file.o wwids.o prioritizers/alua_rtpg.o
> prkey.o \
> + lock.o file.o wwids.o prioritizers/alua_rtpg.o prkey.o \
> io_err_stat.o
>
> all: $(LIBS)
> diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
> index abf5327..77b045b 100644
> --- a/libmultipath/structs_vec.c
> +++ b/libmultipath/structs_vec.c
> @@ -10,7 +10,6 @@
> #include "structs.h"
> #include "structs_vec.h"
> #include "sysfs.h"
> -#include "waiter.h"
> #include "devmapper.h"
> #include "dmparser.h"
> #include "propsel.h"
> @@ -107,17 +106,6 @@ void orphan_paths(vector pathvec, struct
> multipath *mpp)
> }
> }
>
> -static void
> -set_multipath_wwid (struct multipath * mpp)
> -{
> - if (strlen(mpp->wwid))
> - return;
> -
> - dm_get_uuid(mpp->alias, mpp->wwid);
> -}
> -
> -#define PURGE_VEC 1
> -
> void
> remove_map(struct multipath * mpp, struct vectors * vecs, int
> purge_vec)
> {
> @@ -379,92 +367,6 @@ sync_map_state(struct multipath *mpp)
> }
> }
>
> -int
> -update_map (struct multipath *mpp, struct vectors *vecs)
> -{
> - int retries = 3;
> - char params[PARAMS_SIZE] = {0};
> -
> -retry:
> - condlog(4, "%s: updating new map", mpp->alias);
> - if (adopt_paths(vecs->pathvec, mpp)) {
> - condlog(0, "%s: failed to adopt paths for new map
> update",
> - mpp->alias);
> - retries = -1;
> - goto fail;
> - }
> - verify_paths(mpp, vecs);
> - mpp->action = ACT_RELOAD;
> -
> - extract_hwe_from_path(mpp);
> - if (setup_map(mpp, params, PARAMS_SIZE)) {
> - condlog(0, "%s: failed to setup new map in update",
> mpp->alias);
> - retries = -1;
> - goto fail;
> - }
> - if (domap(mpp, params, 1) <= 0 && retries-- > 0) {
> - condlog(0, "%s: map_udate sleep", mpp->alias);
> - sleep(1);
> - goto retry;
> - }
> - dm_lib_release();
> -
> -fail:
> - if (setup_multipath(vecs, mpp))
> - return 1;
> -
> - sync_map_state(mpp);
> -
> - if (retries < 0)
> - condlog(0, "%s: failed reload in new map update",
> mpp->alias);
> - return 0;
> -}
> -
> -struct multipath *add_map_without_path (struct vectors *vecs, char
> *alias)
> -{
> - struct multipath * mpp = alloc_multipath();
> - struct config *conf;
> -
> - if (!mpp)
> - return NULL;
> - if (!alias) {
> - FREE(mpp);
> - return NULL;
> - }
> -
> - mpp->alias = STRDUP(alias);
> -
> - if (dm_get_info(mpp->alias, &mpp->dmi)) {
> - condlog(3, "%s: cannot access table", mpp->alias);
> - goto out;
> - }
> - set_multipath_wwid(mpp);
> - conf = get_multipath_config();
> - mpp->mpe = find_mpe(conf->mptable, mpp->wwid);
> - put_multipath_config(conf);
> -
> - if (update_multipath_table(mpp, vecs->pathvec, 1))
> - goto out;
> - if (update_multipath_status(mpp))
> - goto out;
> -
> - if (!vector_alloc_slot(vecs->mpvec))
> - goto out;
> -
> - vector_set_slot(vecs->mpvec, mpp);
> -
> - if (update_map(mpp, vecs) != 0) /* map removed */
> - return NULL;
> -
> - if (start_waiter_thread(mpp, vecs))
> - goto out;
> -
> - return mpp;
> -out:
> - remove_map(mpp, vecs, PURGE_VEC);
> - return NULL;
> -}
> -
> static void
> find_existing_alias (struct multipath * mpp,
> struct vectors *vecs)
> diff --git a/libmultipath/structs_vec.h b/libmultipath/structs_vec.h
> index d6e17bb..ceab6d9 100644
> --- a/libmultipath/structs_vec.h
> +++ b/libmultipath/structs_vec.h
> @@ -26,12 +26,12 @@ int update_multipath_strings (struct multipath
> *mpp, vector pathvec,
> int is_daemon);
> 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_maps (struct vectors * vecs);
>
> void sync_map_state (struct multipath *);
> -int update_map (struct multipath *mpp, struct vectors *vecs);
> -struct multipath * add_map_without_path (struct vectors * vecs, char
> * alias);
> struct multipath * add_map_with_path (struct vectors * vecs,
> struct path * pp, int add_vec);
> int update_multipath (struct vectors *vecs, char *mapname, int
> reset);
> diff --git a/libmultipath/waiter.c b/libmultipath/waiter.c
> deleted file mode 100644
> index cb9708b..0000000
> --- a/libmultipath/waiter.c
> +++ /dev/null
> @@ -1,215 +0,0 @@
> -/*
> - * Copyright (c) 2004, 2005 Christophe Varoqui
> - * Copyright (c) 2005 Kiyoshi Ueda, NEC
> - * Copyright (c) 2005 Benjamin Marzinski, Redhat
> - * Copyright (c) 2005 Edward Goggin, EMC
> - */
> -#include <unistd.h>
> -#include <libdevmapper.h>
> -#include <sys/mman.h>
> -#include <pthread.h>
> -#include <signal.h>
> -#include <urcu.h>
> -
> -#include "vector.h"
> -#include "memory.h"
> -#include "checkers.h"
> -#include "config.h"
> -#include "structs.h"
> -#include "structs_vec.h"
> -#include "devmapper.h"
> -#include "debug.h"
> -#include "lock.h"
> -#include "waiter.h"
> -
> -pthread_attr_t waiter_attr;
> -
> -static struct event_thread *alloc_waiter (void)
> -{
> -
> - struct event_thread *wp;
> -
> - wp = (struct event_thread *)MALLOC(sizeof(struct
> event_thread));
> - memset(wp, 0, sizeof(struct event_thread));
> -
> - return wp;
> -}
> -
> -static void free_waiter (void *data)
> -{
> - struct event_thread *wp = (struct event_thread *)data;
> -
> - if (wp->dmt)
> - dm_task_destroy(wp->dmt);
> -
> - rcu_unregister_thread();
> - FREE(wp);
> -}
> -
> -void stop_waiter_thread (struct multipath *mpp, struct vectors
> *vecs)
> -{
> - pthread_t thread;
> -
> - if (mpp->waiter == (pthread_t)0) {
> - condlog(3, "%s: event checker thread already
> stopped",
> - mpp->alias);
> - return;
> - }
> - condlog(2, "%s: stop event checker thread (%lu)", mpp-
> >alias,
> - mpp->waiter);
> - thread = mpp->waiter;
> - mpp->waiter = (pthread_t)0;
> - pthread_cancel(thread);
> - pthread_kill(thread, SIGUSR2);
> -}
> -
> -/*
> - * returns the reschedule delay
> - * negative means *stop*
> - */
> -static int waiteventloop (struct event_thread *waiter)
> -{
> - sigset_t set, oldset;
> - int event_nr;
> - int r;
> -
> - if (!waiter->event_nr)
> - waiter->event_nr = dm_geteventnr(waiter->mapname);
> -
> - if (!(waiter->dmt =
> libmp_dm_task_create(DM_DEVICE_WAITEVENT))) {
> - condlog(0, "%s: devmap event #%i dm_task_create
> error",
> - waiter->mapname, waiter->event_nr);
> - return 1;
> - }
> -
> - if (!dm_task_set_name(waiter->dmt, waiter->mapname)) {
> - condlog(0, "%s: devmap event #%i dm_task_set_name
> error",
> - waiter->mapname, waiter->event_nr);
> - dm_task_destroy(waiter->dmt);
> - waiter->dmt = NULL;
> - return 1;
> - }
> -
> - if (waiter->event_nr && !dm_task_set_event_nr(waiter->dmt,
> - waiter-
> >event_nr)) {
> - condlog(0, "%s: devmap event #%i
> dm_task_set_event_nr error",
> - waiter->mapname, waiter->event_nr);
> - dm_task_destroy(waiter->dmt);
> - waiter->dmt = NULL;
> - return 1;
> - }
> -
> - dm_task_no_open_count(waiter->dmt);
> -
> - /* wait */
> - sigemptyset(&set);
> - sigaddset(&set, SIGUSR2);
> - pthread_sigmask(SIG_UNBLOCK, &set, &oldset);
> -
> - pthread_testcancel();
> - r = dm_task_run(waiter->dmt);
> - pthread_testcancel();
> -
> - pthread_sigmask(SIG_SETMASK, &oldset, NULL);
> - dm_task_destroy(waiter->dmt);
> - waiter->dmt = NULL;
> -
> - if (!r) /* wait interrupted by signal */
> - return -1;
> -
> - waiter->event_nr++;
> -
> - /*
> - * upon event ...
> - */
> - while (1) {
> - condlog(3, "%s: devmap event #%i",
> - waiter->mapname, waiter->event_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 = update_multipath(waiter->vecs, waiter->mapname,
> 1);
> - lock_cleanup_pop(waiter->vecs->lock);
> -
> - if (r) {
> - condlog(2, "%s: event checker exit",
> - waiter->mapname);
> - return -1; /* stop the thread */
> - }
> -
> - event_nr = dm_geteventnr(waiter->mapname);
> -
> - if (waiter->event_nr == event_nr)
> - return 1; /* upon problem reschedule 1s
> later */
> -
> - waiter->event_nr = event_nr;
> - }
> - return -1; /* never reach there */
> -}
> -
> -static void *waitevent (void *et)
> -{
> - int r;
> - struct event_thread *waiter;
> -
> - mlockall(MCL_CURRENT | MCL_FUTURE);
> -
> - waiter = (struct event_thread *)et;
> - pthread_cleanup_push(free_waiter, et);
> -
> - rcu_register_thread();
> - while (1) {
> - r = waiteventloop(waiter);
> -
> - if (r < 0)
> - break;
> -
> - sleep(r);
> - }
> -
> - pthread_cleanup_pop(1);
> - return NULL;
> -}
> -
> -int start_waiter_thread (struct multipath *mpp, struct vectors
> *vecs)
> -{
> - struct event_thread *wp;
> -
> - if (!mpp)
> - return 0;
> -
> - wp = alloc_waiter();
> -
> - if (!wp)
> - goto out;
> -
> - strncpy(wp->mapname, mpp->alias, WWID_SIZE - 1);
> - wp->vecs = vecs;
> -
> - if (pthread_create(&wp->thread, &waiter_attr, waitevent,
> wp)) {
> - condlog(0, "%s: cannot create event checker", wp-
> >mapname);
> - goto out1;
> - }
> - mpp->waiter = wp->thread;
> - condlog(2, "%s: event checker started", wp->mapname);
> -
> - return 0;
> -out1:
> - free_waiter(wp);
> - mpp->waiter = (pthread_t)0;
> -out:
> - condlog(0, "failed to start waiter thread");
> - return 1;
> -}
> diff --git a/libmultipath/waiter.h b/libmultipath/waiter.h
> deleted file mode 100644
> index 0cfae46..0000000
> --- a/libmultipath/waiter.h
> +++ /dev/null
> @@ -1,17 +0,0 @@
> -#ifndef _WAITER_H
> -#define _WAITER_H
> -
> -extern pthread_attr_t waiter_attr;
> -
> -struct event_thread {
> - struct dm_task *dmt;
> - pthread_t thread;
> - int event_nr;
> - char mapname[WWID_SIZE];
> - struct vectors *vecs;
> -};
> -
> -void stop_waiter_thread (struct multipath *mpp, struct vectors
> *vecs);
> -int start_waiter_thread (struct multipath *mpp, struct vectors
> *vecs);
> -
> -#endif /* _WAITER_H */
> diff --git a/multipathd/Makefile b/multipathd/Makefile
> index e6f140b..85f29a7 100644
> --- a/multipathd/Makefile
> +++ b/multipathd/Makefile
> @@ -22,7 +22,7 @@ ifdef SYSTEMD
> endif
> endif
>
> -OBJS = main.o pidfile.o uxlsnr.o uxclnt.o cli.o cli_handlers.o
> +OBJS = main.o pidfile.o uxlsnr.o uxclnt.o cli.o cli_handlers.o
> waiter.o
>
> EXEC = multipathd
>
> diff --git a/multipathd/main.c b/multipathd/main.c
> index 72c3c2f..94b2406 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -311,6 +311,102 @@ remove_maps_and_stop_waiters(struct vectors
> *vecs)
> remove_maps(vecs);
> }
>
> +static void
> +set_multipath_wwid (struct multipath * mpp)
> +{
> + if (strlen(mpp->wwid))
> + return;
> +
> + dm_get_uuid(mpp->alias, mpp->wwid);
> +}
> +
> +static int
> +update_map (struct multipath *mpp, struct vectors *vecs)
> +{
> + int retries = 3;
> + char params[PARAMS_SIZE] = {0};
> +
> +retry:
> + condlog(4, "%s: updating new map", mpp->alias);
> + if (adopt_paths(vecs->pathvec, mpp)) {
> + condlog(0, "%s: failed to adopt paths for new map
> update",
> + mpp->alias);
> + retries = -1;
> + goto fail;
> + }
> + verify_paths(mpp, vecs);
> + mpp->action = ACT_RELOAD;
> +
> + extract_hwe_from_path(mpp);
> + if (setup_map(mpp, params, PARAMS_SIZE)) {
> + condlog(0, "%s: failed to setup new map in update",
> mpp->alias);
> + retries = -1;
> + goto fail;
> + }
> + if (domap(mpp, params, 1) <= 0 && retries-- > 0) {
> + condlog(0, "%s: map_udate sleep", mpp->alias);
> + sleep(1);
> + goto retry;
> + }
> + dm_lib_release();
> +
> +fail:
> + if (setup_multipath(vecs, mpp))
> + return 1;
> +
> + sync_map_state(mpp);
> +
> + if (retries < 0)
> + condlog(0, "%s: failed reload in new map update",
> mpp->alias);
> + return 0;
> +}
> +
> +static struct multipath *
> +add_map_without_path (struct vectors *vecs, char *alias)
> +{
> + struct multipath * mpp = alloc_multipath();
> + struct config *conf;
> +
> + if (!mpp)
> + return NULL;
> + if (!alias) {
> + FREE(mpp);
> + return NULL;
> + }
> +
> + mpp->alias = STRDUP(alias);
> +
> + if (dm_get_info(mpp->alias, &mpp->dmi)) {
> + condlog(3, "%s: cannot access table", mpp->alias);
> + goto out;
> + }
> + set_multipath_wwid(mpp);
> + conf = get_multipath_config();
> + mpp->mpe = find_mpe(conf->mptable, mpp->wwid);
> + put_multipath_config(conf);
> +
> + if (update_multipath_table(mpp, vecs->pathvec, 1))
> + goto out;
> + if (update_multipath_status(mpp))
> + goto out;
> +
> + if (!vector_alloc_slot(vecs->mpvec))
> + goto out;
> +
> + vector_set_slot(vecs->mpvec, mpp);
> +
> + if (update_map(mpp, vecs) != 0) /* map removed */
> + return NULL;
> +
> + if (start_waiter_thread(mpp, vecs))
> + goto out;
> +
> + return mpp;
> +out:
> + remove_map(mpp, vecs, PURGE_VEC);
> + return NULL;
> +}
> +
> static int
> coalesce_maps(struct vectors *vecs, vector nmpv)
> {
> diff --git a/multipathd/waiter.c b/multipathd/waiter.c
> new file mode 100644
> index 0000000..cb9708b
> --- /dev/null
> +++ b/multipathd/waiter.c
> @@ -0,0 +1,215 @@
> +/*
> + * Copyright (c) 2004, 2005 Christophe Varoqui
> + * Copyright (c) 2005 Kiyoshi Ueda, NEC
> + * Copyright (c) 2005 Benjamin Marzinski, Redhat
> + * Copyright (c) 2005 Edward Goggin, EMC
> + */
> +#include <unistd.h>
> +#include <libdevmapper.h>
> +#include <sys/mman.h>
> +#include <pthread.h>
> +#include <signal.h>
> +#include <urcu.h>
> +
> +#include "vector.h"
> +#include "memory.h"
> +#include "checkers.h"
> +#include "config.h"
> +#include "structs.h"
> +#include "structs_vec.h"
> +#include "devmapper.h"
> +#include "debug.h"
> +#include "lock.h"
> +#include "waiter.h"
> +
> +pthread_attr_t waiter_attr;
> +
> +static struct event_thread *alloc_waiter (void)
> +{
> +
> + struct event_thread *wp;
> +
> + wp = (struct event_thread *)MALLOC(sizeof(struct
> event_thread));
> + memset(wp, 0, sizeof(struct event_thread));
> +
> + return wp;
> +}
> +
> +static void free_waiter (void *data)
> +{
> + struct event_thread *wp = (struct event_thread *)data;
> +
> + if (wp->dmt)
> + dm_task_destroy(wp->dmt);
> +
> + rcu_unregister_thread();
> + FREE(wp);
> +}
> +
> +void stop_waiter_thread (struct multipath *mpp, struct vectors
> *vecs)
> +{
> + pthread_t thread;
> +
> + if (mpp->waiter == (pthread_t)0) {
> + condlog(3, "%s: event checker thread already
> stopped",
> + mpp->alias);
> + return;
> + }
> + condlog(2, "%s: stop event checker thread (%lu)", mpp-
> >alias,
> + mpp->waiter);
> + thread = mpp->waiter;
> + mpp->waiter = (pthread_t)0;
> + pthread_cancel(thread);
> + pthread_kill(thread, SIGUSR2);
> +}
> +
> +/*
> + * returns the reschedule delay
> + * negative means *stop*
> + */
> +static int waiteventloop (struct event_thread *waiter)
> +{
> + sigset_t set, oldset;
> + int event_nr;
> + int r;
> +
> + if (!waiter->event_nr)
> + waiter->event_nr = dm_geteventnr(waiter->mapname);
> +
> + if (!(waiter->dmt =
> libmp_dm_task_create(DM_DEVICE_WAITEVENT))) {
> + condlog(0, "%s: devmap event #%i dm_task_create
> error",
> + waiter->mapname, waiter->event_nr);
> + return 1;
> + }
> +
> + if (!dm_task_set_name(waiter->dmt, waiter->mapname)) {
> + condlog(0, "%s: devmap event #%i dm_task_set_name
> error",
> + waiter->mapname, waiter->event_nr);
> + dm_task_destroy(waiter->dmt);
> + waiter->dmt = NULL;
> + return 1;
> + }
> +
> + if (waiter->event_nr && !dm_task_set_event_nr(waiter->dmt,
> + waiter-
> >event_nr)) {
> + condlog(0, "%s: devmap event #%i
> dm_task_set_event_nr error",
> + waiter->mapname, waiter->event_nr);
> + dm_task_destroy(waiter->dmt);
> + waiter->dmt = NULL;
> + return 1;
> + }
> +
> + dm_task_no_open_count(waiter->dmt);
> +
> + /* wait */
> + sigemptyset(&set);
> + sigaddset(&set, SIGUSR2);
> + pthread_sigmask(SIG_UNBLOCK, &set, &oldset);
> +
> + pthread_testcancel();
> + r = dm_task_run(waiter->dmt);
> + pthread_testcancel();
> +
> + pthread_sigmask(SIG_SETMASK, &oldset, NULL);
> + dm_task_destroy(waiter->dmt);
> + waiter->dmt = NULL;
> +
> + if (!r) /* wait interrupted by signal */
> + return -1;
> +
> + waiter->event_nr++;
> +
> + /*
> + * upon event ...
> + */
> + while (1) {
> + condlog(3, "%s: devmap event #%i",
> + waiter->mapname, waiter->event_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 = update_multipath(waiter->vecs, waiter->mapname,
> 1);
> + lock_cleanup_pop(waiter->vecs->lock);
> +
> + if (r) {
> + condlog(2, "%s: event checker exit",
> + waiter->mapname);
> + return -1; /* stop the thread */
> + }
> +
> + event_nr = dm_geteventnr(waiter->mapname);
> +
> + if (waiter->event_nr == event_nr)
> + return 1; /* upon problem reschedule 1s
> later */
> +
> + waiter->event_nr = event_nr;
> + }
> + return -1; /* never reach there */
> +}
> +
> +static void *waitevent (void *et)
> +{
> + int r;
> + struct event_thread *waiter;
> +
> + mlockall(MCL_CURRENT | MCL_FUTURE);
> +
> + waiter = (struct event_thread *)et;
> + pthread_cleanup_push(free_waiter, et);
> +
> + rcu_register_thread();
> + while (1) {
> + r = waiteventloop(waiter);
> +
> + if (r < 0)
> + break;
> +
> + sleep(r);
> + }
> +
> + pthread_cleanup_pop(1);
> + return NULL;
> +}
> +
> +int start_waiter_thread (struct multipath *mpp, struct vectors
> *vecs)
> +{
> + struct event_thread *wp;
> +
> + if (!mpp)
> + return 0;
> +
> + wp = alloc_waiter();
> +
> + if (!wp)
> + goto out;
> +
> + strncpy(wp->mapname, mpp->alias, WWID_SIZE - 1);
> + wp->vecs = vecs;
> +
> + if (pthread_create(&wp->thread, &waiter_attr, waitevent,
> wp)) {
> + condlog(0, "%s: cannot create event checker", wp-
> >mapname);
> + goto out1;
> + }
> + mpp->waiter = wp->thread;
> + condlog(2, "%s: event checker started", wp->mapname);
> +
> + return 0;
> +out1:
> + free_waiter(wp);
> + mpp->waiter = (pthread_t)0;
> +out:
> + condlog(0, "failed to start waiter thread");
> + return 1;
> +}
> diff --git a/multipathd/waiter.h b/multipathd/waiter.h
> new file mode 100644
> index 0000000..0cfae46
> --- /dev/null
> +++ b/multipathd/waiter.h
> @@ -0,0 +1,17 @@
> +#ifndef _WAITER_H
> +#define _WAITER_H
> +
> +extern pthread_attr_t waiter_attr;
> +
> +struct event_thread {
> + struct dm_task *dmt;
> + pthread_t thread;
> + int event_nr;
> + char mapname[WWID_SIZE];
> + struct vectors *vecs;
> +};
> +
> +void stop_waiter_thread (struct multipath *mpp, struct vectors
> *vecs);
> +int start_waiter_thread (struct multipath *mpp, struct vectors
> *vecs);
> +
> +#endif /* _WAITER_H */
--
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFC PATCH 3/5] call start_waiter_thread() before setup_multipath()
2018-02-10 5:07 [RFC PATCH 0/5] alternate dmevents waiter method Benjamin Marzinski
2018-02-10 5:07 ` [RFC PATCH 1/5] libmultipath: move remove_map waiter code to multipathd Benjamin Marzinski
2018-02-10 5:07 ` [RFC PATCH 2/5] move waiter code from libmultipath " Benjamin Marzinski
@ 2018-02-10 5:07 ` Benjamin Marzinski
2018-02-10 17:43 ` Martin Wilck
2018-02-10 5:07 ` [RFC PATCH 4/5] libmultipath: add helper functions Benjamin Marzinski
` (2 subsequent siblings)
5 siblings, 1 reply; 19+ messages in thread
From: Benjamin Marzinski @ 2018-02-10 5:07 UTC (permalink / raw)
To: device-mapper development; +Cc: Martin Wilck
If setup_multipath() is called before the waiter thread has started,
there is a window where a dm event can occur between when
setup_multipath() updates the device state and when the waiter thread
starts waiting for new events, causing the new event to be missed and
the multipath device to not get updated.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
multipathd/main.c | 37 ++++++++++++++++++++-----------------
1 file changed, 20 insertions(+), 17 deletions(-)
diff --git a/multipathd/main.c b/multipathd/main.c
index 94b2406..efc39d7 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -321,7 +321,7 @@ set_multipath_wwid (struct multipath * mpp)
}
static int
-update_map (struct multipath *mpp, struct vectors *vecs)
+update_map (struct multipath *mpp, struct vectors *vecs, int new_map)
{
int retries = 3;
char params[PARAMS_SIZE] = {0};
@@ -351,6 +351,12 @@ retry:
dm_lib_release();
fail:
+ if (new_map && (retries < 0 || start_waiter_thread(mpp, vecs))) {
+ condlog(0, "%s: failed to create new map", mpp->alias);
+ remove_map(mpp, vecs, 1);
+ return 1;
+ }
+
if (setup_multipath(vecs, mpp))
return 1;
@@ -395,12 +401,9 @@ add_map_without_path (struct vectors *vecs, char *alias)
vector_set_slot(vecs->mpvec, mpp);
- if (update_map(mpp, vecs) != 0) /* map removed */
+ if (update_map(mpp, vecs, 1) != 0) /* map removed */
return NULL;
- if (start_waiter_thread(mpp, vecs))
- goto out;
-
return mpp;
out:
remove_map(mpp, vecs, PURGE_VEC);
@@ -554,7 +557,7 @@ ev_add_map (char * dev, char * alias, struct vectors * vecs)
if (mpp->wait_for_udev > 1) {
condlog(2, "%s: performing delayed actions",
mpp->alias);
- if (update_map(mpp, vecs))
+ if (update_map(mpp, vecs, 0))
/* setup multipathd removed the map */
return 1;
}
@@ -865,6 +868,11 @@ retry:
}
dm_lib_release();
+ if ((mpp->action == ACT_CREATE ||
+ (mpp->action == ACT_NOTHING && start_waiter && !mpp->waiter)) &&
+ start_waiter_thread(mpp, vecs))
+ goto fail_map;
+
/*
* update our state from kernel regardless of create or reload
*/
@@ -873,11 +881,6 @@ retry:
sync_map_state(mpp);
- if ((mpp->action == ACT_CREATE ||
- (mpp->action == ACT_NOTHING && start_waiter && !mpp->waiter)) &&
- start_waiter_thread(mpp, vecs))
- goto fail_map;
-
if (retries >= 0) {
condlog(2, "%s [%s]: path added to devmap %s",
pp->dev, pp->dev_t, mpp->alias);
@@ -1479,7 +1482,8 @@ missing_uev_wait_tick(struct vectors *vecs)
if (mpp->wait_for_udev && --mpp->uev_wait_tick <= 0) {
timed_out = 1;
condlog(0, "%s: timeout waiting on creation uevent. enabling reloads", mpp->alias);
- if (mpp->wait_for_udev > 1 && update_map(mpp, vecs)) {
+ if (mpp->wait_for_udev > 1 &&
+ update_map(mpp, vecs, 0)) {
/* update_map removed map */
i--;
continue;
@@ -1511,7 +1515,7 @@ ghost_delay_tick(struct vectors *vecs)
condlog(0, "%s: timed out waiting for active path",
mpp->alias);
mpp->force_udev_reload = 1;
- if (update_map(mpp, vecs) != 0) {
+ if (update_map(mpp, vecs, 0) != 0) {
/* update_map removed map */
i--;
continue;
@@ -2169,14 +2173,13 @@ configure (struct vectors * vecs)
* start dm event waiter threads for these new maps
*/
vector_foreach_slot(vecs->mpvec, mpp, i) {
- if (setup_multipath(vecs, mpp)) {
- i--;
- continue;
- }
if (start_waiter_thread(mpp, vecs)) {
remove_map(mpp, vecs, 1);
i--;
+ continue;
}
+ if (setup_multipath(vecs, mpp))
+ i--;
}
return 0;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [RFC PATCH 3/5] call start_waiter_thread() before setup_multipath()
2018-02-10 5:07 ` [RFC PATCH 3/5] call start_waiter_thread() before setup_multipath() Benjamin Marzinski
@ 2018-02-10 17:43 ` Martin Wilck
0 siblings, 0 replies; 19+ messages in thread
From: Martin Wilck @ 2018-02-10 17:43 UTC (permalink / raw)
To: Benjamin Marzinski, device-mapper development
On Fri, 2018-02-09 at 23:07 -0600, Benjamin Marzinski wrote:
> If setup_multipath() is called before the waiter thread has started,
> there is a window where a dm event can occur between when
> setup_multipath() updates the device state and when the waiter thread
> starts waiting for new events, causing the new event to be missed and
> the multipath device to not get updated.
>
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
The window will be still there, but smaller than before.
Reviewed-by: Martin Wilck <mwilck@suse.com>
> ---
> multipathd/main.c | 37 ++++++++++++++++++++-----------------
> 1 file changed, 20 insertions(+), 17 deletions(-)
>
> diff --git a/multipathd/main.c b/multipathd/main.c
> index 94b2406..efc39d7 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -321,7 +321,7 @@ set_multipath_wwid (struct multipath * mpp)
> }
>
> static int
> -update_map (struct multipath *mpp, struct vectors *vecs)
> +update_map (struct multipath *mpp, struct vectors *vecs, int
> new_map)
> {
> int retries = 3;
> char params[PARAMS_SIZE] = {0};
> @@ -351,6 +351,12 @@ retry:
> dm_lib_release();
>
> fail:
> + if (new_map && (retries < 0 || start_waiter_thread(mpp,
> vecs))) {
> + condlog(0, "%s: failed to create new map", mpp-
> >alias);
> + remove_map(mpp, vecs, 1);
> + return 1;
> + }
> +
> if (setup_multipath(vecs, mpp))
> return 1;
>
> @@ -395,12 +401,9 @@ add_map_without_path (struct vectors *vecs, char
> *alias)
>
> vector_set_slot(vecs->mpvec, mpp);
>
> - if (update_map(mpp, vecs) != 0) /* map removed */
> + if (update_map(mpp, vecs, 1) != 0) /* map removed */
> return NULL;
>
> - if (start_waiter_thread(mpp, vecs))
> - goto out;
> -
> return mpp;
> out:
> remove_map(mpp, vecs, PURGE_VEC);
> @@ -554,7 +557,7 @@ ev_add_map (char * dev, char * alias, struct
> vectors * vecs)
> if (mpp->wait_for_udev > 1) {
> condlog(2, "%s: performing delayed actions",
> mpp->alias);
> - if (update_map(mpp, vecs))
> + if (update_map(mpp, vecs, 0))
> /* setup multipathd removed the map
> */
> return 1;
> }
> @@ -865,6 +868,11 @@ retry:
> }
> dm_lib_release();
>
> + if ((mpp->action == ACT_CREATE ||
> + (mpp->action == ACT_NOTHING && start_waiter && !mpp-
> >waiter)) &&
> + start_waiter_thread(mpp, vecs))
> + goto fail_map;
> +
> /*
> * update our state from kernel regardless of create or
> reload
> */
> @@ -873,11 +881,6 @@ retry:
>
> sync_map_state(mpp);
>
> - if ((mpp->action == ACT_CREATE ||
> - (mpp->action == ACT_NOTHING && start_waiter && !mpp-
> >waiter)) &&
> - start_waiter_thread(mpp, vecs))
> - goto fail_map;
> -
> if (retries >= 0) {
> condlog(2, "%s [%s]: path added to devmap %s",
> pp->dev, pp->dev_t, mpp->alias);
> @@ -1479,7 +1482,8 @@ missing_uev_wait_tick(struct vectors *vecs)
> if (mpp->wait_for_udev && --mpp->uev_wait_tick <= 0)
> {
> timed_out = 1;
> condlog(0, "%s: timeout waiting on creation
> uevent. enabling reloads", mpp->alias);
> - if (mpp->wait_for_udev > 1 &&
> update_map(mpp, vecs)) {
> + if (mpp->wait_for_udev > 1 &&
> + update_map(mpp, vecs, 0)) {
> /* update_map removed map */
> i--;
> continue;
> @@ -1511,7 +1515,7 @@ ghost_delay_tick(struct vectors *vecs)
> condlog(0, "%s: timed out waiting for active
> path",
> mpp->alias);
> mpp->force_udev_reload = 1;
> - if (update_map(mpp, vecs) != 0) {
> + if (update_map(mpp, vecs, 0) != 0) {
> /* update_map removed map */
> i--;
> continue;
> @@ -2169,14 +2173,13 @@ configure (struct vectors * vecs)
> * start dm event waiter threads for these new maps
> */
> vector_foreach_slot(vecs->mpvec, mpp, i) {
> - if (setup_multipath(vecs, mpp)) {
> - i--;
> - continue;
> - }
> if (start_waiter_thread(mpp, vecs)) {
> remove_map(mpp, vecs, 1);
> i--;
> + continue;
> }
> + if (setup_multipath(vecs, mpp))
> + i--;
> }
> return 0;
> }
--
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFC PATCH 4/5] libmultipath: add helper functions
2018-02-10 5:07 [RFC PATCH 0/5] alternate dmevents waiter method Benjamin Marzinski
` (2 preceding siblings ...)
2018-02-10 5:07 ` [RFC PATCH 3/5] call start_waiter_thread() before setup_multipath() Benjamin Marzinski
@ 2018-02-10 5:07 ` Benjamin Marzinski
2018-02-10 19:12 ` Martin Wilck
2018-02-10 5:07 ` [RFC PATCH 5/5] multipathd: RFC add new polling dmevents waiter thread Benjamin Marzinski
2018-03-08 19:59 ` [RFC PATCH 0/5] alternate dmevents waiter method Xose Vazquez Perez
5 siblings, 1 reply; 19+ messages in thread
From: Benjamin Marzinski @ 2018-02-10 5:07 UTC (permalink / raw)
To: device-mapper development; +Cc: Martin Wilck
Add the ability to reset a vector without completely freeing it, and to
check the version of the device-mapper module. The existing version
checking code checks the version of a specific device mapper target, and
has been renamed for clarity's sake. These functions will be used in a
later patch.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
libmultipath/devmapper.c | 28 ++++++++++++++++++++++++----
libmultipath/devmapper.h | 3 ++-
libmultipath/vector.c | 16 ++++++++++++----
libmultipath/vector.h | 1 +
multipathd/main.c | 2 +-
5 files changed, 40 insertions(+), 10 deletions(-)
diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index 573fc75..2960bf5 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -132,7 +132,27 @@ dm_lib_prereq (void)
}
int
-dm_drv_version (unsigned int * version, char * str)
+dm_drv_version(unsigned int *v)
+{
+ char buff[64];
+
+ v[0] = 0;
+ v[1] = 0;
+ v[2] = 0;
+
+ if (!dm_driver_version(buff, sizeof(buff))) {
+ condlog(0, "cannot get kernel dm version");
+ return 1;
+ }
+ if (sscanf(buff, "%u.%u.%u ", &v[0], &v[1], &v[2]) != 3) {
+ condlog(0, "invalid kernel dm version '%s'", buff);
+ return 1;
+ }
+ return 0;
+}
+
+int
+dm_tgt_version (unsigned int * version, char * str)
{
int r = 2;
struct dm_task *dmt;
@@ -179,13 +199,13 @@ out:
}
static int
-dm_drv_prereq (unsigned int *ver)
+dm_tgt_prereq (unsigned int *ver)
{
unsigned int minv[3] = {1, 0, 3};
unsigned int version[3] = {0, 0, 0};
unsigned int * v = version;
- if (dm_drv_version(v, TGT_MPATH)) {
+ if (dm_tgt_version(v, TGT_MPATH)) {
/* in doubt return not capable */
return 1;
}
@@ -210,7 +230,7 @@ static int dm_prereq(unsigned int *v)
{
if (dm_lib_prereq())
return 1;
- return dm_drv_prereq(v);
+ return dm_tgt_prereq(v);
}
static int libmp_dm_udev_sync = 0;
diff --git a/libmultipath/devmapper.h b/libmultipath/devmapper.h
index 62e14d1..52d4af8 100644
--- a/libmultipath/devmapper.h
+++ b/libmultipath/devmapper.h
@@ -28,7 +28,8 @@ void dm_init(int verbosity);
void libmp_dm_init(void);
void libmp_udev_set_sync_support(int on);
struct dm_task *libmp_dm_task_create(int task);
-int dm_drv_version (unsigned int * version, char * str);
+int dm_drv_version (unsigned int * version);
+int dm_tgt_version (unsigned int * version, char * str);
int dm_simplecmd_flush (int, const char *, uint16_t);
int dm_simplecmd_noflush (int, const char *, uint16_t);
int dm_addmap_create (struct multipath *mpp, char *params);
diff --git a/libmultipath/vector.c b/libmultipath/vector.c
index 6266e0a..f741ae0 100644
--- a/libmultipath/vector.c
+++ b/libmultipath/vector.c
@@ -145,18 +145,26 @@ vector_repack(vector v)
vector_del_slot(v, i--);
}
-/* Free memory vector allocation */
-void
-vector_free(vector v)
+vector
+vector_reset(vector v)
{
if (!v)
- return;
+ return NULL;
if (v->slot)
FREE(v->slot);
v->allocated = 0;
v->slot = NULL;
+ return v;
+}
+
+/* Free memory vector allocation */
+void
+vector_free(vector v)
+{
+ if (!vector_reset(v))
+ return;
FREE(v);
}
diff --git a/libmultipath/vector.h b/libmultipath/vector.h
index 5cfd4d0..d69cd0b 100644
--- a/libmultipath/vector.h
+++ b/libmultipath/vector.h
@@ -45,6 +45,7 @@ typedef struct _vector *vector;
/* Prototypes */
extern vector vector_alloc(void);
extern void *vector_alloc_slot(vector v);
+vector vector_reset(vector v);
extern void vector_free(vector v);
extern void free_strvec(vector strvec);
extern void vector_set_slot(vector v, void *value);
diff --git a/multipathd/main.c b/multipathd/main.c
index efc39d7..2963bde 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -2228,7 +2228,7 @@ reconfigure (struct vectors * vecs)
/* Re-read any timezone changes */
tzset();
- dm_drv_version(conf->version, TGT_MPATH);
+ dm_tgt_version(conf->version, TGT_MPATH);
if (verbosity)
conf->verbosity = verbosity;
if (bindings_read_only)
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [RFC PATCH 4/5] libmultipath: add helper functions
2018-02-10 5:07 ` [RFC PATCH 4/5] libmultipath: add helper functions Benjamin Marzinski
@ 2018-02-10 19:12 ` Martin Wilck
0 siblings, 0 replies; 19+ messages in thread
From: Martin Wilck @ 2018-02-10 19:12 UTC (permalink / raw)
To: Benjamin Marzinski, device-mapper development
On Fri, 2018-02-09 at 23:07 -0600, Benjamin Marzinski wrote:
> Add the ability to reset a vector without completely freeing it, and
> to
> check the version of the device-mapper module. The existing version
> checking code checks the version of a specific device mapper target,
> and
> has been renamed for clarity's sake. These functions will be used in
> a
> later patch.
>
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
Reviewed-by: Martin Wilck <mwilck@suse.com>
> libmultipath/devmapper.c | 28 ++++++++++++++++++++++++----
> libmultipath/devmapper.h | 3 ++-
> libmultipath/vector.c | 16 ++++++++++++----
> libmultipath/vector.h | 1 +
> multipathd/main.c | 2 +-
> 5 files changed, 40 insertions(+), 10 deletions(-)
>
> diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
> index 573fc75..2960bf5 100644
> --- a/libmultipath/devmapper.c
> +++ b/libmultipath/devmapper.c
> @@ -132,7 +132,27 @@ dm_lib_prereq (void)
> }
>
> int
> -dm_drv_version (unsigned int * version, char * str)
> +dm_drv_version(unsigned int *v)
> +{
> + char buff[64];
> +
> + v[0] = 0;
> + v[1] = 0;
> + v[2] = 0;
> +
> + if (!dm_driver_version(buff, sizeof(buff))) {
> + condlog(0, "cannot get kernel dm version");
> + return 1;
> + }
> + if (sscanf(buff, "%u.%u.%u ", &v[0], &v[1], &v[2]) != 3) {
> + condlog(0, "invalid kernel dm version '%s'", buff);
> + return 1;
> + }
> + return 0;
> +}
> +
> +int
> +dm_tgt_version (unsigned int * version, char * str)
> {
> int r = 2;
> struct dm_task *dmt;
> @@ -179,13 +199,13 @@ out:
> }
>
> static int
> -dm_drv_prereq (unsigned int *ver)
> +dm_tgt_prereq (unsigned int *ver)
> {
> unsigned int minv[3] = {1, 0, 3};
> unsigned int version[3] = {0, 0, 0};
> unsigned int * v = version;
>
> - if (dm_drv_version(v, TGT_MPATH)) {
> + if (dm_tgt_version(v, TGT_MPATH)) {
> /* in doubt return not capable */
> return 1;
> }
> @@ -210,7 +230,7 @@ static int dm_prereq(unsigned int *v)
> {
> if (dm_lib_prereq())
> return 1;
> - return dm_drv_prereq(v);
> + return dm_tgt_prereq(v);
> }
>
> static int libmp_dm_udev_sync = 0;
> diff --git a/libmultipath/devmapper.h b/libmultipath/devmapper.h
> index 62e14d1..52d4af8 100644
> --- a/libmultipath/devmapper.h
> +++ b/libmultipath/devmapper.h
> @@ -28,7 +28,8 @@ void dm_init(int verbosity);
> void libmp_dm_init(void);
> void libmp_udev_set_sync_support(int on);
> struct dm_task *libmp_dm_task_create(int task);
> -int dm_drv_version (unsigned int * version, char * str);
> +int dm_drv_version (unsigned int * version);
> +int dm_tgt_version (unsigned int * version, char * str);
> int dm_simplecmd_flush (int, const char *, uint16_t);
> int dm_simplecmd_noflush (int, const char *, uint16_t);
> int dm_addmap_create (struct multipath *mpp, char *params);
> diff --git a/libmultipath/vector.c b/libmultipath/vector.c
> index 6266e0a..f741ae0 100644
> --- a/libmultipath/vector.c
> +++ b/libmultipath/vector.c
> @@ -145,18 +145,26 @@ vector_repack(vector v)
> vector_del_slot(v, i--);
> }
>
> -/* Free memory vector allocation */
> -void
> -vector_free(vector v)
> +vector
> +vector_reset(vector v)
> {
> if (!v)
> - return;
> + return NULL;
>
> if (v->slot)
> FREE(v->slot);
>
> v->allocated = 0;
> v->slot = NULL;
> + return v;
> +}
> +
> +/* Free memory vector allocation */
> +void
> +vector_free(vector v)
> +{
> + if (!vector_reset(v))
> + return;
> FREE(v);
> }
>
> diff --git a/libmultipath/vector.h b/libmultipath/vector.h
> index 5cfd4d0..d69cd0b 100644
> --- a/libmultipath/vector.h
> +++ b/libmultipath/vector.h
> @@ -45,6 +45,7 @@ typedef struct _vector *vector;
> /* Prototypes */
> extern vector vector_alloc(void);
> extern void *vector_alloc_slot(vector v);
> +vector vector_reset(vector v);
> extern void vector_free(vector v);
> extern void free_strvec(vector strvec);
> extern void vector_set_slot(vector v, void *value);
> diff --git a/multipathd/main.c b/multipathd/main.c
> index efc39d7..2963bde 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -2228,7 +2228,7 @@ reconfigure (struct vectors * vecs)
> /* Re-read any timezone changes */
> tzset();
>
> - dm_drv_version(conf->version, TGT_MPATH);
> + dm_tgt_version(conf->version, TGT_MPATH);
> if (verbosity)
> conf->verbosity = verbosity;
> if (bindings_read_only)
--
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFC PATCH 5/5] multipathd: RFC add new polling dmevents waiter thread
2018-02-10 5:07 [RFC PATCH 0/5] alternate dmevents waiter method Benjamin Marzinski
` (3 preceding siblings ...)
2018-02-10 5:07 ` [RFC PATCH 4/5] libmultipath: add helper functions Benjamin Marzinski
@ 2018-02-10 5:07 ` Benjamin Marzinski
2018-02-10 19:55 ` Martin Wilck
2018-03-08 19:59 ` [RFC PATCH 0/5] alternate dmevents waiter method Xose Vazquez Perez
5 siblings, 1 reply; 19+ messages in thread
From: Benjamin Marzinski @ 2018-02-10 5:07 UTC (permalink / raw)
To: device-mapper development; +Cc: Martin Wilck
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>
---
multipathd/Makefile | 3 +-
multipathd/dmevents.c | 396 ++++++++++++++++++++++++++++++++++++++++++++++++++
multipathd/dmevents.h | 13 ++
multipathd/main.c | 58 +++++++-
4 files changed, 461 insertions(+), 9 deletions(-)
create mode 100644 multipathd/dmevents.c
create mode 100644 multipathd/dmevents.h
diff --git a/multipathd/Makefile b/multipathd/Makefile
index 85f29a7..4c438f0 100644
--- a/multipathd/Makefile
+++ b/multipathd/Makefile
@@ -22,7 +22,8 @@ ifdef SYSTEMD
endif
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..a56c055
--- /dev/null
+++ b/multipathd/dmevents.c
@@ -0,0 +1,396 @@
+/*
+ * 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 alloc_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 free_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;
+ struct multipath *mpp;
+
+ 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) {
+ mpp = find_mp_by_alias(waiter->vecs->mpvec,
+ curr_dev.name);
+ if (mpp)
+ remove_map(mpp, 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..569e855
--- /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 alloc_dmevent_waiter(struct vectors *vecs);
+void free_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 2963bde..6dabf2c 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 "../third-party/valgrind/drd.h"
@@ -108,6 +109,7 @@ int uxsock_timeout;
int verbosity;
int bindings_read_only;
int ignore_new_devs;
+int poll_dmevents = 1;
enum daemon_status running_state = DAEMON_INIT;
pid_t daemon_pid;
pthread_mutex_t config_lock = PTHREAD_MUTEX_INITIALIZER;
@@ -288,11 +290,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);
}
@@ -305,8 +319,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);
}
@@ -351,7 +369,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;
@@ -870,7 +888,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;
/*
@@ -2173,7 +2191,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;
@@ -2414,7 +2432,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;
@@ -2476,6 +2494,8 @@ child (void * param)
goto failed;
}
+ if (poll_dmevents)
+ poll_dmevents = dmevent_poll_supported();
setlogmask(LOG_UPTO(conf->verbosity + 3));
envp = getenv("LimitNOFILE");
@@ -2542,6 +2562,19 @@ child (void * param)
init_path_check_interval(vecs);
+ if (poll_dmevents) {
+ if (alloc_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
*/
@@ -2615,11 +2648,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();
@@ -2634,6 +2671,8 @@ child (void * param)
cleanup_checkers();
cleanup_prio();
+ if (poll_dmevents)
+ free_dmevent_waiter();
dm_lib_release();
dm_lib_exit();
@@ -2765,7 +2804,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;
@@ -2799,6 +2838,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
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [RFC PATCH 5/5] multipathd: RFC add new polling dmevents waiter thread
2018-02-10 5:07 ` [RFC PATCH 5/5] multipathd: RFC add new polling dmevents waiter thread Benjamin Marzinski
@ 2018-02-10 19:55 ` Martin Wilck
2018-02-12 23:18 ` Benjamin Marzinski
0 siblings, 1 reply; 19+ messages in thread
From: Martin Wilck @ 2018-02-10 19:55 UTC (permalink / raw)
To: Benjamin Marzinski, device-mapper development
Hi Ben,
thanks a lot for this. I have only a few minor nitpicks (see below).
I suppose you've tested this already?
Regards
Martin
On Fri, 2018-02-09 at 23:07 -0600, Benjamin Marzinski wrote:
> 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.
Why use a command line option here rather than a config file option?
>
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
> multipathd/Makefile | 3 +-
> multipathd/dmevents.c | 396
> ++++++++++++++++++++++++++++++++++++++++++++++++++
> multipathd/dmevents.h | 13 ++
> multipathd/main.c | 58 +++++++-
> 4 files changed, 461 insertions(+), 9 deletions(-)
> create mode 100644 multipathd/dmevents.c
> create mode 100644 multipathd/dmevents.h
>
> diff --git a/multipathd/Makefile b/multipathd/Makefile
> index 85f29a7..4c438f0 100644
> --- a/multipathd/Makefile
> +++ b/multipathd/Makefile
> @@ -22,7 +22,8 @@ ifdef SYSTEMD
> endif
> 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..a56c055
> --- /dev/null
> +++ b/multipathd/dmevents.c
> @@ -0,0 +1,396 @@
> +/*
> + * 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 alloc_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;
> +}
Nitpick: conventionally, an "alloc"-type function would return the
pointer, and NULL on failure.
> +
> +void free_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;
> +}
Nitpick: Similarly, a "free" function typically takes the pointer to be
freed as argument.
> +
> +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;
What's the meaning of this flag? I couldn't find it in dm-ioctl.h
> + 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;
Nitpick: It might be better to use strlcpy or snprintf here.
> + 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;
> + struct multipath *mpp;
> +
> + 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) {
> + mpp = find_mp_by_alias(waiter->vecs->mpvec,
> + curr_dev.name);
> + if (mpp)
> + remove_map(mpp, 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..569e855
> --- /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 alloc_dmevent_waiter(struct vectors *vecs);
> +void free_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 2963bde..6dabf2c 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 "../third-party/valgrind/drd.h"
> @@ -108,6 +109,7 @@ int uxsock_timeout;
> int verbosity;
> int bindings_read_only;
> int ignore_new_devs;
> +int poll_dmevents = 1;
> enum daemon_status running_state = DAEMON_INIT;
> pid_t daemon_pid;
> pthread_mutex_t config_lock = PTHREAD_MUTEX_INITIALIZER;
> @@ -288,11 +290,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);
> }
>
> @@ -305,8 +319,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);
> }
> @@ -351,7 +369,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;
> @@ -870,7 +888,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;
>
> /*
> @@ -2173,7 +2191,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;
> @@ -2414,7 +2432,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;
> @@ -2476,6 +2494,8 @@ child (void * param)
> goto failed;
> }
>
> + if (poll_dmevents)
> + poll_dmevents = dmevent_poll_supported();
> setlogmask(LOG_UPTO(conf->verbosity + 3));
>
> envp = getenv("LimitNOFILE");
> @@ -2542,6 +2562,19 @@ child (void * param)
>
> init_path_check_interval(vecs);
>
> + if (poll_dmevents) {
> + if (alloc_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
> */
> @@ -2615,11 +2648,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();
>
> @@ -2634,6 +2671,8 @@ child (void * param)
>
> cleanup_checkers();
> cleanup_prio();
> + if (poll_dmevents)
> + free_dmevent_waiter();
>
> dm_lib_release();
> dm_lib_exit();
> @@ -2765,7 +2804,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;
> @@ -2799,6 +2838,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);
--
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC PATCH 5/5] multipathd: RFC add new polling dmevents waiter thread
2018-02-10 19:55 ` Martin Wilck
@ 2018-02-12 23:18 ` Benjamin Marzinski
2018-02-13 1:13 ` Alasdair G Kergon
2018-02-13 8:50 ` Martin Wilck
0 siblings, 2 replies; 19+ messages in thread
From: Benjamin Marzinski @ 2018-02-12 23:18 UTC (permalink / raw)
To: Martin Wilck; +Cc: device-mapper development
On Sat, Feb 10, 2018 at 08:55:53PM +0100, Martin Wilck wrote:
> Hi Ben,
>
> thanks a lot for this. I have only a few minor nitpicks (see below).
> I suppose you've tested this already?
Yes. I do plan on doing some more testing after I look into making
libdevmapper support re-arming the polling interface and grabbing the
event number from the names listing, before I repost this without the
RFC tag. I was also thinking of trying out cmocka by mocking up a
device-mapper interface that let me test this code in isolation.
>
> Regards
> Martin
>
> On Fri, 2018-02-09 at 23:07 -0600, Benjamin Marzinski wrote:
> > 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.
>
> Why use a command line option here rather than a config file option?
Mostly because it was faster, and I wanted to get to testing it. The
other reason is that I don't see any benefit for the work involved in
making this be changeable in
# multipathd reconfigure
However, we already have configuration settings that can't get changed
on reconfigure, so making this another one is not a big deal. I agree
that it is easier for users to change if it is a configuration setting,
but I'm hoping that this change will be invisible to users. If you would
prefer it as a configuration setting, I have no problem with changing
that.
> >
> > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> > ---
> > multipathd/Makefile | 3 +-
> > multipathd/dmevents.c | 396
> > ++++++++++++++++++++++++++++++++++++++++++++++++++
> > multipathd/dmevents.h | 13 ++
> > multipathd/main.c | 58 +++++++-
> > 4 files changed, 461 insertions(+), 9 deletions(-)
> > create mode 100644 multipathd/dmevents.c
> > create mode 100644 multipathd/dmevents.h
> >
> > diff --git a/multipathd/Makefile b/multipathd/Makefile
> > index 85f29a7..4c438f0 100644
> > --- a/multipathd/Makefile
> > +++ b/multipathd/Makefile
> > @@ -22,7 +22,8 @@ ifdef SYSTEMD
> > endif
> > 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..a56c055
> > --- /dev/null
> > +++ b/multipathd/dmevents.c
> > @@ -0,0 +1,396 @@
> > +/*
> > + * 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 alloc_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;
> > +}
>
> Nitpick: conventionally, an "alloc"-type function would return the
> pointer, and NULL on failure.
Is this a naming complaint, or an interface complaint? I'm fine with
changing the names so they follow the lead of checkers and prio, i.e.
init_dmevents_waiter() and cleanup_dmevents_waiter(). The init and
cleanup functions for checkers and prio have the same returns as the
dmevent functions (well the init functions return 1 for failure, and I
can do that as well)
Since there is only ever going to be one dmevent waiter thread running,
I'm not sure what the rest of multipathd gains by returning a pointer to
its data structure. It would be extra work to make multipathd store and
pass this data structure on calls to watch or unwatch devices, for no
benefit that I can see. If you see a good reason for doing this, I'm
open to being convinced otherwise.
I was originally going to make this allocation and deallocation happen
when starting and shutting down the thread, but I through that not doing
this would allow me to create and join the threads directly in child(),
like most of the other threads. Also it would allow me to initialize the
data structure without having to start a thread, which seemed useful
for unit testing.
> > +
> > +void free_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;
> > +}
>
> Nitpick: Similarly, a "free" function typically takes the pointer to be
> freed as argument.
>
> > +
> > +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;
>
> What's the meaning of this flag? I couldn't find it in dm-ioctl.h
>
It is the DM_EXISTS_FLAG. It's defined in libdm/ioctl/libdm-iface.c in
the lvm2 source. It is unconditionally set on all dm control ioctls
from libdevmapper by _do_dm_ioctl() (also in libdm/ioctl/libdm-iface.c).
I don't know the reason for this. I don't see anything that uses it in
driver/md/dm-ioctl.c, and I see that line in the libdm source has a
/* FIXME */ next to it. On the other hand, all I'm trying to do here is
run the same ioctl that libdevmapper would if it supported this command,
and there may well be a reason for it that I'm missing.
> > + 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;
>
> Nitpick: It might be better to use strlcpy or snprintf here.
Sure.
> > + 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;
> > + struct multipath *mpp;
> > +
> > + 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) {
> > + mpp = find_mp_by_alias(waiter->vecs->mpvec,
> > + curr_dev.name);
> > + if (mpp)
> > + remove_map(mpp, 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..569e855
> > --- /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 alloc_dmevent_waiter(struct vectors *vecs);
> > +void free_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 2963bde..6dabf2c 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 "../third-party/valgrind/drd.h"
> > @@ -108,6 +109,7 @@ int uxsock_timeout;
> > int verbosity;
> > int bindings_read_only;
> > int ignore_new_devs;
> > +int poll_dmevents = 1;
> > enum daemon_status running_state = DAEMON_INIT;
> > pid_t daemon_pid;
> > pthread_mutex_t config_lock = PTHREAD_MUTEX_INITIALIZER;
> > @@ -288,11 +290,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);
> > }
> >
> > @@ -305,8 +319,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);
> > }
> > @@ -351,7 +369,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;
> > @@ -870,7 +888,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;
> >
> > /*
> > @@ -2173,7 +2191,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;
> > @@ -2414,7 +2432,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;
> > @@ -2476,6 +2494,8 @@ child (void * param)
> > goto failed;
> > }
> >
> > + if (poll_dmevents)
> > + poll_dmevents = dmevent_poll_supported();
> > setlogmask(LOG_UPTO(conf->verbosity + 3));
> >
> > envp = getenv("LimitNOFILE");
> > @@ -2542,6 +2562,19 @@ child (void * param)
> >
> > init_path_check_interval(vecs);
> >
> > + if (poll_dmevents) {
> > + if (alloc_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
> > */
> > @@ -2615,11 +2648,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();
> >
> > @@ -2634,6 +2671,8 @@ child (void * param)
> >
> > cleanup_checkers();
> > cleanup_prio();
> > + if (poll_dmevents)
> > + free_dmevent_waiter();
> >
> > dm_lib_release();
> > dm_lib_exit();
> > @@ -2765,7 +2804,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;
> > @@ -2799,6 +2838,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);
>
> --
> Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
> SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
> HRB 21284 (AG Nürnberg)
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC PATCH 5/5] multipathd: RFC add new polling dmevents waiter thread
2018-02-12 23:18 ` Benjamin Marzinski
@ 2018-02-13 1:13 ` Alasdair G Kergon
2018-02-13 8:50 ` Martin Wilck
1 sibling, 0 replies; 19+ messages in thread
From: Alasdair G Kergon @ 2018-02-13 1:13 UTC (permalink / raw)
To: Benjamin Marzinski; +Cc: device-mapper development, Martin Wilck
On Mon, Feb 12, 2018 at 05:18:02PM -0600, Benjamin Marzinski wrote:
> It is the DM_EXISTS_FLAG. It's defined in libdm/ioctl/libdm-iface.c in
> the lvm2 source. It is unconditionally set on all dm control ioctls
> from libdevmapper by _do_dm_ioctl() (also in libdm/ioctl/libdm-iface.c).
> I don't know the reason for this. I don't see anything that uses it in
> driver/md/dm-ioctl.c, and I see that line in the libdm source has a
> /* FIXME */ next to it. On the other hand, all I'm trying to do here is
> run the same ioctl that libdevmapper would if it supported this command,
> and there may well be a reason for it that I'm missing.
DM_EXISTS_FLAG indicates that the device the ioctl referenced exists.
If you performed certain queries and the device was not found, you
got a successful return but without this flag.
In the dim and distant past this flag was handled kernel-side. Later,
we dropped it but left that userspace code emulating it. The FIXMEs
were just saying that the library code could be cleaned up too one day.
Ref. https://lwn.net/Articles/38512/
Alasdair
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH 5/5] multipathd: RFC add new polling dmevents waiter thread
2018-02-12 23:18 ` Benjamin Marzinski
2018-02-13 1:13 ` Alasdair G Kergon
@ 2018-02-13 8:50 ` Martin Wilck
2018-02-13 16:49 ` Benjamin Marzinski
1 sibling, 1 reply; 19+ messages in thread
From: Martin Wilck @ 2018-02-13 8:50 UTC (permalink / raw)
To: Benjamin Marzinski; +Cc: device-mapper development
Hi Ben,
On Mon, 2018-02-12 at 17:18 -0600, Benjamin Marzinski wrote:
> On Sat, Feb 10, 2018 at 08:55:53PM +0100, Martin Wilck wrote:
> > Hi Ben,
> >
> > thanks a lot for this. I have only a few minor nitpicks (see
> > below).
> > I suppose you've tested this already?
>
> Yes. I do plan on doing some more testing after I look into making
> libdevmapper support re-arming the polling interface and grabbing the
> event number from the names listing, before I repost this without the
> RFC tag. I was also thinking of trying out cmocka by mocking up a
> device-mapper interface that let me test this code in isolation.
Great idea.
Am I understanding correctly that you are working on libdevmapper in
parallel? If yes, would it make sense to have libmultipath use the
newly developed libdevmapper API right away, rather than using a
custom-made ioctl interface until libdevmapper is ready?
> > > 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.
> >
> > Why use a command line option here rather than a config file
> > option?
>
> Mostly because it was faster, and I wanted to get to testing it. The
> other reason is that I don't see any benefit for the work involved in
> making this be changeable in
>
> # multipathd reconfigure
>
> However, we already have configuration settings that can't get
> changed
> on reconfigure, so making this another one is not a big deal. I agree
> that it is easier for users to change if it is a configuration
> setting,
> but I'm hoping that this change will be invisible to users. If you
> would
> prefer it as a configuration setting, I have no problem with changing
> that
> .
Right. It doesn't need to be user-configurable. We may want to leave a
compile-time option to disable it for the time being.
> > >
> > > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> > > ---
> > > multipathd/Makefile | 3 +-
> > > multipathd/dmevents.c | 396
> > > ++++++++++++++++++++++++++++++++++++++++++++++++++
> > > multipathd/dmevents.h | 13 ++
> > > multipathd/main.c | 58 +++++++-
> > > 4 files changed, 461 insertions(+), 9 deletions(-)
> > > create mode 100644 multipathd/dmevents.c
> > > create mode 100644 multipathd/dmevents.h
> > >
> > > diff --git a/multipathd/Makefile b/multipathd/Makefile
> > > index 85f29a7..4c438f0 100644
> > > --- a/multipathd/Makefile
> > > +++ b/multipathd/Makefile
> > > @@ -22,7 +22,8 @@ ifdef SYSTEMD
> > > endif
> > > 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..a56c055
> > > --- /dev/null
> > > +++ b/multipathd/dmevents.c
> > >
> > > +
> > > +
> > > +int alloc_dmevent_waiter(struct vectors *vecs)
> > > +{
> > > + if (!vecs) {
> > >
> > Nitpick: conventionally, an "alloc"-type function would return the
> > pointer, and NULL on failure.
>
> Is this a naming complaint, or an interface complaint? I'm fine with
> changing the names so they follow the lead of checkers and prio, i.e.
> init_dmevents_waiter() and cleanup_dmevents_waiter(). The init and
> cleanup functions for checkers and prio have the same returns as the
> dmevent functions (well the init functions return 1 for failure, and
> I
> can do that as well)
I'm fine with simply changing the names.
Regards
Martin
--
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC PATCH 5/5] multipathd: RFC add new polling dmevents waiter thread
2018-02-13 8:50 ` Martin Wilck
@ 2018-02-13 16:49 ` Benjamin Marzinski
2018-02-13 19:55 ` Martin Wilck
0 siblings, 1 reply; 19+ messages in thread
From: Benjamin Marzinski @ 2018-02-13 16:49 UTC (permalink / raw)
To: Martin Wilck; +Cc: device-mapper development
On Tue, Feb 13, 2018 at 09:50:19AM +0100, Martin Wilck wrote:
> Hi Ben,
>
> On Mon, 2018-02-12 at 17:18 -0600, Benjamin Marzinski wrote:
> > On Sat, Feb 10, 2018 at 08:55:53PM +0100, Martin Wilck wrote:
> > > Hi Ben,
> > >
> > > thanks a lot for this. I have only a few minor nitpicks (see
> > > below).
> > > I suppose you've tested this already?
> >
> > Yes. I do plan on doing some more testing after I look into making
> > libdevmapper support re-arming the polling interface and grabbing the
> > event number from the names listing, before I repost this without the
> > RFC tag. I was also thinking of trying out cmocka by mocking up a
> > device-mapper interface that let me test this code in isolation.
>
> Great idea.
>
> Am I understanding correctly that you are working on libdevmapper in
> parallel? If yes, would it make sense to have libmultipath use the
> newly developed libdevmapper API right away, rather than using a
> custom-made ioctl interface until libdevmapper is ready?
I haven't been working on adding the re-arming support to libdevmapper.
I just started looking into that now that I have all of these multipath
patches posted.
I'm not sure I understand you suggestion. There's a large amount of code
that can get executed when you call dm_task_run(). But the core bit of
code that it would execute for the DM_DEV_ARM_POLL command is that
ioctl. Also, the calculation to find the offset of the event number in
the dm_names structure will be the same when libdevmapper does them. I
have no problem with moving the functions I wrote (arm_dev_event_poll
and dm_event_nr) to libmultipath/devmapper.c, where they will eventually
use libdevmapper to do their work, but the actual code they will execute
as part of libdevmapper will be functionally the same.
> > > > 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.
> > >
> > > Why use a command line option here rather than a config file
> > > option?
> >
> > Mostly because it was faster, and I wanted to get to testing it. The
> > other reason is that I don't see any benefit for the work involved in
> > making this be changeable in
> >
> > # multipathd reconfigure
> >
> > However, we already have configuration settings that can't get
> > changed
> > on reconfigure, so making this another one is not a big deal. I agree
> > that it is easier for users to change if it is a configuration
> > setting,
> > but I'm hoping that this change will be invisible to users. If you
> > would
> > prefer it as a configuration setting, I have no problem with changing
> > that
> > .
>
> Right. It doesn't need to be user-configurable. We may want to leave a
> compile-time option to disable it for the time being.
>
I'm fine with adding a compile-time option. When this option is
compiled in, we do want to make multipathd able to use either method,
since not everyone will be running on a recent enough kernel. Since we
are doing that, I do want to keep some way to force the old method, even
if it is just for testing and debuging purposes. So I would like to keep
the -w option.
> > > >
> > > > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> > > > ---
> > > > multipathd/Makefile | 3 +-
> > > > multipathd/dmevents.c | 396
> > > > ++++++++++++++++++++++++++++++++++++++++++++++++++
> > > > multipathd/dmevents.h | 13 ++
> > > > multipathd/main.c | 58 +++++++-
> > > > 4 files changed, 461 insertions(+), 9 deletions(-)
> > > > create mode 100644 multipathd/dmevents.c
> > > > create mode 100644 multipathd/dmevents.h
> > > >
> > > > diff --git a/multipathd/Makefile b/multipathd/Makefile
> > > > index 85f29a7..4c438f0 100644
> > > > --- a/multipathd/Makefile
> > > > +++ b/multipathd/Makefile
> > > > @@ -22,7 +22,8 @@ ifdef SYSTEMD
> > > > endif
> > > > 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..a56c055
> > > > --- /dev/null
> > > > +++ b/multipathd/dmevents.c
> > > >
>
> > > > +
> > > > +
> > > > +int alloc_dmevent_waiter(struct vectors *vecs)
> > > > +{
> > > > + if (!vecs) {
> > > >
> > > Nitpick: conventionally, an "alloc"-type function would return the
> > > pointer, and NULL on failure.
> >
> > Is this a naming complaint, or an interface complaint? I'm fine with
> > changing the names so they follow the lead of checkers and prio, i.e.
> > init_dmevents_waiter() and cleanup_dmevents_waiter(). The init and
> > cleanup functions for checkers and prio have the same returns as the
> > dmevent functions (well the init functions return 1 for failure, and
> > I
> > can do that as well)
>
> I'm fine with simply changing the names.
Sure. I can do that.
> Regards
> Martin
>
> --
> Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
> SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
> HRB 21284 (AG Nürnberg)
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC PATCH 5/5] multipathd: RFC add new polling dmevents waiter thread
2018-02-13 16:49 ` Benjamin Marzinski
@ 2018-02-13 19:55 ` Martin Wilck
0 siblings, 0 replies; 19+ messages in thread
From: Martin Wilck @ 2018-02-13 19:55 UTC (permalink / raw)
To: Benjamin Marzinski; +Cc: device-mapper development
On Tue, 2018-02-13 at 10:49 -0600, Benjamin Marzinski wrote:
> On Tue, Feb 13, 2018 at 09:50:19AM +0100, Martin Wilck wrote:
> > Hi Ben,
> >
> > Am I understanding correctly that you are working on libdevmapper
> > in
> > parallel? If yes, would it make sense to have libmultipath use the
> > newly developed libdevmapper API right away, rather than using a
> > custom-made ioctl interface until libdevmapper is ready?
>
> I haven't been working on adding the re-arming support to
> libdevmapper.
> I just started looking into that now that I have all of these
> multipath
> patches posted.
>
> I'm not sure I understand you suggestion. There's a large amount of
> code
> that can get executed when you call dm_task_run(). But the core bit
> of
> code that it would execute for the DM_DEV_ARM_POLL command is that
> ioctl. Also, the calculation to find the offset of the event number
> in
> the dm_names structure will be the same when libdevmapper does them.
> I
> have no problem with moving the functions I wrote (arm_dev_event_poll
> and dm_event_nr) to libmultipath/devmapper.c, where they will
> eventually
> use libdevmapper to do their work, but the actual code they will
> execute
> as part of libdevmapper will be functionally the same.
OK. I think I misunderstood your remark about libdevmapper support.
Just go ahead according to your initial plan, fine with me.
Regards
Martin
--
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH 0/5] alternate dmevents waiter method
2018-02-10 5:07 [RFC PATCH 0/5] alternate dmevents waiter method Benjamin Marzinski
` (4 preceding siblings ...)
2018-02-10 5:07 ` [RFC PATCH 5/5] multipathd: RFC add new polling dmevents waiter thread Benjamin Marzinski
@ 2018-03-08 19:59 ` Xose Vazquez Perez
2018-03-08 20:08 ` Xose Vazquez Perez
5 siblings, 1 reply; 19+ messages in thread
From: Xose Vazquez Perez @ 2018-03-08 19:59 UTC (permalink / raw)
To: Benjamin Marzinski, device-mapper development, Martin Wilck,
Christophe Varoqui
On 02/10/2018 06:07 AM, Benjamin Marzinski wrote:
> This patchset implements a new method of getting dmevents for
> multipathd.
1-4/5 were Reviewed-by: Martin Wilck <mwilck@suse.com>.
And in 5/5, consensus was reached at the end: https://marc.info/?t=151823942200001
but they are t.
status?
> With the existing wait code, multipathd needs to create a waiter thread
> for every multipath device. This can become very wasteful in setups with
> large numbers of multipath devices. These duplicate threads all are
> serialized to update the multipath devices, so they don't actually speed
> up dmevent handling.
>
> The new method uses the new dmevent polling ability introduced in the
> 4.37.0 device-mapper kernel module. The original method has been
> retained for backwards compatablility, and it is possible to force
> multipathd to use the orignal method on newer kernels. The benefit of
> this new method is that there is only one thread necessary to wait on
> dmevents, which can be started when device-mapper starts, and stopped
> during shutdown, just like the other main threads.
>
> These patches use device-mapper features that don't have a libdevmapper
> API. They will switch over as soon as support is available in
> libdevmapper.
>
> This patchset is based on top of my recent "[PATCH v2 0/7] multipath:
> miscellaneous bug fixes". It doesn't touch the tur checker, so changes
> to "[PATCH v2 1/7] libmultipath: fix tur checker locking" won't change
> anything in this set.
>
> Benjamin Marzinski (5):
> libmultipath: move remove_map waiter code to multipathd
> move waiter code from libmultipath to multipathd
> call start_waiter_thread() before setup_multipath()
> libmultipath: add helper functions
> multipathd: RFC add new polling dmevents waiter thread
>
> libmultipath/Makefile | 2 +-
> libmultipath/devmapper.c | 28 +++-
> libmultipath/devmapper.h | 3 +-
> libmultipath/structs_vec.c | 138 +---------------
> libmultipath/structs_vec.h | 6 +-
> libmultipath/vector.c | 16 +-
> libmultipath/vector.h | 1 +
> libmultipath/waiter.c | 215 ------------------------
> libmultipath/waiter.h | 17 --
> multipathd/Makefile | 3 +-
> multipathd/dmevents.c | 396 +++++++++++++++++++++++++++++++++++++++++++++
> multipathd/dmevents.h | 13 ++
> multipathd/main.c | 194 ++++++++++++++++++++--
> multipathd/waiter.c | 215 ++++++++++++++++++++++++
> multipathd/waiter.h | 17 ++
> 15 files changed, 869 insertions(+), 395 deletions(-)
> delete mode 100644 libmultipath/waiter.c
> delete mode 100644 libmultipath/waiter.h
> create mode 100644 multipathd/dmevents.c
> create mode 100644 multipathd/dmevents.h
> create mode 100644 multipathd/waiter.c
> create mode 100644 multipathd/waiter.h
>
^ permalink raw reply [flat|nested] 19+ messages in thread