[ovs-dev] [threads 01/17] signals: Make thread-safe.

Ben Pfaff blp at nicira.com
Wed Jun 5 22:04:55 UTC 2013


Applied, thanks.

On Wed, Jun 05, 2013 at 02:06:17PM -0700, Ethan Jackson wrote:
> Acked-by: Ethan Jackson <ethan at nicira.com>
> 
> On Wed, Jun 5, 2013 at 1:05 PM, Ben Pfaff <blp at nicira.com> wrote:
> > Signed-off-by: Ben Pfaff <blp at nicira.com>
> > ---
> >  lib/signals.c           |   66 ++++++++++-------------------------------------
> >  lib/signals.h           |    4 ---
> >  vswitchd/ovs-vswitchd.c |    1 -
> >  3 files changed, 14 insertions(+), 57 deletions(-)
> >
> > diff --git a/lib/signals.c b/lib/signals.c
> > index 99a36a4..1340f54 100644
> > --- a/lib/signals.c
> > +++ b/lib/signals.c
> > @@ -40,80 +40,47 @@ VLOG_DEFINE_THIS_MODULE(signals);
> >  #endif
> >
> >  struct signal {
> > -    struct sigaction saved_sa;
> > -    int signr;
> > +    int fds[2];
> >  };
> >
> > -static volatile sig_atomic_t signaled[N_SIGNALS];
> > -
> > -static int fds[2];
> > +static struct signal signals[N_SIGNALS];
> >
> >  static void signal_handler(int signr);
> >
> > -/* Initializes the signals subsystem (if it is not already initialized).  Calls
> > - * exit() if initialization fails.
> > - *
> > - * Calling this function is optional; it will be called automatically by
> > - * signal_start() if necessary.  Calling it explicitly allows the client to
> > - * prevent the process from exiting at an unexpected time. */
> > -void
> > -signal_init(void)
> > -{
> > -    static bool inited;
> > -    if (!inited) {
> > -        inited = true;
> > -        xpipe_nonblocking(fds);
> > -    }
> > -}
> > -
> >  /* Sets up a handler for 'signr' and returns a structure that represents it.
> >   *
> > - * Only one handler for a given signal may be registered at a time. */
> > + * Only one handler for a given signal may be registered. */
> >  struct signal *
> >  signal_register(int signr)
> >  {
> >      struct sigaction sa;
> >      struct signal *s;
> >
> > -    signal_init();
> > +    ovs_assert(signr >= 1 && signr < N_SIGNALS);
> >
> > -    s = xmalloc(sizeof *s);
> > -    s->signr = signr;
> > +    /* Create a pipe. */
> > +    s = &signals[signr];
> > +    ovs_assert(!s->fds[0] && !s->fds[1]);
> > +    xpipe_nonblocking(s->fds);
> >
> > -    /* Set up signal handler. */
> > -    ovs_assert(signr >= 1 && signr < N_SIGNALS);
> > +    /* Install signal handler. */
> >      memset(&sa, 0, sizeof sa);
> >      sa.sa_handler = signal_handler;
> >      sigemptyset(&sa.sa_mask);
> >      sa.sa_flags = SA_RESTART;
> > -    xsigaction(signr, &sa, &s->saved_sa);
> > +    xsigaction(signr, &sa, NULL);
> >
> >      return s;
> >  }
> >
> > -/* Unregisters the handler for 's', restores the signal handler that was in
> > - * effect before signal_register() was called, and frees 's'. */
> > -void
> > -signal_unregister(struct signal *s)
> > -{
> > -    if (s) {
> > -        xsigaction(s->signr, &s->saved_sa, NULL);
> > -        free(s);
> > -    }
> > -}
> > -
> >  /* Returns true if signal 's' has been received since the last call to this
> >   * function with argument 's'. */
> >  bool
> >  signal_poll(struct signal *s)
> >  {
> >      char buf[_POSIX_PIPE_BUF];
> > -    ignore(read(fds[0], buf, sizeof buf));
> > -    if (signaled[s->signr]) {
> > -        signaled[s->signr] = 0;
> > -        return true;
> > -    }
> > -    return false;
> > +
> > +    return read(s->fds[0], buf, sizeof buf) > 0;
> >  }
> >
> >  /* Causes the next call to poll_block() to wake up when signal_poll(s) would
> > @@ -121,19 +88,14 @@ signal_poll(struct signal *s)
> >  void
> >  signal_wait(struct signal *s)
> >  {
> > -    if (signaled[s->signr]) {
> > -        poll_immediate_wake();
> > -    } else {
> > -        poll_fd_wait(fds[0], POLLIN);
> > -    }
> > +    poll_fd_wait(s->fds[0], POLLIN);
> >  }
> >
> >  static void
> >  signal_handler(int signr)
> >  {
> >      if (signr >= 1 && signr < N_SIGNALS) {
> > -        ignore(write(fds[1], "", 1));
> > -        signaled[signr] = true;
> > +        ignore(write(signals[signr].fds[1], "", 1));
> >      }
> >  }
> >
> > diff --git a/lib/signals.h b/lib/signals.h
> > index 015d329..b3983b2 100644
> > --- a/lib/signals.h
> > +++ b/lib/signals.h
> > @@ -21,11 +21,7 @@
> >  #include <stdbool.h>
> >  #include "type-props.h"
> >
> > -void signal_init(void);
> > -
> >  struct signal *signal_register(int signr);
> > -void signal_unregister(struct signal *);
> > -
> >  bool signal_poll(struct signal *);
> >  void signal_wait(struct signal *);
> >
> > diff --git a/vswitchd/ovs-vswitchd.c b/vswitchd/ovs-vswitchd.c
> > index 78ebd31..e11febd 100644
> > --- a/vswitchd/ovs-vswitchd.c
> > +++ b/vswitchd/ovs-vswitchd.c
> > @@ -140,7 +140,6 @@ main(int argc, char *argv[])
> >      }
> >      bridge_exit();
> >      unixctl_server_destroy(unixctl);
> > -    signal_unregister(sighup);
> >
> >      return 0;
> >  }
> > --
> > 1.7.2.5
> >
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev



More information about the dev mailing list