Signed-off-by: Ben Pfaff <b...@nicira.com> --- lib/vlog.c | 113 +++++++++++++++++++++++++++++++++++++----------------------- lib/vlog.h | 9 +++++ 2 files changed, 79 insertions(+), 43 deletions(-)
diff --git a/lib/vlog.c b/lib/vlog.c index d134c29..97c5b44 100644 --- a/lib/vlog.c +++ b/lib/vlog.c @@ -33,6 +33,7 @@ #include "dirs.h" #include "dynamic-string.h" #include "ofpbuf.h" +#include "ovs-thread.h" #include "sat-math.h" #include "svec.h" #include "timeval.h" @@ -84,7 +85,7 @@ struct vlog_module *vlog_modules[] = { /* Information about each facility. */ struct facility { const char *name; /* Name. */ - char *pattern; /* Current pattern. */ + char *pattern; /* Current pattern (see 'pattern_rwlock'). */ bool default_pattern; /* Whether current pattern is the default. */ }; static struct facility facilities[VLF_N_FACILITIES] = { @@ -93,16 +94,25 @@ static struct facility facilities[VLF_N_FACILITIES] = { #undef VLOG_FACILITY }; -/* VLF_FILE configuration. */ +/* Protects the 'pattern' in all "struct facility"s, so that a race between + * changing and reading the pattern does not cause an access to freed + * memory. */ +static pthread_rwlock_t pattern_rwlock = PTHREAD_RWLOCK_INITIALIZER; + +/* Sequence number for the message currently being composed. */ +DEFINE_PER_THREAD_DATA(unsigned int, msg_num, 0); + +/* VLF_FILE configuration. + * + * All of the following is protected by 'log_file_mutex', which nests inside + * pattern_rwlock. */ +static pthread_mutex_t log_file_mutex = PTHREAD_ADAPTIVE_MUTEX_INITIALIZER; static char *log_file_name; static int log_fd = -1; static struct async_append *log_writer; -/* vlog initialized? */ -static bool vlog_inited; - static void format_log_message(const struct vlog_module *, enum vlog_level, - enum vlog_facility, unsigned int msg_num, + enum vlog_facility, const char *message, va_list, struct ds *) PRINTF_FORMAT(5, 0); @@ -191,7 +201,7 @@ vlog_get_level(const struct vlog_module *module, enum vlog_facility facility) return module->levels[facility]; } -static void +static void OVS_MUST_HOLD(log_file_mutex) update_min_level(struct vlog_module *module) { enum vlog_facility facility; @@ -214,6 +224,7 @@ set_facility_level(enum vlog_facility facility, struct vlog_module *module, assert(facility >= 0 && facility < VLF_N_FACILITIES); assert(level < VLL_N_LEVELS); + xpthread_mutex_lock(&log_file_mutex); if (!module) { struct vlog_module **mp; @@ -225,6 +236,7 @@ set_facility_level(enum vlog_facility facility, struct vlog_module *module, module->levels[facility] = level; update_min_level(module); } + xpthread_mutex_unlock(&log_file_mutex); } /* Sets the logging level for the given 'module' and 'facility' to 'level'. A @@ -248,12 +260,15 @@ static void do_set_pattern(enum vlog_facility facility, const char *pattern) { struct facility *f = &facilities[facility]; + + xpthread_rwlock_wrlock(&pattern_rwlock); if (!f->default_pattern) { free(f->pattern); } else { f->default_pattern = false; } f->pattern = xstrdup(pattern); + xpthread_rwlock_unlock(&pattern_rwlock); } /* Sets the pattern for the given 'facility' to 'pattern'. */ @@ -280,6 +295,8 @@ vlog_set_log_file(const char *file_name) struct vlog_module **mp; int error; + xpthread_mutex_lock(&log_file_mutex); + /* Close old log file. */ if (log_fd >= 0) { VLOG_INFO("closing log file"); @@ -318,6 +335,8 @@ vlog_set_log_file(const char *file_name) error = 0; } + xpthread_mutex_unlock(&log_file_mutex); + return error; } @@ -328,24 +347,27 @@ int vlog_reopen_log_file(void) { struct stat old, new; + bool retval; - /* Skip re-opening if there's nothing to reopen. */ + xpthread_mutex_lock(&log_file_mutex); if (!log_file_name) { - return 0; - } - - /* Skip re-opening if it would be a no-op because the old and new files are - * the same. (This avoids writing "closing log file" followed immediately - * by "opened log file".) */ - if (log_fd >= 0 - && !fstat(log_fd, &old) - && !stat(log_file_name, &new) - && old.st_dev == new.st_dev - && old.st_ino == new.st_ino) { - return 0; + /* Nothing to reopen. */ + retval = 0; + } else if (log_fd >= 0 + && !fstat(log_fd, &old) + && !stat(log_file_name, &new) + && old.st_dev == new.st_dev + && old.st_ino == new.st_ino) { + /* The old and new files are the same, so the reopen would be a no-op. + * Skip it, to avoid writing "closing log file" followed immediately by + * "opened log file". */ + retval = 0; + } else { + retval = vlog_set_log_file(log_file_name); } + xpthread_mutex_unlock(&log_file_mutex); - return vlog_set_log_file(log_file_name); + return retval; } /* Set debugging levels. Returns null if successful, otherwise an error @@ -538,19 +560,12 @@ vlog_disable_rate_limit(struct unixctl_conn *conn, int argc, set_rate_limits(conn, argc, argv, false); } -/* Initializes the logging subsystem and registers its unixctl server - * commands. */ -void -vlog_init(void) +static void +vlog_init__(void) { static char *program_name_copy; time_t now; - if (vlog_inited) { - return; - } - vlog_inited = true; - /* openlog() is allowed to keep the pointer passed in, without making a * copy. The daemonize code sometimes frees and replaces 'program_name', * so make a private copy just for openlog(). (We keep a pointer to the @@ -578,6 +593,15 @@ vlog_init(void) vlog_unixctl_reopen, NULL); } +/* Initializes the logging subsystem and registers its unixctl server + * commands. */ +void +vlog_init(void) +{ + static pthread_once_t once = PTHREAD_ONCE_INIT; + pthread_once(&once, vlog_init__); +} + /* Print the current logging level for each module. */ char * vlog_get_levels(void) @@ -643,7 +667,7 @@ fetch_braces(const char *p, const char *def, char *out, size_t out_size) static void format_log_message(const struct vlog_module *module, enum vlog_level level, - enum vlog_facility facility, unsigned int msg_num, + enum vlog_facility facility, const char *message, va_list args_, struct ds *s) { char tmp[128]; @@ -705,7 +729,7 @@ format_log_message(const struct vlog_module *module, enum vlog_level level, } break; case 'N': - ds_put_format(s, "%u", msg_num); + ds_put_format(s, "%u", *msg_num_get_unsafe()); break; case 'n': ds_put_char(s, '\n'); @@ -760,18 +784,17 @@ vlog_valist(const struct vlog_module *module, enum vlog_level level, bool log_to_file = module->levels[VLF_FILE] >= level && log_fd >= 0; if (log_to_console || log_to_syslog || log_to_file) { int save_errno = errno; - static unsigned int msg_num; struct ds s; vlog_init(); ds_init(&s); ds_reserve(&s, 1024); - msg_num++; + ++*msg_num_get(); + xpthread_rwlock_rdlock(&pattern_rwlock); if (log_to_console) { - format_log_message(module, level, VLF_CONSOLE, msg_num, - message, args, &s); + format_log_message(module, level, VLF_CONSOLE, message, args, &s); ds_put_char(&s, '\n'); fputs(ds_cstr(&s), stderr); } @@ -781,8 +804,7 @@ vlog_valist(const struct vlog_module *module, enum vlog_level level, char *save_ptr = NULL; char *line; - format_log_message(module, level, VLF_SYSLOG, msg_num, - message, args, &s); + format_log_message(module, level, VLF_SYSLOG, message, args, &s); for (line = strtok_r(s.string, "\n", &save_ptr); line; line = strtok_r(NULL, "\n", &save_ptr)) { syslog(syslog_level, "%s", line); @@ -790,14 +812,19 @@ vlog_valist(const struct vlog_module *module, enum vlog_level level, } if (log_to_file) { - format_log_message(module, level, VLF_FILE, msg_num, - message, args, &s); + format_log_message(module, level, VLF_FILE, message, args, &s); ds_put_char(&s, '\n'); - async_append_write(log_writer, s.string, s.length); - if (level == VLL_EMER) { - async_append_flush(log_writer); + + xpthread_mutex_lock(&log_file_mutex); + if (log_fd >= 0) { + async_append_write(log_writer, s.string, s.length); + if (level == VLL_EMER) { + async_append_flush(log_writer); + } } + xpthread_mutex_unlock(&log_file_mutex); } + xpthread_rwlock_unlock(&pattern_rwlock); ds_destroy(&s); errno = save_errno; diff --git a/lib/vlog.h b/lib/vlog.h index eeec5fc..9576687 100644 --- a/lib/vlog.h +++ b/lib/vlog.h @@ -17,6 +17,15 @@ #ifndef VLOG_H #define VLOG_H 1 +/* Logging. + * + * + * Thread-safety + * ============= + * + * Fully thread safe. + */ + #include <limits.h> #include <stdarg.h> #include <stdbool.h> -- 1.7.2.5 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev