[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PULL 15/25] Add use of RCU for qemu_logfile.
From: |
Alex Bennée |
Subject: |
[PULL 15/25] Add use of RCU for qemu_logfile. |
Date: |
Thu, 19 Dec 2019 10:49:24 +0000 |
From: Robert Foley <address@hidden>
This now allows changing the logfile while logging is active,
and also solves the issue of a seg fault while changing the logfile.
Any read access to the qemu_logfile handle will use
the rcu_read_lock()/unlock() around the use of the handle.
To fetch the handle we will use atomic_rcu_read().
We also in many cases do a check for validity of the
logfile handle before using it to deal with the case where the
file is closed and set to NULL.
The cases where we write to the qemu_logfile will use atomic_rcu_set().
Writers will also use call_rcu() with a newly added qemu_logfile_free
function for freeing/closing when readers have finished.
Signed-off-by: Robert Foley <address@hidden>
Reviewed-by: Alex Bennée <address@hidden>
Signed-off-by: Alex Bennée <address@hidden>
Message-Id: <address@hidden>
diff --git a/include/exec/log.h b/include/exec/log.h
index e2cfd436e61..9bd1e4aa20b 100644
--- a/include/exec/log.h
+++ b/include/exec/log.h
@@ -15,8 +15,15 @@
*/
static inline void log_cpu_state(CPUState *cpu, int flags)
{
+ QemuLogFile *logfile;
+
if (qemu_log_enabled()) {
- cpu_dump_state(cpu, qemu_logfile, flags);
+ rcu_read_lock();
+ logfile = atomic_rcu_read(&qemu_logfile);
+ if (logfile) {
+ cpu_dump_state(cpu, logfile->fd, flags);
+ }
+ rcu_read_unlock();
}
}
@@ -40,19 +47,37 @@ static inline void log_cpu_state_mask(int mask, CPUState
*cpu, int flags)
static inline void log_target_disas(CPUState *cpu, target_ulong start,
target_ulong len)
{
- target_disas(qemu_logfile, cpu, start, len);
+ QemuLogFile *logfile;
+ rcu_read_lock();
+ logfile = atomic_rcu_read(&qemu_logfile);
+ if (logfile) {
+ target_disas(logfile->fd, cpu, start, len);
+ }
+ rcu_read_unlock();
}
static inline void log_disas(void *code, unsigned long size)
{
- disas(qemu_logfile, code, size);
+ QemuLogFile *logfile;
+ rcu_read_lock();
+ logfile = atomic_rcu_read(&qemu_logfile);
+ if (logfile) {
+ disas(logfile->fd, code, size);
+ }
+ rcu_read_unlock();
}
#if defined(CONFIG_USER_ONLY)
/* page_dump() output to the log file: */
static inline void log_page_dump(void)
{
- page_dump(qemu_logfile);
+ QemuLogFile *logfile;
+ rcu_read_lock();
+ logfile = atomic_rcu_read(&qemu_logfile);
+ if (logfile) {
+ page_dump(logfile->fd);
+ }
+ rcu_read_unlock();
}
#endif
#endif
diff --git a/include/qemu/log.h b/include/qemu/log.h
index a7c5b01571f..e0f4e406283 100644
--- a/include/qemu/log.h
+++ b/include/qemu/log.h
@@ -3,9 +3,16 @@
/* A small part of this API is split into its own header */
#include "qemu/log-for-trace.h"
+#include "qemu/rcu.h"
+
+typedef struct QemuLogFile {
+ struct rcu_head rcu;
+ FILE *fd;
+} QemuLogFile;
/* Private global variable, don't use */
-extern FILE *qemu_logfile;
+extern QemuLogFile *qemu_logfile;
+
/*
* The new API:
@@ -25,7 +32,16 @@ static inline bool qemu_log_enabled(void)
*/
static inline bool qemu_log_separate(void)
{
- return qemu_logfile != NULL && qemu_logfile != stderr;
+ QemuLogFile *logfile;
+ bool res = false;
+
+ rcu_read_lock();
+ logfile = atomic_rcu_read(&qemu_logfile);
+ if (logfile && logfile->fd != stderr) {
+ res = true;
+ }
+ rcu_read_unlock();
+ return res;
}
#define CPU_LOG_TB_OUT_ASM (1 << 0)
@@ -55,8 +71,15 @@ static inline bool qemu_log_separate(void)
static inline FILE *qemu_log_lock(void)
{
- qemu_flockfile(qemu_logfile);
- return logfile->fd;
+ QemuLogFile *logfile;
+ rcu_read_lock();
+ logfile = atomic_rcu_read(&qemu_logfile);
+ if (logfile) {
+ qemu_flockfile(logfile->fd);
+ return logfile->fd;
+ } else {
+ return NULL;
+ }
}
static inline void qemu_log_unlock(FILE *fd)
@@ -64,6 +87,7 @@ static inline void qemu_log_unlock(FILE *fd)
if (fd) {
qemu_funlockfile(fd);
}
+ rcu_read_unlock();
}
/* Logging functions: */
@@ -73,9 +97,14 @@ static inline void qemu_log_unlock(FILE *fd)
static inline void GCC_FMT_ATTR(1, 0)
qemu_log_vprintf(const char *fmt, va_list va)
{
- if (qemu_logfile) {
- vfprintf(qemu_logfile, fmt, va);
+ QemuLogFile *logfile;
+
+ rcu_read_lock();
+ logfile = atomic_rcu_read(&qemu_logfile);
+ if (logfile) {
+ vfprintf(logfile->fd, fmt, va);
}
+ rcu_read_unlock();
}
/* log only if a bit is set on the current loglevel mask:
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 0511266d85d..4f616ba38bf 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -2114,9 +2114,17 @@ static void tcg_dump_ops(TCGContext *s, bool have_prefs)
}
if (have_prefs || op->life) {
- for (; col < 40; ++col) {
- putc(' ', qemu_logfile);
+
+ QemuLogFile *logfile;
+
+ rcu_read_lock();
+ logfile = atomic_rcu_read(&qemu_logfile);
+ if (logfile) {
+ for (; col < 40; ++col) {
+ putc(' ', logfile->fd);
+ }
}
+ rcu_read_unlock();
}
if (op->life) {
diff --git a/util/log.c b/util/log.c
index 953a66b5a8d..867264da8d0 100644
--- a/util/log.c
+++ b/util/log.c
@@ -28,7 +28,7 @@
static char *logfilename;
static QemuMutex qemu_logfile_mutex;
-FILE *qemu_logfile;
+QemuLogFile *qemu_logfile;
int qemu_loglevel;
static int log_append = 0;
static GArray *debug_regions;
@@ -37,10 +37,14 @@ static GArray *debug_regions;
int qemu_log(const char *fmt, ...)
{
int ret = 0;
- if (qemu_logfile) {
+ QemuLogFile *logfile;
+
+ rcu_read_lock();
+ logfile = atomic_rcu_read(&qemu_logfile);
+ if (logfile) {
va_list ap;
va_start(ap, fmt);
- ret = vfprintf(qemu_logfile, fmt, ap);
+ ret = vfprintf(logfile->fd, fmt, ap);
va_end(ap);
/* Don't pass back error results. */
@@ -48,6 +52,7 @@ int qemu_log(const char *fmt, ...)
ret = 0;
}
}
+ rcu_read_unlock();
return ret;
}
@@ -56,12 +61,24 @@ static void __attribute__((__constructor__))
qemu_logfile_init(void)
qemu_mutex_init(&qemu_logfile_mutex);
}
+static void qemu_logfile_free(QemuLogFile *logfile)
+{
+ g_assert(logfile);
+
+ if (logfile->fd != stderr) {
+ fclose(logfile->fd);
+ }
+ g_free(logfile);
+}
+
static bool log_uses_own_buffers;
/* enable or disable low levels log */
void qemu_set_log(int log_flags)
{
bool need_to_open_file = false;
+ QemuLogFile *logfile;
+
qemu_loglevel = log_flags;
#ifdef CONFIG_TRACE_LOG
qemu_loglevel |= LOG_TRACE;
@@ -79,43 +96,47 @@ void qemu_set_log(int log_flags)
}
qemu_mutex_lock(&qemu_logfile_mutex);
if (qemu_logfile && !need_to_open_file) {
- qemu_mutex_unlock(&qemu_logfile_mutex);
- qemu_log_close();
+ logfile = qemu_logfile;
+ atomic_rcu_set(&qemu_logfile, NULL);
+ call_rcu(logfile, qemu_logfile_free, rcu);
} else if (!qemu_logfile && need_to_open_file) {
+ logfile = g_new0(QemuLogFile, 1);
if (logfilename) {
- qemu_logfile = fopen(logfilename, log_append ? "a" : "w");
- if (!qemu_logfile) {
+ logfile->fd = fopen(logfilename, log_append ? "a" : "w");
+ if (!logfile->fd) {
+ g_free(logfile);
perror(logfilename);
_exit(1);
}
/* In case we are a daemon redirect stderr to logfile */
if (is_daemonized()) {
- dup2(fileno(qemu_logfile), STDERR_FILENO);
- fclose(qemu_logfile);
+ dup2(fileno(logfile->fd), STDERR_FILENO);
+ fclose(logfile->fd);
/* This will skip closing logfile in qemu_log_close() */
- qemu_logfile = stderr;
+ logfile->fd = stderr;
}
} else {
/* Default to stderr if no log file specified */
assert(!is_daemonized());
- qemu_logfile = stderr;
+ logfile->fd = stderr;
}
/* must avoid mmap() usage of glibc by setting a buffer "by hand" */
if (log_uses_own_buffers) {
static char logfile_buf[4096];
- setvbuf(qemu_logfile, logfile_buf, _IOLBF, sizeof(logfile_buf));
+ setvbuf(logfile->fd, logfile_buf, _IOLBF, sizeof(logfile_buf));
} else {
#if defined(_WIN32)
/* Win32 doesn't support line-buffering, so use unbuffered output.
*/
- setvbuf(qemu_logfile, NULL, _IONBF, 0);
+ setvbuf(logfile->fd, NULL, _IONBF, 0);
#else
- setvbuf(qemu_logfile, NULL, _IOLBF, 0);
+ setvbuf(logfile->fd, NULL, _IOLBF, 0);
#endif
log_append = 1;
}
- qemu_mutex_unlock(&qemu_logfile_mutex);
+ atomic_rcu_set(&qemu_logfile, logfile);
}
+ qemu_mutex_unlock(&qemu_logfile_mutex);
}
void qemu_log_needs_buffers(void)
@@ -244,18 +265,27 @@ out:
/* fflush() the log file */
void qemu_log_flush(void)
{
- fflush(qemu_logfile);
+ QemuLogFile *logfile;
+
+ rcu_read_lock();
+ logfile = atomic_rcu_read(&qemu_logfile);
+ if (logfile) {
+ fflush(logfile->fd);
+ }
+ rcu_read_unlock();
}
/* Close the log file */
void qemu_log_close(void)
{
+ QemuLogFile *logfile;
+
qemu_mutex_lock(&qemu_logfile_mutex);
- if (qemu_logfile) {
- if (qemu_logfile != stderr) {
- fclose(qemu_logfile);
- }
- qemu_logfile = NULL;
+ logfile = qemu_logfile;
+
+ if (logfile) {
+ atomic_rcu_set(&qemu_logfile, NULL);
+ call_rcu(logfile, qemu_logfile_free, rcu);
}
qemu_mutex_unlock(&qemu_logfile_mutex);
}
--
2.20.1
- [PULL 06/25] iotests: Skip test 079 if it is not possible to create large files, (continued)
- [PULL 06/25] iotests: Skip test 079 if it is not possible to create large files, Alex Bennée, 2019/12/19
- [PULL 08/25] tests/test-util-filemonitor: Skip test on non-x86 Travis containers, Alex Bennée, 2019/12/19
- [PULL 09/25] travis.yml: Enable builds on arm64, ppc64le and s390x, Alex Bennée, 2019/12/19
- [PULL 07/25] tests/hd-geo-test: Skip test when images can not be created, Alex Bennée, 2019/12/19
- [PULL 10/25] ci: build out-of-tree, Alex Bennée, 2019/12/19
- [PULL 11/25] Fix double free issue in qemu_set_log_filename()., Alex Bennée, 2019/12/19
- [PULL 12/25] Cleaned up flow of code in qemu_set_log(), to simplify and clarify., Alex Bennée, 2019/12/19
- [PULL 13/25] Add a mutex to guarantee single writer to qemu_logfile handle., Alex Bennée, 2019/12/19
- [PULL 18/25] travis.yml: Remove the redundant clang-with-MAIN_SOFTMMU_TARGETS entry, Alex Bennée, 2019/12/19
- [PULL 21/25] linux-user: add target_mmap_complete tracepoint, Alex Bennée, 2019/12/19
- [PULL 15/25] Add use of RCU for qemu_logfile.,
Alex Bennée <=
- [PULL 20/25] linux-user: convert target_mmap debug to tracepoint, Alex Bennée, 2019/12/19
- [PULL 17/25] docker: gtester is no longer used, Alex Bennée, 2019/12/19
- [PULL 19/25] linux-user: convert target_mprotect debug to tracepoint, Alex Bennée, 2019/12/19
- [PULL 16/25] Added tests for close and change of logfile., Alex Bennée, 2019/12/19
- [PULL 25/25] tests/tcg: ensure we re-configure if configure.sh is updated, Alex Bennée, 2019/12/19
- [PULL 24/25] trace: replace hand-crafted pattern_glob with g_pattern_match_simple, Alex Bennée, 2019/12/19
- [PULL 23/25] linux-user: convert target_munmap debug to a tracepoint, Alex Bennée, 2019/12/19
- [PULL 22/25] linux-user: log page table changes under -d page, Alex Bennée, 2019/12/19
- [PULL 14/25] qemu_log_lock/unlock now preserves the qemu_logfile handle., Alex Bennée, 2019/12/19
- Re: [PULL 00/25] testing and logging updates, Peter Maydell, 2019/12/20