* [PATCH 2/2] simplify multipath signal handlers
2013-05-07 22:12 [PATCH 1/2] Add missing includes for remember_wwid Benjamin Marzinski
@ 2013-05-07 22:15 ` Benjamin Marzinski
2013-05-08 5:58 ` Bart Van Assche
0 siblings, 1 reply; 3+ messages in thread
From: Benjamin Marzinski @ 2013-05-07 22:15 UTC (permalink / raw)
To: device-mapper development; +Cc: Bart Van Assche, Christophe Varoqui
This patch changes how multipath's signal handlers work. Instead of
having sighup and sigusr1 acquire locks and call functions directly,
they now both simply set atomic variables. These two signals are
blocked in child(), and all other threads inherit this sigmask. The
only place in all the multipath code that doesn't block these signals
is now the ppoll() call by the uxlsnr thread. When it is interrupted
by a signal, the uxlsnr thread does the actual processing work.
Instead of sigend using mutex locks and condition variables to tell
the child() function to exit, it now uses a signal_handler safe
semaphore that child() waits on and exit_daemon() increments.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
libmultipath/lock.c | 9 -----
libmultipath/lock.h | 1 -
libmultipath/log_pthread.c | 22 ------------
libmultipath/waiter.c | 2 --
multipathd/cli_handlers.c | 4 +--
multipathd/main.c | 90 +++++++++++++++++++++-------------------------
multipathd/main.h | 3 +-
multipathd/uxlsnr.c | 21 +++++++----
multipathd/uxlsnr.h | 3 ++
9 files changed, 63 insertions(+), 92 deletions(-)
diff --git a/libmultipath/lock.c b/libmultipath/lock.c
index 4439a51..6ce9a74 100644
--- a/libmultipath/lock.c
+++ b/libmultipath/lock.c
@@ -1,16 +1,7 @@
#include <pthread.h>
-#include <signal.h>
#include "lock.h"
#include <stdio.h>
-void block_signal (int signum, sigset_t *old)
-{
- sigset_t set;
- sigemptyset(&set);
- sigaddset(&set, signum);
- pthread_sigmask(SIG_BLOCK, &set, old);
-}
-
void cleanup_lock (void * data)
{
unlock ((*(struct mutex_lock *)data));
diff --git a/libmultipath/lock.h b/libmultipath/lock.h
index 6897a74..04ef78d 100644
--- a/libmultipath/lock.h
+++ b/libmultipath/lock.h
@@ -29,6 +29,5 @@ struct mutex_lock {
#endif
void cleanup_lock (void * data);
-void block_signal(int signum, sigset_t *old);
#endif /* _LOCK_H */
diff --git a/libmultipath/log_pthread.c b/libmultipath/log_pthread.c
index b8f9119..47d75a1 100644
--- a/libmultipath/log_pthread.c
+++ b/libmultipath/log_pthread.c
@@ -22,26 +22,13 @@ pthread_cond_t logev_cond;
int logq_running;
-static void
-sigusr1 (int sig)
-{
- pthread_mutex_lock(&logq_lock);
- log_reset("multipathd");
- pthread_mutex_unlock(&logq_lock);
-}
-
void log_safe (int prio, const char * fmt, va_list ap)
{
- sigset_t old;
-
if (log_thr == (pthread_t)0) {
syslog(prio, fmt, ap);
return;
}
- block_signal(SIGUSR1, &old);
- block_signal(SIGHUP, NULL);
-
pthread_mutex_lock(&logq_lock);
log_enqueue(prio, fmt, ap);
pthread_mutex_unlock(&logq_lock);
@@ -49,8 +36,6 @@ void log_safe (int prio, const char * fmt, va_list ap)
pthread_mutex_lock(&logev_lock);
pthread_cond_signal(&logev_cond);
pthread_mutex_unlock(&logev_lock);
-
- pthread_sigmask(SIG_SETMASK, &old, NULL);
}
void log_thread_flush (void)
@@ -81,15 +66,8 @@ static void flush_logqueue (void)
static void * log_thread (void * et)
{
- struct sigaction sig;
int running;
- sig.sa_handler = sigusr1;
- sigemptyset(&sig.sa_mask);
- sig.sa_flags = 0;
- if (sigaction(SIGUSR1, &sig, NULL) < 0)
- logdbg(stderr, "Cannot set signal handler");
-
pthread_mutex_lock(&logev_lock);
logq_running = 1;
pthread_mutex_unlock(&logev_lock);
diff --git a/libmultipath/waiter.c b/libmultipath/waiter.c
index da71543..5094290 100644
--- a/libmultipath/waiter.c
+++ b/libmultipath/waiter.c
@@ -157,8 +157,6 @@ void *waitevent (void *et)
waiter = (struct event_thread *)et;
pthread_cleanup_push(free_waiter, et);
- block_signal(SIGUSR1, NULL);
- block_signal(SIGHUP, NULL);
while (1) {
r = waiteventloop(waiter);
diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index ac648ad..7b1cb62 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -939,8 +939,8 @@ int
cli_shutdown (void * v, char ** reply, int * len, void * data)
{
condlog(3, "shutdown (operator)");
-
- return exit_daemon(0);
+ exit_daemon();
+ return 0;
}
int
diff --git a/multipathd/main.c b/multipathd/main.c
index eb4bc8a..0fa1b17 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -17,6 +17,7 @@
#include <limits.h>
#include <linux/oom.h>
#include <libudev.h>
+#include <semaphore.h>
#include <mpath_persist.h>
/*
@@ -51,6 +52,7 @@
#include <prio.h>
#include <pgpolicies.h>
#include <uevent.h>
+#include <log.h>
#include "main.h"
#include "pidfile.h"
@@ -81,13 +83,11 @@ struct mpath_event_param
unsigned int mpath_mx_alloc_len;
-pthread_cond_t exit_cond = PTHREAD_COND_INITIALIZER;
-pthread_mutex_t exit_mutex = PTHREAD_MUTEX_INITIALIZER;
-
int logsink;
enum daemon_status running_state;
pid_t daemon_pid;
+static sem_t exit_sem;
/*
* global copy of vecs for use in sig handlers
*/
@@ -833,9 +833,6 @@ out:
static void *
ueventloop (void * ap)
{
- block_signal(SIGUSR1, NULL);
- block_signal(SIGHUP, NULL);
-
if (uevent_listen())
condlog(0, "error starting uevent listener");
@@ -845,9 +842,6 @@ ueventloop (void * ap)
static void *
uevqloop (void * ap)
{
- block_signal(SIGUSR1, NULL);
- block_signal(SIGHUP, NULL);
-
if (uevent_dispatch(&uev_trigger, ap))
condlog(0, "error starting uevent dispatcher");
@@ -856,9 +850,6 @@ uevqloop (void * ap)
static void *
uxlsnrloop (void * ap)
{
- block_signal(SIGUSR1, NULL);
- block_signal(SIGHUP, NULL);
-
if (cli_init())
return NULL;
@@ -908,18 +899,10 @@ uxlsnrloop (void * ap)
return NULL;
}
-int
-exit_daemon (int status)
+void
+exit_daemon (void)
{
- if (status != 0)
- fprintf(stderr, "bad exit status. see daemon.log\n");
-
- if (running_state != DAEMON_SHUTDOWN) {
- pthread_mutex_lock(&exit_mutex);
- pthread_cond_signal(&exit_cond);
- pthread_mutex_unlock(&exit_mutex);
- }
- return status;
+ sem_post(&exit_sem);
}
const char *
@@ -1282,7 +1265,6 @@ checkerloop (void *ap)
struct path *pp;
int count = 0;
unsigned int i;
- sigset_t old;
mlockall(MCL_CURRENT | MCL_FUTURE);
vecs = (struct vectors *)ap;
@@ -1296,7 +1278,6 @@ checkerloop (void *ap)
}
while (1) {
- block_signal(SIGHUP, &old);
pthread_cleanup_push(cleanup_lock, &vecs->lock);
lock(vecs->lock);
pthread_testcancel();
@@ -1320,7 +1301,6 @@ checkerloop (void *ap)
}
lock_cleanup_pop(vecs->lock);
- pthread_sigmask(SIG_SETMASK, &old, NULL);
sleep(1);
}
return NULL;
@@ -1480,36 +1460,56 @@ signal_set(int signo, void (*func) (int))
return (osig.sa_handler);
}
+void
+handle_signals(void)
+{
+ if (reconfig_sig && running_state == DAEMON_RUNNING) {
+ condlog(2, "reconfigure (signal)");
+ pthread_cleanup_push(cleanup_lock,
+ &gvecs->lock);
+ lock(gvecs->lock);
+ pthread_testcancel();
+ reconfigure(gvecs);
+ lock_cleanup_pop(gvecs->lock);
+ }
+ if (log_reset_sig) {
+ condlog(2, "reset log (signal)");
+ pthread_mutex_lock(&logq_lock);
+ log_reset("multipathd");
+ pthread_mutex_unlock(&logq_lock);
+ }
+ reconfig_sig = 0;
+ log_reset_sig = 0;
+}
+
static void
sighup (int sig)
{
- condlog(2, "reconfigure (SIGHUP)");
-
- if (running_state != DAEMON_RUNNING)
- return;
-
- reconfigure(gvecs);
-
-#ifdef _DEBUG_
- dbg_free_final(NULL);
-#endif
+ reconfig_sig = 1;
}
static void
sigend (int sig)
{
- exit_daemon(0);
+ exit_daemon();
}
static void
sigusr1 (int sig)
{
- condlog(3, "SIGUSR1 received");
+ log_reset_sig = 1;
}
static void
signal_init(void)
{
+ sigset_t set;
+
+ sigemptyset(&set);
+ sigaddset(&set, SIGHUP);
+ sigaddset(&set, SIGUSR1);
+ pthread_sigmask(SIG_BLOCK, &set, NULL);
+
signal_set(SIGHUP, sighup);
signal_set(SIGUSR1, sigusr1);
signal_set(SIGINT, sigend);
@@ -1582,10 +1582,11 @@ child (void * param)
struct vectors * vecs;
struct multipath * mpp;
int i;
- sigset_t set;
int rc, pid_rc;
mlockall(MCL_CURRENT | MCL_FUTURE);
+ sem_init(&exit_sem, 0, 0);
+ signal_init();
setup_thread_attr(&misc_attr, 64 * 1024, 1);
setup_thread_attr(&waiter_attr, 32 * 1024, 1);
@@ -1645,7 +1646,6 @@ child (void * param)
if (!vecs)
exit(1);
- signal_init();
setscheduler();
set_oom_adj();
@@ -1688,25 +1688,17 @@ child (void * param)
}
pthread_attr_destroy(&misc_attr);
- pthread_mutex_lock(&exit_mutex);
/* Startup complete, create logfile */
pid_rc = pidfile_create(DEFAULT_PIDFILE, daemon_pid);
/* Ignore errors, we can live without */
running_state = DAEMON_RUNNING;
- pthread_cond_wait(&exit_cond, &exit_mutex);
- /* Need to block these to avoid deadlocking */
- sigemptyset(&set);
- sigaddset(&set, SIGTERM);
- sigaddset(&set, SIGINT);
- pthread_sigmask(SIG_BLOCK, &set, NULL);
/*
* exit path
*/
+ while(sem_wait(&exit_sem) != 0); /* Do nothing */
running_state = DAEMON_SHUTDOWN;
- pthread_sigmask(SIG_UNBLOCK, &set, NULL);
- block_signal(SIGHUP, NULL);
lock(vecs->lock);
if (conf->queue_without_daemon == QUE_NO_DAEMON_OFF)
vector_foreach_slot(vecs->mpvec, mpp, i)
diff --git a/multipathd/main.h b/multipathd/main.h
index 242f5e1..10378ef 100644
--- a/multipathd/main.h
+++ b/multipathd/main.h
@@ -16,7 +16,7 @@ struct prin_resp;
extern pid_t daemon_pid;
-int exit_daemon(int);
+void exit_daemon(void);
const char * daemon_status(void);
int reconfigure (struct vectors *);
int ev_add_path (struct path *, struct vectors *);
@@ -35,5 +35,6 @@ int mpath_pr_event_handle(struct path *pp);
void * mpath_pr_event_handler_fn (void * );
int update_map_pr(struct multipath *mpp);
void * mpath_pr_event_handler_fn (void * pathp );
+void handle_signals(void);
#endif /* MAIN_H */
diff --git a/multipathd/uxlsnr.c b/multipathd/uxlsnr.c
index 71fc3b4..5334d69 100644
--- a/multipathd/uxlsnr.c
+++ b/multipathd/uxlsnr.c
@@ -8,6 +8,7 @@
/*
* A simple domain socket listener
*/
+#define _GNU_SOURCE
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
@@ -19,20 +20,21 @@
#include <sys/socket.h>
#include <sys/un.h>
#include <sys/poll.h>
-
+#include <signal.h>
#include <checkers.h>
-
#include <memory.h>
#include <debug.h>
#include <vector.h>
#include <structs.h>
+#include <structs_vec.h>
#include <uxsock.h>
#include <defaults.h>
+#include "main.h"
#include "cli.h"
#include "uxlsnr.h"
-#define SLEEP_TIME 5000
+struct timespec sleep_time = {5, 0};
struct client {
int fd;
@@ -42,6 +44,8 @@ struct client {
static struct client *clients;
static unsigned num_clients;
struct pollfd *polls;
+volatile sig_atomic_t reconfig_sig = 0;
+volatile sig_atomic_t log_reset_sig = 0;
/*
* handle a new client joining
@@ -104,6 +108,7 @@ void * uxsock_listen(int (*uxsock_trigger)(char *, char **, int *, void *),
int rlen;
char *inbuf;
char *reply;
+ sigset_t mask;
ux_sock = ux_socket_listen(DEFAULT_SOCKET);
@@ -115,7 +120,9 @@ void * uxsock_listen(int (*uxsock_trigger)(char *, char **, int *, void *),
pthread_cleanup_push(uxsock_cleanup, NULL);
polls = (struct pollfd *)MALLOC(0);
-
+ sigprocmask(SIG_SETMASK, NULL, &mask);
+ sigdelset(&mask, SIGHUP);
+ sigdelset(&mask, SIGUSR1);
while (1) {
struct client *c;
int i, poll_count;
@@ -132,11 +139,13 @@ void * uxsock_listen(int (*uxsock_trigger)(char *, char **, int *, void *),
}
/* most of our life is spent in this call */
- poll_count = poll(polls, i, SLEEP_TIME);
+ poll_count = ppoll(polls, i, &sleep_time, &mask);
if (poll_count == -1) {
- if (errno == EINTR)
+ if (errno == EINTR) {
+ handle_signals();
continue;
+ }
/* something went badly wrong! */
condlog(0, "poll");
diff --git a/multipathd/uxlsnr.h b/multipathd/uxlsnr.h
index c646d5d..8b5a525 100644
--- a/multipathd/uxlsnr.h
+++ b/multipathd/uxlsnr.h
@@ -4,5 +4,8 @@
void * uxsock_listen(int (*uxsock_trigger)
(char *, char **, int *, void *),
void * trigger_data);
+
+extern volatile sig_atomic_t reconfig_sig;
+extern volatile sig_atomic_t log_reset_sig;
#endif
--
1.8.2
^ permalink raw reply related [flat|nested] 3+ messages in thread