[ovs-dev] [threads 28/28] vlog: Make thread-safe.
Ben Pfaff
blp at nicira.com
Wed Jul 10 23:04:10 UTC 2013
Signed-off-by: Ben Pfaff <blp at 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
More information about the dev
mailing list