From patchwork Tue May 7 22:15:57 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Benjamin Marzinski X-Patchwork-Id: 2536961 Return-Path: X-Original-To: patchwork-dm-devel@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from mx4-phx2.redhat.com (mx4-phx2.redhat.com [209.132.183.25]) by patchwork2.kernel.org (Postfix) with ESMTP id 0069EDF215 for ; Tue, 7 May 2013 22:19:49 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by mx4-phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r47MG2nr020281; Tue, 7 May 2013 18:16:04 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r47MG0mU010602 for ; Tue, 7 May 2013 18:16:00 -0400 Received: from ether.msp.redhat.com (ether.msp.redhat.com [10.15.80.119]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r47MFxLH027155 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 7 May 2013 18:16:00 -0400 Received: from ether.msp.redhat.com (localhost.localdomain [127.0.0.1]) by ether.msp.redhat.com (8.14.1/8.14.1) with ESMTP id r47MFwjX028518; Tue, 7 May 2013 17:15:59 -0500 Received: (from bmarzins@localhost) by ether.msp.redhat.com (8.14.1/8.14.1/Submit) id r47MFvgD028517; Tue, 7 May 2013 17:15:57 -0500 Date: Tue, 7 May 2013 17:15:57 -0500 From: Benjamin Marzinski To: device-mapper development Message-ID: <20130507221557.GU6253@ether.msp.redhat.com> References: <20130507221237.GT6253@ether.msp.redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20130507221237.GT6253@ether.msp.redhat.com> User-Agent: Mutt/1.5.17 (2007-11-01) X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 X-loop: dm-devel@redhat.com Cc: Bart Van Assche , Christophe Varoqui Subject: [dm-devel] [PATCH 2/2] simplify multipath signal handlers X-BeenThere: dm-devel@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk Reply-To: device-mapper development List-Id: device-mapper development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dm-devel-bounces@redhat.com Errors-To: dm-devel-bounces@redhat.com 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 --- 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 -#include #include "lock.h" #include -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 #include #include +#include #include /* @@ -51,6 +52,7 @@ #include #include #include +#include #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 #include #include @@ -19,20 +20,21 @@ #include #include #include - +#include #include - #include #include #include #include +#include #include #include +#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