* [PULL for-7.1 01/36] util/log: Drop manual log buffering
2022-03-20 17:10 [PULL for-7.1 00/36] Logging cleanup and per-thread logfiles Richard Henderson
@ 2022-03-20 17:11 ` Richard Henderson
2022-03-23 13:54 ` Alex Bennée
2022-03-20 17:11 ` [PULL for-7.1 02/36] target/hexagon: Remove qemu_set_log in hexagon_translate_init Richard Henderson
` (35 subsequent siblings)
36 siblings, 1 reply; 60+ messages in thread
From: Richard Henderson @ 2022-03-20 17:11 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
This buffering was introduced during the Paleozoic: 9fa3e853531.
There has never been an explanation as to why we may not allow
glibc to allocate the file buffer itself. We certainly have
many other uses of mmap and malloc during user-only startup,
so presumably whatever the issue was, it has been fixed during
the preceeding 18 years.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/qemu/log.h | 1 -
bsd-user/main.c | 1 -
linux-user/main.c | 1 -
util/log.c | 21 ++++-----------------
4 files changed, 4 insertions(+), 20 deletions(-)
diff --git a/include/qemu/log.h b/include/qemu/log.h
index 9b80660207..08b0cd0bb3 100644
--- a/include/qemu/log.h
+++ b/include/qemu/log.h
@@ -148,7 +148,6 @@ typedef struct QEMULogItem {
extern const QEMULogItem qemu_log_items[];
void qemu_set_log(int log_flags);
-void qemu_log_needs_buffers(void);
void qemu_set_log_filename(const char *filename, Error **errp);
void qemu_set_dfilter_ranges(const char *ranges, Error **errp);
bool qemu_log_in_addr_range(uint64_t addr);
diff --git a/bsd-user/main.c b/bsd-user/main.c
index 88d347d05e..08f43af235 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -405,7 +405,6 @@ int main(int argc, char **argv)
}
/* init debug */
- qemu_log_needs_buffers();
qemu_set_log_filename(log_file, &error_fatal);
if (log_mask) {
int mask;
diff --git a/linux-user/main.c b/linux-user/main.c
index fbc9bcfd5f..2b06350688 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -679,7 +679,6 @@ int main(int argc, char **argv, char **envp)
log_mask = last_log_mask | (enable_strace ? LOG_STRACE : 0);
if (log_mask) {
- qemu_log_needs_buffers();
qemu_set_log(log_mask);
}
diff --git a/util/log.c b/util/log.c
index 2ee1500bee..ffa66a267e 100644
--- a/util/log.c
+++ b/util/log.c
@@ -72,8 +72,6 @@ static void qemu_logfile_free(QemuLogFile *logfile)
g_free(logfile);
}
-static bool log_uses_own_buffers;
-
/* enable or disable low levels log */
void qemu_set_log(int log_flags)
{
@@ -121,29 +119,18 @@ void qemu_set_log(int log_flags)
assert(!is_daemonized());
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(logfile->fd, logfile_buf, _IOLBF, sizeof(logfile_buf));
- } else {
#if defined(_WIN32)
- /* Win32 doesn't support line-buffering, so use unbuffered output. */
- setvbuf(logfile->fd, NULL, _IONBF, 0);
+ /* Win32 doesn't support line-buffering, so use unbuffered output. */
+ setvbuf(logfile->fd, NULL, _IONBF, 0);
#else
- setvbuf(logfile->fd, NULL, _IOLBF, 0);
+ setvbuf(logfile->fd, NULL, _IOLBF, 0);
#endif
- log_append = 1;
- }
+ log_append = 1;
qatomic_rcu_set(&qemu_logfile, logfile);
}
}
-void qemu_log_needs_buffers(void)
-{
- log_uses_own_buffers = true;
-}
-
/*
* Allow the user to include %d in their logfile which will be
* substituted with the current PID. This is useful for debugging many
--
2.25.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* Re: [PULL for-7.1 01/36] util/log: Drop manual log buffering
2022-03-20 17:11 ` [PULL for-7.1 01/36] util/log: Drop manual log buffering Richard Henderson
@ 2022-03-23 13:54 ` Alex Bennée
0 siblings, 0 replies; 60+ messages in thread
From: Alex Bennée @ 2022-03-23 13:54 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> This buffering was introduced during the Paleozoic: 9fa3e853531.
>
> There has never been an explanation as to why we may not allow
> glibc to allocate the file buffer itself. We certainly have
> many other uses of mmap and malloc during user-only startup,
> so presumably whatever the issue was, it has been fixed during
> the preceeding 18 years.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PULL for-7.1 02/36] target/hexagon: Remove qemu_set_log in hexagon_translate_init
2022-03-20 17:10 [PULL for-7.1 00/36] Logging cleanup and per-thread logfiles Richard Henderson
2022-03-20 17:11 ` [PULL for-7.1 01/36] util/log: Drop manual log buffering Richard Henderson
@ 2022-03-20 17:11 ` Richard Henderson
2022-03-23 13:56 ` Alex Bennée
2022-03-20 17:11 ` [PULL for-7.1 03/36] util/log: Pass Error pointer to qemu_set_log Richard Henderson
` (34 subsequent siblings)
36 siblings, 1 reply; 60+ messages in thread
From: Richard Henderson @ 2022-03-20 17:11 UTC (permalink / raw)
To: qemu-devel; +Cc: Taylor Simpson, alex.bennee
This code appears to be trying to make sure there is a logfile.
But that's already true -- the logfile will either be set by -D,
or will be stderr. In either case, not appropriate here.
Cc: Taylor Simpson <tsimpson@quicinc.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/hexagon/translate.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/target/hexagon/translate.c b/target/hexagon/translate.c
index b6f541ecb2..98f1452cad 100644
--- a/target/hexagon/translate.c
+++ b/target/hexagon/translate.c
@@ -874,12 +874,6 @@ void hexagon_translate_init(void)
opcode_init();
- if (HEX_DEBUG) {
- if (!qemu_logfile) {
- qemu_set_log(qemu_loglevel);
- }
- }
-
for (i = 0; i < TOTAL_PER_THREAD_REGS; i++) {
hex_gpr[i] = tcg_global_mem_new(cpu_env,
offsetof(CPUHexagonState, gpr[i]),
--
2.25.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PULL for-7.1 03/36] util/log: Pass Error pointer to qemu_set_log
2022-03-20 17:10 [PULL for-7.1 00/36] Logging cleanup and per-thread logfiles Richard Henderson
2022-03-20 17:11 ` [PULL for-7.1 01/36] util/log: Drop manual log buffering Richard Henderson
2022-03-20 17:11 ` [PULL for-7.1 02/36] target/hexagon: Remove qemu_set_log in hexagon_translate_init Richard Henderson
@ 2022-03-20 17:11 ` Richard Henderson
2022-03-20 21:50 ` Philippe Mathieu-Daudé
2022-03-20 17:11 ` [PULL for-7.1 04/36] os-posix: Use qemu_log_enabled Richard Henderson
` (33 subsequent siblings)
36 siblings, 1 reply; 60+ messages in thread
From: Richard Henderson @ 2022-03-20 17:11 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
Do not force exit within qemu_set_log. Pass an Error
value back up the stack as per usual.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/qemu/log.h | 2 +-
bsd-user/main.c | 2 +-
linux-user/main.c | 2 +-
monitor/misc.c | 7 ++++++-
qemu-img.c | 2 +-
qemu-io.c | 2 +-
qemu-nbd.c | 2 +-
scsi/qemu-pr-helper.c | 2 +-
softmmu/vl.c | 4 ++--
storage-daemon/qemu-storage-daemon.c | 2 +-
tests/unit/test-logging.c | 2 +-
util/log.c | 10 +++++-----
12 files changed, 22 insertions(+), 17 deletions(-)
diff --git a/include/qemu/log.h b/include/qemu/log.h
index 08b0cd0bb3..64f28785ae 100644
--- a/include/qemu/log.h
+++ b/include/qemu/log.h
@@ -147,7 +147,7 @@ typedef struct QEMULogItem {
extern const QEMULogItem qemu_log_items[];
-void qemu_set_log(int log_flags);
+void qemu_set_log(int log_flags, Error **errp);
void qemu_set_log_filename(const char *filename, Error **errp);
void qemu_set_dfilter_ranges(const char *ranges, Error **errp);
bool qemu_log_in_addr_range(uint64_t addr);
diff --git a/bsd-user/main.c b/bsd-user/main.c
index 08f43af235..0c8aa1ae90 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -414,7 +414,7 @@ int main(int argc, char **argv)
qemu_print_log_usage(stdout);
exit(1);
}
- qemu_set_log(mask);
+ qemu_set_log(mask, &error_fatal);
}
if (optind >= argc) {
diff --git a/linux-user/main.c b/linux-user/main.c
index 2b06350688..6aed4929ab 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -679,7 +679,7 @@ int main(int argc, char **argv, char **envp)
log_mask = last_log_mask | (enable_strace ? LOG_STRACE : 0);
if (log_mask) {
- qemu_set_log(log_mask);
+ qemu_set_log(log_mask, &error_fatal);
}
if (!trace_init_backends()) {
diff --git a/monitor/misc.c b/monitor/misc.c
index b1839cb8ee..8e8b8e18c7 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -429,6 +429,7 @@ static void hmp_log(Monitor *mon, const QDict *qdict)
{
int mask;
const char *items = qdict_get_str(qdict, "items");
+ Error *err = NULL;
if (!strcmp(items, "none")) {
mask = 0;
@@ -439,7 +440,11 @@ static void hmp_log(Monitor *mon, const QDict *qdict)
return;
}
}
- qemu_set_log(mask);
+
+ qemu_set_log(mask, &err);
+ if (err) {
+ error_report_err(err);
+ }
}
static void hmp_singlestep(Monitor *mon, const QDict *qdict)
diff --git a/qemu-img.c b/qemu-img.c
index 5dffb3e616..498f6d3646 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -5442,7 +5442,7 @@ int main(int argc, char **argv)
exit(1);
}
trace_init_file();
- qemu_set_log(LOG_TRACE);
+ qemu_set_log(LOG_TRACE, &error_fatal);
/* find the command */
for (cmd = img_cmds; cmd->name != NULL; cmd++) {
diff --git a/qemu-io.c b/qemu-io.c
index 3924639b92..49957a980e 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -634,7 +634,7 @@ int main(int argc, char **argv)
exit(1);
}
trace_init_file();
- qemu_set_log(LOG_TRACE);
+ qemu_set_log(LOG_TRACE, &error_fatal);
/* initialize commands */
qemuio_add_command(&quit_cmd);
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 713e7557a9..bf9c5fedce 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -804,7 +804,7 @@ int main(int argc, char **argv)
exit(1);
}
trace_init_file();
- qemu_set_log(LOG_TRACE);
+ qemu_set_log(LOG_TRACE, &error_fatal);
socket_activation = check_socket_activation();
if (socket_activation == 0) {
diff --git a/scsi/qemu-pr-helper.c b/scsi/qemu-pr-helper.c
index f281daeced..1f2a84c534 100644
--- a/scsi/qemu-pr-helper.c
+++ b/scsi/qemu-pr-helper.c
@@ -1001,7 +1001,7 @@ int main(int argc, char **argv)
exit(EXIT_FAILURE);
}
trace_init_file();
- qemu_set_log(LOG_TRACE);
+ qemu_set_log(LOG_TRACE, &error_fatal);
#ifdef CONFIG_MPATH
dm_init();
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 0b81f61535..73296015af 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2569,9 +2569,9 @@ static void qemu_process_early_options(void)
qemu_print_log_usage(stdout);
exit(1);
}
- qemu_set_log(mask);
+ qemu_set_log(mask, &error_fatal);
} else {
- qemu_set_log(0);
+ qemu_set_log(0, &error_fatal);
}
qemu_add_default_firmwarepath();
diff --git a/storage-daemon/qemu-storage-daemon.c b/storage-daemon/qemu-storage-daemon.c
index dd18b2cde8..cdaf69f764 100644
--- a/storage-daemon/qemu-storage-daemon.c
+++ b/storage-daemon/qemu-storage-daemon.c
@@ -392,7 +392,7 @@ int main(int argc, char *argv[])
if (!trace_init_backends()) {
return EXIT_FAILURE;
}
- qemu_set_log(LOG_TRACE);
+ qemu_set_log(LOG_TRACE, &error_fatal);
qemu_init_main_loop(&error_fatal);
process_options(argc, argv, false);
diff --git a/tests/unit/test-logging.c b/tests/unit/test-logging.c
index ccb819f193..93c6d666e8 100644
--- a/tests/unit/test-logging.c
+++ b/tests/unit/test-logging.c
@@ -123,7 +123,7 @@ static void test_logfile_write(gconstpointer data)
* In cases where a logging backend other than log is used,
* this is needed.
*/
- qemu_set_log(CPU_LOG_TB_OUT_ASM);
+ qemu_set_log(CPU_LOG_TB_OUT_ASM, &error_abort);
file_path = g_build_filename(dir, "qemu_test_log_write0.log", NULL);
file_path1 = g_build_filename(dir, "qemu_test_log_write1.log", NULL);
diff --git a/util/log.c b/util/log.c
index ffa66a267e..00773ab717 100644
--- a/util/log.c
+++ b/util/log.c
@@ -73,7 +73,7 @@ static void qemu_logfile_free(QemuLogFile *logfile)
}
/* enable or disable low levels log */
-void qemu_set_log(int log_flags)
+void qemu_set_log(int log_flags, Error **errp)
{
bool need_to_open_file = false;
QemuLogFile *logfile;
@@ -103,9 +103,9 @@ void qemu_set_log(int log_flags)
if (logfilename) {
logfile->fd = fopen(logfilename, log_append ? "a" : "w");
if (!logfile->fd) {
- g_free(logfile);
- perror(logfilename);
- _exit(1);
+ error_setg_errno(errp, errno, "Error opening logfile %s",
+ logfilename);
+ return;
}
/* In case we are a daemon redirect stderr to logfile */
if (is_daemonized()) {
@@ -159,7 +159,7 @@ void qemu_set_log_filename(const char *filename, Error **errp)
}
qemu_log_close();
- qemu_set_log(qemu_loglevel);
+ qemu_set_log(qemu_loglevel, errp);
}
/* Returns true if addr is in our debug filter or no filter defined
--
2.25.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* Re: [PULL for-7.1 03/36] util/log: Pass Error pointer to qemu_set_log
2022-03-20 17:11 ` [PULL for-7.1 03/36] util/log: Pass Error pointer to qemu_set_log Richard Henderson
@ 2022-03-20 21:50 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 60+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-20 21:50 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: alex.bennee
On 20/3/22 18:11, Richard Henderson wrote:
> Do not force exit within qemu_set_log. Pass an Error
> value back up the stack as per usual.
"Pass an Error value back up the stack [and return a boolean
value] as per usual.", see:
https://lore.kernel.org/qemu-devel/87bky6ugmx.fsf@pond.sub.org/
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> include/qemu/log.h | 2 +-
> bsd-user/main.c | 2 +-
> linux-user/main.c | 2 +-
> monitor/misc.c | 7 ++++++-
> qemu-img.c | 2 +-
> qemu-io.c | 2 +-
> qemu-nbd.c | 2 +-
> scsi/qemu-pr-helper.c | 2 +-
> softmmu/vl.c | 4 ++--
> storage-daemon/qemu-storage-daemon.c | 2 +-
> tests/unit/test-logging.c | 2 +-
> util/log.c | 10 +++++-----
> 12 files changed, 22 insertions(+), 17 deletions(-)
>
> diff --git a/include/qemu/log.h b/include/qemu/log.h
> index 08b0cd0bb3..64f28785ae 100644
> --- a/include/qemu/log.h
> +++ b/include/qemu/log.h
> @@ -147,7 +147,7 @@ typedef struct QEMULogItem {
>
> extern const QEMULogItem qemu_log_items[];
>
> -void qemu_set_log(int log_flags);
> +void qemu_set_log(int log_flags, Error **errp);
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PULL for-7.1 04/36] os-posix: Use qemu_log_enabled
2022-03-20 17:10 [PULL for-7.1 00/36] Logging cleanup and per-thread logfiles Richard Henderson
` (2 preceding siblings ...)
2022-03-20 17:11 ` [PULL for-7.1 03/36] util/log: Pass Error pointer to qemu_set_log Richard Henderson
@ 2022-03-20 17:11 ` Richard Henderson
2022-03-20 21:51 ` Philippe Mathieu-Daudé
2022-03-23 16:41 ` Alex Bennée
2022-03-20 17:11 ` [PULL for-7.1 05/36] util/log: Move qemu_log_lock, qemu_log_unlock out of line Richard Henderson
` (32 subsequent siblings)
36 siblings, 2 replies; 60+ messages in thread
From: Richard Henderson @ 2022-03-20 17:11 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
Do not reference qemu_logfile directly;
use the predicate provided by qemu/log.h.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
os-posix.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/os-posix.c b/os-posix.c
index 24692c8593..20d31fc518 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -292,7 +292,7 @@ void os_setup_post(void)
dup2(fd, 0);
dup2(fd, 1);
/* In case -D is given do not redirect stderr to /dev/null */
- if (!qemu_logfile) {
+ if (!qemu_log_enabled()) {
dup2(fd, 2);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PULL for-7.1 05/36] util/log: Move qemu_log_lock, qemu_log_unlock out of line
2022-03-20 17:10 [PULL for-7.1 00/36] Logging cleanup and per-thread logfiles Richard Henderson
` (3 preceding siblings ...)
2022-03-20 17:11 ` [PULL for-7.1 04/36] os-posix: Use qemu_log_enabled Richard Henderson
@ 2022-03-20 17:11 ` Richard Henderson
2022-03-20 21:52 ` Philippe Mathieu-Daudé
2022-03-23 16:42 ` Alex Bennée
2022-03-20 17:11 ` [PULL for-7.1 06/36] util/log: Treat qemu_log_lock like trylock Richard Henderson
` (31 subsequent siblings)
36 siblings, 2 replies; 60+ messages in thread
From: Richard Henderson @ 2022-03-20 17:11 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/qemu/log.h | 28 +++-------------------------
util/log.c | 23 +++++++++++++++++++++++
2 files changed, 26 insertions(+), 25 deletions(-)
diff --git a/include/qemu/log.h b/include/qemu/log.h
index 64f28785ae..10e284291c 100644
--- a/include/qemu/log.h
+++ b/include/qemu/log.h
@@ -65,32 +65,10 @@ static inline bool qemu_log_separate(void)
/* LOG_STRACE is used for user-mode strace logging. */
#define LOG_STRACE (1 << 19)
-/* Lock output for a series of related logs. Since this is not needed
- * for a single qemu_log / qemu_log_mask / qemu_log_mask_and_addr, we
- * assume that qemu_loglevel_mask has already been tested, and that
- * qemu_loglevel is never set when qemu_logfile is unset.
- */
+/* Lock/unlock output. */
-static inline FILE *qemu_log_lock(void)
-{
- QemuLogFile *logfile;
- rcu_read_lock();
- logfile = qatomic_rcu_read(&qemu_logfile);
- if (logfile) {
- qemu_flockfile(logfile->fd);
- return logfile->fd;
- } else {
- return NULL;
- }
-}
-
-static inline void qemu_log_unlock(FILE *fd)
-{
- if (fd) {
- qemu_funlockfile(fd);
- }
- rcu_read_unlock();
-}
+FILE *qemu_log_lock(void);
+void qemu_log_unlock(FILE *fd);
/* Logging functions: */
diff --git a/util/log.c b/util/log.c
index 00773ab717..248db6211e 100644
--- a/util/log.c
+++ b/util/log.c
@@ -34,6 +34,29 @@ int qemu_loglevel;
static int log_append = 0;
static GArray *debug_regions;
+/* Lock/unlock output. */
+
+FILE *qemu_log_lock(void)
+{
+ QemuLogFile *logfile;
+ rcu_read_lock();
+ logfile = qatomic_rcu_read(&qemu_logfile);
+ if (logfile) {
+ qemu_flockfile(logfile->fd);
+ return logfile->fd;
+ } else {
+ return NULL;
+ }
+}
+
+void qemu_log_unlock(FILE *fd)
+{
+ if (fd) {
+ qemu_funlockfile(fd);
+ }
+ rcu_read_unlock();
+}
+
/* Return the number of characters emitted. */
int qemu_log(const char *fmt, ...)
{
--
2.25.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* Re: [PULL for-7.1 05/36] util/log: Move qemu_log_lock, qemu_log_unlock out of line
2022-03-20 17:11 ` [PULL for-7.1 05/36] util/log: Move qemu_log_lock, qemu_log_unlock out of line Richard Henderson
@ 2022-03-20 21:52 ` Philippe Mathieu-Daudé
2022-03-23 16:42 ` Alex Bennée
1 sibling, 0 replies; 60+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-20 21:52 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: alex.bennee
On 20/3/22 18:11, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> include/qemu/log.h | 28 +++-------------------------
> util/log.c | 23 +++++++++++++++++++++++
> 2 files changed, 26 insertions(+), 25 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PULL for-7.1 05/36] util/log: Move qemu_log_lock, qemu_log_unlock out of line
2022-03-20 17:11 ` [PULL for-7.1 05/36] util/log: Move qemu_log_lock, qemu_log_unlock out of line Richard Henderson
2022-03-20 21:52 ` Philippe Mathieu-Daudé
@ 2022-03-23 16:42 ` Alex Bennée
1 sibling, 0 replies; 60+ messages in thread
From: Alex Bennée @ 2022-03-23 16:42 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PULL for-7.1 06/36] util/log: Treat qemu_log_lock like trylock
2022-03-20 17:10 [PULL for-7.1 00/36] Logging cleanup and per-thread logfiles Richard Henderson
` (4 preceding siblings ...)
2022-03-20 17:11 ` [PULL for-7.1 05/36] util/log: Move qemu_log_lock, qemu_log_unlock out of line Richard Henderson
@ 2022-03-20 17:11 ` Richard Henderson
2022-03-23 16:43 ` Alex Bennée
2022-03-20 17:11 ` [PULL for-7.1 07/36] hw/xen: Split out xen_pv_output_msg Richard Henderson
` (30 subsequent siblings)
36 siblings, 1 reply; 60+ messages in thread
From: Richard Henderson @ 2022-03-20 17:11 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
Do not require qemu_log_unlock to be called when
qemu_log_lock returns NULL.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
util/log.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/util/log.c b/util/log.c
index 248db6211e..b1651d8122 100644
--- a/util/log.c
+++ b/util/log.c
@@ -39,12 +39,14 @@ static GArray *debug_regions;
FILE *qemu_log_lock(void)
{
QemuLogFile *logfile;
+
rcu_read_lock();
logfile = qatomic_rcu_read(&qemu_logfile);
if (logfile) {
qemu_flockfile(logfile->fd);
return logfile->fd;
} else {
+ rcu_read_unlock();
return NULL;
}
}
@@ -53,8 +55,8 @@ void qemu_log_unlock(FILE *fd)
{
if (fd) {
qemu_funlockfile(fd);
+ rcu_read_unlock();
}
- rcu_read_unlock();
}
/* Return the number of characters emitted. */
--
2.25.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PULL for-7.1 07/36] hw/xen: Split out xen_pv_output_msg
2022-03-20 17:10 [PULL for-7.1 00/36] Logging cleanup and per-thread logfiles Richard Henderson
` (5 preceding siblings ...)
2022-03-20 17:11 ` [PULL for-7.1 06/36] util/log: Treat qemu_log_lock like trylock Richard Henderson
@ 2022-03-20 17:11 ` Richard Henderson
2022-03-23 17:19 ` Alex Bennée
2022-03-20 17:11 ` [PULL for-7.1 08/36] *: Use fprintf between qemu_log_lock/unlock Richard Henderson
` (29 subsequent siblings)
36 siblings, 1 reply; 60+ messages in thread
From: Richard Henderson @ 2022-03-20 17:11 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
Do not replicate the individual logging statements.
Use qemu_log_lock/unlock instead of qemu_log directly.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
hw/xen/xen_pvdev.c | 44 ++++++++++++++++++++++++--------------------
1 file changed, 24 insertions(+), 20 deletions(-)
diff --git a/hw/xen/xen_pvdev.c b/hw/xen/xen_pvdev.c
index 8ab458922a..355bbda60d 100644
--- a/hw/xen/xen_pvdev.c
+++ b/hw/xen/xen_pvdev.c
@@ -196,34 +196,38 @@ const char *xenbus_strstate(enum xenbus_state state)
* 2 == noisy debug messages (logfile only).
* 3 == will flood your log (logfile only).
*/
+static void xen_pv_output_msg(struct XenLegacyDevice *xendev,
+ FILE *f, const char *fmt, va_list args)
+{
+ if (xendev) {
+ fprintf(f, "xen be: %s: ", xendev->name);
+ } else {
+ fprintf(f, "xen be core: ");
+ }
+ vfprintf(f, fmt, args);
+}
+
void xen_pv_printf(struct XenLegacyDevice *xendev, int msg_level,
const char *fmt, ...)
{
+ FILE *logfile;
va_list args;
- if (xendev) {
- if (msg_level > xendev->debug) {
- return;
- }
- qemu_log("xen be: %s: ", xendev->name);
- if (msg_level == 0) {
- fprintf(stderr, "xen be: %s: ", xendev->name);
- }
- } else {
- if (msg_level > debug) {
- return;
- }
- qemu_log("xen be core: ");
- if (msg_level == 0) {
- fprintf(stderr, "xen be core: ");
- }
+ if (msg_level > (xendev ? xendev->debug : debug)) {
+ return;
}
- va_start(args, fmt);
- qemu_log_vprintf(fmt, args);
- va_end(args);
+
+ logfile = qemu_log_lock();
+ if (logfile) {
+ va_start(args, fmt);
+ xen_pv_output_msg(xendev, logfile, fmt, args);
+ va_end(args);
+ qemu_log_unlock(logfile);
+ }
+
if (msg_level == 0) {
va_start(args, fmt);
- vfprintf(stderr, fmt, args);
+ xen_pv_output_msg(xendev, stderr, fmt, args);
va_end(args);
}
qemu_log_flush();
--
2.25.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* Re: [PULL for-7.1 07/36] hw/xen: Split out xen_pv_output_msg
2022-03-20 17:11 ` [PULL for-7.1 07/36] hw/xen: Split out xen_pv_output_msg Richard Henderson
@ 2022-03-23 17:19 ` Alex Bennée
0 siblings, 0 replies; 60+ messages in thread
From: Alex Bennée @ 2022-03-23 17:19 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> Do not replicate the individual logging statements.
> Use qemu_log_lock/unlock instead of qemu_log directly.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
I don't know why Xen wants to have it's own special handling for errors.
It seems a lot of the xen_pv_printf's of level 0 should probably be
using qemu's report_err infrastructure. But this change itself seems
sane:
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PULL for-7.1 08/36] *: Use fprintf between qemu_log_lock/unlock
2022-03-20 17:10 [PULL for-7.1 00/36] Logging cleanup and per-thread logfiles Richard Henderson
` (6 preceding siblings ...)
2022-03-20 17:11 ` [PULL for-7.1 07/36] hw/xen: Split out xen_pv_output_msg Richard Henderson
@ 2022-03-20 17:11 ` Richard Henderson
2022-03-23 17:22 ` Alex Bennée
2022-03-20 17:11 ` [PULL for-7.1 09/36] util/log: Remove qemu_log_vprintf Richard Henderson
` (28 subsequent siblings)
36 siblings, 1 reply; 60+ messages in thread
From: Richard Henderson @ 2022-03-20 17:11 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
Inside qemu_log, we perform qemu_log_lock/unlock, which need
not be done if we have already performed the lock beforehand.
Always check the result of qemu_log_lock -- only checking
qemu_loglevel_mask races with the acquisition of the lock
on the logfile.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/exec/log.h | 2 +-
accel/tcg/cpu-exec.c | 16 ++---
accel/tcg/translate-all.c | 117 +++++++++++++++++++-----------------
accel/tcg/translator.c | 10 +--
cpu.c | 8 +--
hw/net/can/can_sja1000.c | 25 ++++----
net/can/can_socketcan.c | 24 ++++----
target/i386/tcg/translate.c | 14 +++--
tcg/tcg.c | 74 +++++++++++++----------
9 files changed, 158 insertions(+), 132 deletions(-)
diff --git a/include/exec/log.h b/include/exec/log.h
index 3c7fa65ead..6a53199d44 100644
--- a/include/exec/log.h
+++ b/include/exec/log.h
@@ -73,7 +73,7 @@ static inline void log_page_dump(const char *operation)
{
FILE *logfile = qemu_log_lock();
if (logfile) {
- qemu_log("page layout changed following %s\n", operation);
+ fprintf(logfile, "page layout changed following %s\n", operation);
page_dump(logfile);
}
qemu_log_unlock(logfile);
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index c997c2e8e0..7bc489101f 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -216,16 +216,18 @@ static inline void log_cpu_exec(target_ulong pc, CPUState *cpu,
#if defined(DEBUG_DISAS)
if (qemu_loglevel_mask(CPU_LOG_TB_CPU)) {
FILE *logfile = qemu_log_lock();
- int flags = 0;
+ if (logfile) {
+ int flags = 0;
- if (qemu_loglevel_mask(CPU_LOG_TB_FPU)) {
- flags |= CPU_DUMP_FPU;
- }
+ if (qemu_loglevel_mask(CPU_LOG_TB_FPU)) {
+ flags |= CPU_DUMP_FPU;
+ }
#if defined(TARGET_I386)
- flags |= CPU_DUMP_CCOP;
+ flags |= CPU_DUMP_CCOP;
#endif
- log_cpu_state(cpu, flags);
- qemu_log_unlock(logfile);
+ log_cpu_state(cpu, flags);
+ qemu_log_unlock(logfile);
+ }
}
#endif /* DEBUG_DISAS */
}
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 5971cd53ab..4af7455522 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -1529,68 +1529,75 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
if (qemu_loglevel_mask(CPU_LOG_TB_OUT_ASM) &&
qemu_log_in_addr_range(tb->pc)) {
FILE *logfile = qemu_log_lock();
- int code_size, data_size;
- const tcg_target_ulong *rx_data_gen_ptr;
- size_t chunk_start;
- int insn = 0;
+ if (logfile) {
+ int code_size, data_size;
+ const tcg_target_ulong *rx_data_gen_ptr;
+ size_t chunk_start;
+ int insn = 0;
- if (tcg_ctx->data_gen_ptr) {
- rx_data_gen_ptr = tcg_splitwx_to_rx(tcg_ctx->data_gen_ptr);
- code_size = (const void *)rx_data_gen_ptr - tb->tc.ptr;
- data_size = gen_code_size - code_size;
- } else {
- rx_data_gen_ptr = 0;
- code_size = gen_code_size;
- data_size = 0;
- }
-
- /* Dump header and the first instruction */
- qemu_log("OUT: [size=%d]\n", gen_code_size);
- qemu_log(" -- guest addr 0x" TARGET_FMT_lx " + tb prologue\n",
- tcg_ctx->gen_insn_data[insn][0]);
- chunk_start = tcg_ctx->gen_insn_end_off[insn];
- log_disas(tb->tc.ptr, chunk_start);
-
- /*
- * Dump each instruction chunk, wrapping up empty chunks into
- * the next instruction. The whole array is offset so the
- * first entry is the beginning of the 2nd instruction.
- */
- while (insn < tb->icount) {
- size_t chunk_end = tcg_ctx->gen_insn_end_off[insn];
- if (chunk_end > chunk_start) {
- qemu_log(" -- guest addr 0x" TARGET_FMT_lx "\n",
- tcg_ctx->gen_insn_data[insn][0]);
- log_disas(tb->tc.ptr + chunk_start, chunk_end - chunk_start);
- chunk_start = chunk_end;
+ if (tcg_ctx->data_gen_ptr) {
+ rx_data_gen_ptr = tcg_splitwx_to_rx(tcg_ctx->data_gen_ptr);
+ code_size = (const void *)rx_data_gen_ptr - tb->tc.ptr;
+ data_size = gen_code_size - code_size;
+ } else {
+ rx_data_gen_ptr = 0;
+ code_size = gen_code_size;
+ data_size = 0;
}
- insn++;
- }
- if (chunk_start < code_size) {
- qemu_log(" -- tb slow paths + alignment\n");
- log_disas(tb->tc.ptr + chunk_start, code_size - chunk_start);
- }
+ /* Dump header and the first instruction */
+ fprintf(logfile, "OUT: [size=%d]\n", gen_code_size);
+ fprintf(logfile,
+ " -- guest addr 0x" TARGET_FMT_lx " + tb prologue\n",
+ tcg_ctx->gen_insn_data[insn][0]);
+ chunk_start = tcg_ctx->gen_insn_end_off[insn];
+ disas(logfile, tb->tc.ptr, chunk_start);
- /* Finally dump any data we may have after the block */
- if (data_size) {
- int i;
- qemu_log(" data: [size=%d]\n", data_size);
- for (i = 0; i < data_size / sizeof(tcg_target_ulong); i++) {
- if (sizeof(tcg_target_ulong) == 8) {
- qemu_log("0x%08" PRIxPTR ": .quad 0x%016" TCG_PRIlx "\n",
- (uintptr_t)&rx_data_gen_ptr[i], rx_data_gen_ptr[i]);
- } else if (sizeof(tcg_target_ulong) == 4) {
- qemu_log("0x%08" PRIxPTR ": .long 0x%08" TCG_PRIlx "\n",
- (uintptr_t)&rx_data_gen_ptr[i], rx_data_gen_ptr[i]);
- } else {
- qemu_build_not_reached();
+ /*
+ * Dump each instruction chunk, wrapping up empty chunks into
+ * the next instruction. The whole array is offset so the
+ * first entry is the beginning of the 2nd instruction.
+ */
+ while (insn < tb->icount) {
+ size_t chunk_end = tcg_ctx->gen_insn_end_off[insn];
+ if (chunk_end > chunk_start) {
+ fprintf(logfile, " -- guest addr 0x" TARGET_FMT_lx "\n",
+ tcg_ctx->gen_insn_data[insn][0]);
+ disas(logfile, tb->tc.ptr + chunk_start,
+ chunk_end - chunk_start);
+ chunk_start = chunk_end;
+ }
+ insn++;
+ }
+
+ if (chunk_start < code_size) {
+ fprintf(logfile, " -- tb slow paths + alignment\n");
+ disas(logfile, tb->tc.ptr + chunk_start,
+ code_size - chunk_start);
+ }
+
+ /* Finally dump any data we may have after the block */
+ if (data_size) {
+ int i;
+ fprintf(logfile, " data: [size=%d]\n", data_size);
+ for (i = 0; i < data_size / sizeof(tcg_target_ulong); i++) {
+ if (sizeof(tcg_target_ulong) == 8) {
+ fprintf(logfile,
+ "0x%08" PRIxPTR ": .quad 0x%016" TCG_PRIlx "\n",
+ (uintptr_t)&rx_data_gen_ptr[i], rx_data_gen_ptr[i]);
+ } else if (sizeof(tcg_target_ulong) == 4) {
+ fprintf(logfile,
+ "0x%08" PRIxPTR ": .long 0x%08" TCG_PRIlx "\n",
+ (uintptr_t)&rx_data_gen_ptr[i], rx_data_gen_ptr[i]);
+ } else {
+ qemu_build_not_reached();
+ }
}
}
+ fprintf(logfile, "\n");
+ qemu_log_flush();
+ qemu_log_unlock(logfile);
}
- qemu_log("\n");
- qemu_log_flush();
- qemu_log_unlock(logfile);
}
#endif
diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index f06c314266..90da944dab 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -140,10 +140,12 @@ void translator_loop(const TranslatorOps *ops, DisasContextBase *db,
if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)
&& qemu_log_in_addr_range(db->pc_first)) {
FILE *logfile = qemu_log_lock();
- qemu_log("----------------\n");
- ops->disas_log(db, cpu);
- qemu_log("\n");
- qemu_log_unlock(logfile);
+ if (logfile) {
+ fprintf(logfile, "----------------\n");
+ ops->disas_log(db, cpu);
+ fprintf(logfile, "\n");
+ qemu_log_unlock(logfile);
+ }
}
#endif
}
diff --git a/cpu.c b/cpu.c
index be1f8b074c..159d0b0d1d 100644
--- a/cpu.c
+++ b/cpu.c
@@ -401,10 +401,10 @@ void cpu_abort(CPUState *cpu, const char *fmt, ...)
cpu_dump_state(cpu, stderr, CPU_DUMP_FPU | CPU_DUMP_CCOP);
if (qemu_log_separate()) {
FILE *logfile = qemu_log_lock();
- qemu_log("qemu: fatal: ");
- qemu_log_vprintf(fmt, ap2);
- qemu_log("\n");
- log_cpu_state(cpu, CPU_DUMP_FPU | CPU_DUMP_CCOP);
+ fprintf(logfile, "qemu: fatal: ");
+ vfprintf(logfile, fmt, ap2);
+ fprintf(logfile, "\n");
+ cpu_dump_state(cpu, logfile, CPU_DUMP_FPU | CPU_DUMP_CCOP);
qemu_log_flush();
qemu_log_unlock(logfile);
qemu_log_close();
diff --git a/hw/net/can/can_sja1000.c b/hw/net/can/can_sja1000.c
index 3ba803e947..919e93664d 100644
--- a/hw/net/can/can_sja1000.c
+++ b/hw/net/can/can_sja1000.c
@@ -246,22 +246,23 @@ int can_sja_accept_filter(CanSJA1000State *s,
static void can_display_msg(const char *prefix, const qemu_can_frame *msg)
{
- int i;
FILE *logfile = qemu_log_lock();
- qemu_log("%s%03X [%01d] %s %s",
- prefix,
- msg->can_id & QEMU_CAN_EFF_MASK,
- msg->can_dlc,
- msg->can_id & QEMU_CAN_EFF_FLAG ? "EFF" : "SFF",
- msg->can_id & QEMU_CAN_RTR_FLAG ? "RTR" : "DAT");
+ if (logfile) {
+ fprintf(logfile, "%s%03X [%01d] %s %s",
+ prefix,
+ msg->can_id & QEMU_CAN_EFF_MASK,
+ msg->can_dlc,
+ msg->can_id & QEMU_CAN_EFF_FLAG ? "EFF" : "SFF",
+ msg->can_id & QEMU_CAN_RTR_FLAG ? "RTR" : "DAT");
- for (i = 0; i < msg->can_dlc; i++) {
- qemu_log(" %02X", msg->data[i]);
+ for (int i = 0; i < msg->can_dlc; i++) {
+ fprintf(logfile, " %02X", msg->data[i]);
+ }
+ fprintf(logfile, "\n");
+ qemu_log_flush();
+ qemu_log_unlock(logfile);
}
- qemu_log("\n");
- qemu_log_flush();
- qemu_log_unlock(logfile);
}
static void buff2frame_pel(const uint8_t *buff, qemu_can_frame *frame)
diff --git a/net/can/can_socketcan.c b/net/can/can_socketcan.c
index 4b68f60c6b..ae19d35001 100644
--- a/net/can/can_socketcan.c
+++ b/net/can/can_socketcan.c
@@ -75,20 +75,22 @@ QEMU_BUILD_BUG_ON(offsetof(qemu_can_frame, data)
static void can_host_socketcan_display_msg(struct qemu_can_frame *msg)
{
- int i;
FILE *logfile = qemu_log_lock();
- qemu_log("[cansocketcan]: %03X [%01d] %s %s",
- msg->can_id & QEMU_CAN_EFF_MASK,
- msg->can_dlc,
- msg->can_id & QEMU_CAN_EFF_FLAG ? "EFF" : "SFF",
- msg->can_id & QEMU_CAN_RTR_FLAG ? "RTR" : "DAT");
- for (i = 0; i < msg->can_dlc; i++) {
- qemu_log(" %02X", msg->data[i]);
+ if (logfile) {
+ fprintf(logfile, "[cansocketcan]: %03X [%01d] %s %s",
+ msg->can_id & QEMU_CAN_EFF_MASK,
+ msg->can_dlc,
+ msg->can_id & QEMU_CAN_EFF_FLAG ? "EFF" : "SFF",
+ msg->can_id & QEMU_CAN_RTR_FLAG ? "RTR" : "DAT");
+
+ for (int i = 0; i < msg->can_dlc; i++) {
+ fprintf(logfile, " %02X", msg->data[i]);
+ }
+ fprintf(logfile, "\n");
+ qemu_log_flush();
+ qemu_log_unlock(logfile);
}
- qemu_log("\n");
- qemu_log_flush();
- qemu_log_unlock(logfile);
}
static void can_host_socketcan_read(void *opaque)
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 2a94d33742..7b75e79e2b 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -2581,14 +2581,16 @@ static void gen_unknown_opcode(CPUX86State *env, DisasContext *s)
if (qemu_loglevel_mask(LOG_UNIMP)) {
FILE *logfile = qemu_log_lock();
- target_ulong pc = s->pc_start, end = s->pc;
+ if (logfile) {
+ target_ulong pc = s->pc_start, end = s->pc;
- qemu_log("ILLOPC: " TARGET_FMT_lx ":", pc);
- for (; pc < end; ++pc) {
- qemu_log(" %02x", cpu_ldub_code(env, pc));
+ fprintf(logfile, "ILLOPC: " TARGET_FMT_lx ":", pc);
+ for (; pc < end; ++pc) {
+ fprintf(logfile, " %02x", cpu_ldub_code(env, pc));
+ }
+ fprintf(logfile, "\n");
+ qemu_log_unlock(logfile);
}
- qemu_log("\n");
- qemu_log_unlock(logfile);
}
}
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 33a97eabdb..42c1620ec6 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -757,31 +757,35 @@ void tcg_prologue_init(TCGContext *s)
#ifdef DEBUG_DISAS
if (qemu_loglevel_mask(CPU_LOG_TB_OUT_ASM)) {
FILE *logfile = qemu_log_lock();
- qemu_log("PROLOGUE: [size=%zu]\n", prologue_size);
- if (s->data_gen_ptr) {
- size_t code_size = s->data_gen_ptr - s->code_gen_ptr;
- size_t data_size = prologue_size - code_size;
- size_t i;
+ if (logfile) {
+ fprintf(logfile, "PROLOGUE: [size=%zu]\n", prologue_size);
+ if (s->data_gen_ptr) {
+ size_t code_size = s->data_gen_ptr - s->code_gen_ptr;
+ size_t data_size = prologue_size - code_size;
+ size_t i;
- log_disas(s->code_gen_ptr, code_size);
+ disas(logfile, s->code_gen_ptr, code_size);
- for (i = 0; i < data_size; i += sizeof(tcg_target_ulong)) {
- if (sizeof(tcg_target_ulong) == 8) {
- qemu_log("0x%08" PRIxPTR ": .quad 0x%016" PRIx64 "\n",
- (uintptr_t)s->data_gen_ptr + i,
- *(uint64_t *)(s->data_gen_ptr + i));
- } else {
- qemu_log("0x%08" PRIxPTR ": .long 0x%08x\n",
- (uintptr_t)s->data_gen_ptr + i,
- *(uint32_t *)(s->data_gen_ptr + i));
+ for (i = 0; i < data_size; i += sizeof(tcg_target_ulong)) {
+ if (sizeof(tcg_target_ulong) == 8) {
+ fprintf(logfile,
+ "0x%08" PRIxPTR ": .quad 0x%016" PRIx64 "\n",
+ (uintptr_t)s->data_gen_ptr + i,
+ *(uint64_t *)(s->data_gen_ptr + i));
+ } else {
+ fprintf(logfile,
+ "0x%08" PRIxPTR ": .long 0x%08x\n",
+ (uintptr_t)s->data_gen_ptr + i,
+ *(uint32_t *)(s->data_gen_ptr + i));
+ }
}
+ } else {
+ disas(logfile, s->code_gen_ptr, prologue_size);
}
- } else {
- log_disas(s->code_gen_ptr, prologue_size);
+ fprintf(logfile, "\n");
+ qemu_log_flush();
+ qemu_log_unlock(logfile);
}
- qemu_log("\n");
- qemu_log_flush();
- qemu_log_unlock(logfile);
}
#endif
@@ -4201,10 +4205,12 @@ int tcg_gen_code(TCGContext *s, TranslationBlock *tb)
if (unlikely(qemu_loglevel_mask(CPU_LOG_TB_OP)
&& qemu_log_in_addr_range(tb->pc))) {
FILE *logfile = qemu_log_lock();
- qemu_log("OP:\n");
- tcg_dump_ops(s, false);
- qemu_log("\n");
- qemu_log_unlock(logfile);
+ if (logfile) {
+ fprintf(logfile, "OP:\n");
+ tcg_dump_ops(s, false);
+ fprintf(logfile, "\n");
+ qemu_log_unlock(logfile);
+ }
}
#endif
@@ -4246,10 +4252,12 @@ int tcg_gen_code(TCGContext *s, TranslationBlock *tb)
if (unlikely(qemu_loglevel_mask(CPU_LOG_TB_OP_IND)
&& qemu_log_in_addr_range(tb->pc))) {
FILE *logfile = qemu_log_lock();
- qemu_log("OP before indirect lowering:\n");
- tcg_dump_ops(s, false);
- qemu_log("\n");
- qemu_log_unlock(logfile);
+ if (logfile) {
+ fprintf(logfile, "OP before indirect lowering:\n");
+ tcg_dump_ops(s, false);
+ fprintf(logfile, "\n");
+ qemu_log_unlock(logfile);
+ }
}
#endif
/* Replace indirect temps with direct temps. */
@@ -4267,10 +4275,12 @@ int tcg_gen_code(TCGContext *s, TranslationBlock *tb)
if (unlikely(qemu_loglevel_mask(CPU_LOG_TB_OP_OPT)
&& qemu_log_in_addr_range(tb->pc))) {
FILE *logfile = qemu_log_lock();
- qemu_log("OP after optimization and liveness analysis:\n");
- tcg_dump_ops(s, true);
- qemu_log("\n");
- qemu_log_unlock(logfile);
+ if (logfile) {
+ fprintf(logfile, "OP after optimization and liveness analysis:\n");
+ tcg_dump_ops(s, true);
+ fprintf(logfile, "\n");
+ qemu_log_unlock(logfile);
+ }
}
#endif
--
2.25.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* Re: [PULL for-7.1 08/36] *: Use fprintf between qemu_log_lock/unlock
2022-03-20 17:11 ` [PULL for-7.1 08/36] *: Use fprintf between qemu_log_lock/unlock Richard Henderson
@ 2022-03-23 17:22 ` Alex Bennée
2022-03-23 20:46 ` Richard Henderson
0 siblings, 1 reply; 60+ messages in thread
From: Alex Bennée @ 2022-03-23 17:22 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> Inside qemu_log, we perform qemu_log_lock/unlock, which need
> not be done if we have already performed the lock beforehand.
>
> Always check the result of qemu_log_lock -- only checking
> qemu_loglevel_mask races with the acquisition of the lock
> on the logfile.
I'm not sure I like introducing all these raw fprintfs over introducing
a function like qemu_log__locked().
--
Alex Bennée
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PULL for-7.1 08/36] *: Use fprintf between qemu_log_lock/unlock
2022-03-23 17:22 ` Alex Bennée
@ 2022-03-23 20:46 ` Richard Henderson
2022-03-24 14:30 ` Alex Bennée
0 siblings, 1 reply; 60+ messages in thread
From: Richard Henderson @ 2022-03-23 20:46 UTC (permalink / raw)
To: Alex Bennée; +Cc: qemu-devel
On 3/23/22 10:22, Alex Bennée wrote:
>
> Richard Henderson <richard.henderson@linaro.org> writes:
>
>> Inside qemu_log, we perform qemu_log_lock/unlock, which need
>> not be done if we have already performed the lock beforehand.
>>
>> Always check the result of qemu_log_lock -- only checking
>> qemu_loglevel_mask races with the acquisition of the lock
>> on the logfile.
>
> I'm not sure I like introducing all these raw fprintfs over introducing
> a function like qemu_log__locked().
There's no way to implement qemu_log__locked with rcu. The lookup itself is what needs
the locking; the return value of the FILE* is then valid until the unlock. To lookup the
FILE* again would require more atomic operations.
And I do prefer the printfs over repeated qemu_log.
r~
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PULL for-7.1 08/36] *: Use fprintf between qemu_log_lock/unlock
2022-03-23 20:46 ` Richard Henderson
@ 2022-03-24 14:30 ` Alex Bennée
2022-03-25 15:00 ` Richard Henderson
0 siblings, 1 reply; 60+ messages in thread
From: Alex Bennée @ 2022-03-24 14:30 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> On 3/23/22 10:22, Alex Bennée wrote:
>> Richard Henderson <richard.henderson@linaro.org> writes:
>>
>>> Inside qemu_log, we perform qemu_log_lock/unlock, which need
>>> not be done if we have already performed the lock beforehand.
>>>
>>> Always check the result of qemu_log_lock -- only checking
>>> qemu_loglevel_mask races with the acquisition of the lock
>>> on the logfile.
>> I'm not sure I like introducing all these raw fprintfs over
>> introducing
>> a function like qemu_log__locked().
>
> There's no way to implement qemu_log__locked with rcu. The lookup
> itself is what needs the locking; the return value of the FILE* is
> then valid until the unlock. To lookup the FILE* again would require
> more atomic operations.
That's not what I'm suggesting. qemu_log__locked would be a fairly
simple wrapper around the fprintf:
modified include/qemu/log.h
@@ -70,6 +70,25 @@ void qemu_log_unlock(FILE *fd);
} \
} while (0)
+/**
+ * qemu_log__locked() - log to a locked file
+ * @logfile: FILE handle from qemu_log_lock()
+ * @fmt: printf format
+ * ...: varargs
+ */
+static inline void G_GNUC_PRINTF(2, 3)
+ qemu_log__locked(FILE *logfile, const char *fmt, ...)
+{
+ if (logfile) {
+ va_list ap;
+
+ va_start(ap, fmt);
+ vfprintf(logfile, fmt, ap);
+ va_end(ap);
+ }
+}
+
+
/* Maintenance: */
/* define log items */
modified accel/tcg/translate-all.c
@@ -1546,10 +1546,10 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
}
/* Dump header and the first instruction */
- fprintf(logfile, "OUT: [size=%d]\n", gen_code_size);
- fprintf(logfile,
- " -- guest addr 0x" TARGET_FMT_lx " + tb prologue\n",
- tcg_ctx->gen_insn_data[insn][0]);
+ qemu_log__locked(logfile, "OUT: [size=%d]\n", gen_code_size);
+ qemu_log__locked(logfile,
+ " -- guest addr 0x" TARGET_FMT_lx " + tb prologue\n",
+ tcg_ctx->gen_insn_data[insn][0]);
chunk_start = tcg_ctx->gen_insn_end_off[insn];
disas(logfile, tb->tc.ptr, chunk_start);
@@ -1561,8 +1561,8 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
while (insn < tb->icount) {
size_t chunk_end = tcg_ctx->gen_insn_end_off[insn];
if (chunk_end > chunk_start) {
- fprintf(logfile, " -- guest addr 0x" TARGET_FMT_lx "\n",
- tcg_ctx->gen_insn_data[insn][0]);
+ qemu_log__locked(logfile, " -- guest addr 0x" TARGET_FMT_lx "\n",
+ tcg_ctx->gen_insn_data[insn][0]);
disas(logfile, tb->tc.ptr + chunk_start,
chunk_end - chunk_start);
I would home the inline would mean the compiler could do a semi-decent
job of eliding the multiple logfile checks. The _locked suffix is simply
to indicate it expects a pre-locked file.
> And I do prefer the printfs over repeated qemu_log.
The main benefit from my point of view is it keeps qemu_log operations
grouped together and easier to fix if we for example want to tweak how
we deal with log files in the future.
>
>
> r~
--
Alex Bennée
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PULL for-7.1 08/36] *: Use fprintf between qemu_log_lock/unlock
2022-03-24 14:30 ` Alex Bennée
@ 2022-03-25 15:00 ` Richard Henderson
0 siblings, 0 replies; 60+ messages in thread
From: Richard Henderson @ 2022-03-25 15:00 UTC (permalink / raw)
To: Alex Bennée; +Cc: qemu-devel
On 3/24/22 08:30, Alex Bennée wrote:
>
> Richard Henderson <richard.henderson@linaro.org> writes:
>
>> On 3/23/22 10:22, Alex Bennée wrote:
>>> Richard Henderson <richard.henderson@linaro.org> writes:
>>>
>>>> Inside qemu_log, we perform qemu_log_lock/unlock, which need
>>>> not be done if we have already performed the lock beforehand.
>>>>
>>>> Always check the result of qemu_log_lock -- only checking
>>>> qemu_loglevel_mask races with the acquisition of the lock
>>>> on the logfile.
>>> I'm not sure I like introducing all these raw fprintfs over
>>> introducing
>>> a function like qemu_log__locked().
>>
>> There's no way to implement qemu_log__locked with rcu. The lookup
>> itself is what needs the locking; the return value of the FILE* is
>> then valid until the unlock. To lookup the FILE* again would require
>> more atomic operations.
>
> That's not what I'm suggesting. qemu_log__locked would be a fairly
> simple wrapper around the fprintf:
>
> modified include/qemu/log.h
> @@ -70,6 +70,25 @@ void qemu_log_unlock(FILE *fd);
> } \
> } while (0)
>
> +/**
> + * qemu_log__locked() - log to a locked file
> + * @logfile: FILE handle from qemu_log_lock()
> + * @fmt: printf format
> + * ...: varargs
> + */
> +static inline void G_GNUC_PRINTF(2, 3)
> + qemu_log__locked(FILE *logfile, const char *fmt, ...)
> +{
> + if (logfile) {
> + va_list ap;
> +
> + va_start(ap, fmt);
> + vfprintf(logfile, fmt, ap);
> + va_end(ap);
> + }
> +}
If the lock succeeded, then we *know* that logfile != NULL -- indeed, that is exactly the
test that is required following qemu_log_lock(). There is no point in structuring the
code otherwise.
If you remove that, then you're left with
#define qemu_log__locked fprintf
Can you tell me that's really an improvement?
>> And I do prefer the printfs over repeated qemu_log.
>
> The main benefit from my point of view is it keeps qemu_log operations
> grouped together and easier to fix if we for example want to tweak how
> we deal with log files in the future.
I can't see that. If we have a radical adjustment to logfiles that requires something
other than fprintf, then we're probably going to change the type of logfile too. At which
point all of the other existing places that we pass the FILE*, e.g. cpu_dump_state are
also affected.
r~
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PULL for-7.1 09/36] util/log: Remove qemu_log_vprintf
2022-03-20 17:10 [PULL for-7.1 00/36] Logging cleanup and per-thread logfiles Richard Henderson
` (7 preceding siblings ...)
2022-03-20 17:11 ` [PULL for-7.1 08/36] *: Use fprintf between qemu_log_lock/unlock Richard Henderson
@ 2022-03-20 17:11 ` Richard Henderson
2022-03-23 13:32 ` Alex Bennée
2022-03-20 17:11 ` [PULL for-7.1 10/36] tcg: Pass the locked filepointer to tcg_dump_ops Richard Henderson
` (27 subsequent siblings)
36 siblings, 1 reply; 60+ messages in thread
From: Richard Henderson @ 2022-03-20 17:11 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
This function is no longer used.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/qemu/log.h | 15 ---------------
1 file changed, 15 deletions(-)
diff --git a/include/qemu/log.h b/include/qemu/log.h
index 10e284291c..6e45b31253 100644
--- a/include/qemu/log.h
+++ b/include/qemu/log.h
@@ -72,21 +72,6 @@ void qemu_log_unlock(FILE *fd);
/* Logging functions: */
-/* vfprintf-like logging function
- */
-static inline void GCC_FMT_ATTR(1, 0)
-qemu_log_vprintf(const char *fmt, va_list va)
-{
- QemuLogFile *logfile;
-
- rcu_read_lock();
- logfile = qatomic_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:
* @mask: bit to check in the mask
* @fmt: printf-style format string
--
2.25.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PULL for-7.1 10/36] tcg: Pass the locked filepointer to tcg_dump_ops
2022-03-20 17:10 [PULL for-7.1 00/36] Logging cleanup and per-thread logfiles Richard Henderson
` (8 preceding siblings ...)
2022-03-20 17:11 ` [PULL for-7.1 09/36] util/log: Remove qemu_log_vprintf Richard Henderson
@ 2022-03-20 17:11 ` Richard Henderson
2022-03-20 17:11 ` [PULL for-7.1 11/36] exec/translator: Pass the locked filepointer to disas_log hook Richard Henderson
` (26 subsequent siblings)
36 siblings, 0 replies; 60+ messages in thread
From: Richard Henderson @ 2022-03-20 17:11 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
We have already looked up and locked the filepointer.
Use fprintf instead of qemu_log directly for output
in and around tcg_dump_ops.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
tcg/tcg.c | 109 ++++++++++++++++++++++++++----------------------------
1 file changed, 52 insertions(+), 57 deletions(-)
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 42c1620ec6..1e27367af6 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -1808,7 +1808,11 @@ static inline TCGReg tcg_regset_first(TCGRegSet d)
}
}
-static void tcg_dump_ops(TCGContext *s, bool have_prefs)
+/* Return only the number of characters output -- no error return. */
+#define ne_fprintf(...) \
+ ({ int ret_ = fprintf(__VA_ARGS__); ret_ >= 0 ? ret_ : 0; })
+
+static void tcg_dump_ops(TCGContext *s, FILE *f, bool have_prefs)
{
char buf[128];
TCGOp *op;
@@ -1824,7 +1828,7 @@ static void tcg_dump_ops(TCGContext *s, bool have_prefs)
if (c == INDEX_op_insn_start) {
nb_oargs = 0;
- col += qemu_log("\n ----");
+ col += ne_fprintf(f, "\n ----");
for (i = 0; i < TARGET_INSN_START_WORDS; ++i) {
target_ulong a;
@@ -1833,7 +1837,7 @@ static void tcg_dump_ops(TCGContext *s, bool have_prefs)
#else
a = op->args[i];
#endif
- col += qemu_log(" " TARGET_FMT_lx, a);
+ col += ne_fprintf(f, " " TARGET_FMT_lx, a);
}
} else if (c == INDEX_op_call) {
const TCGHelperInfo *info = tcg_call_info(op);
@@ -1844,7 +1848,7 @@ static void tcg_dump_ops(TCGContext *s, bool have_prefs)
nb_iargs = TCGOP_CALLI(op);
nb_cargs = def->nb_cargs;
- col += qemu_log(" %s ", def->name);
+ col += ne_fprintf(f, " %s ", def->name);
/*
* Print the function name from TCGHelperInfo, if available.
@@ -1852,15 +1856,15 @@ static void tcg_dump_ops(TCGContext *s, bool have_prefs)
* but the actual function pointer comes from the plugin.
*/
if (func == info->func) {
- col += qemu_log("%s", info->name);
+ col += ne_fprintf(f, "%s", info->name);
} else {
- col += qemu_log("plugin(%p)", func);
+ col += ne_fprintf(f, "plugin(%p)", func);
}
- col += qemu_log(",$0x%x,$%d", info->flags, nb_oargs);
+ col += ne_fprintf(f, ",$0x%x,$%d", info->flags, nb_oargs);
for (i = 0; i < nb_oargs; i++) {
- col += qemu_log(",%s", tcg_get_arg_str(s, buf, sizeof(buf),
- op->args[i]));
+ col += ne_fprintf(f, ",%s", tcg_get_arg_str(s, buf, sizeof(buf),
+ op->args[i]));
}
for (i = 0; i < nb_iargs; i++) {
TCGArg arg = op->args[nb_oargs + i];
@@ -1868,34 +1872,32 @@ static void tcg_dump_ops(TCGContext *s, bool have_prefs)
if (arg != TCG_CALL_DUMMY_ARG) {
t = tcg_get_arg_str(s, buf, sizeof(buf), arg);
}
- col += qemu_log(",%s", t);
+ col += ne_fprintf(f, ",%s", t);
}
} else {
- col += qemu_log(" %s ", def->name);
+ col += ne_fprintf(f, " %s ", def->name);
nb_oargs = def->nb_oargs;
nb_iargs = def->nb_iargs;
nb_cargs = def->nb_cargs;
if (def->flags & TCG_OPF_VECTOR) {
- col += qemu_log("v%d,e%d,", 64 << TCGOP_VECL(op),
- 8 << TCGOP_VECE(op));
+ col += ne_fprintf(f, "v%d,e%d,", 64 << TCGOP_VECL(op),
+ 8 << TCGOP_VECE(op));
}
k = 0;
for (i = 0; i < nb_oargs; i++) {
- if (k != 0) {
- col += qemu_log(",");
- }
- col += qemu_log("%s", tcg_get_arg_str(s, buf, sizeof(buf),
- op->args[k++]));
+ const char *sep = k ? "," : "";
+ col += ne_fprintf(f, "%s%s", sep,
+ tcg_get_arg_str(s, buf, sizeof(buf),
+ op->args[k++]));
}
for (i = 0; i < nb_iargs; i++) {
- if (k != 0) {
- col += qemu_log(",");
- }
- col += qemu_log("%s", tcg_get_arg_str(s, buf, sizeof(buf),
- op->args[k++]));
+ const char *sep = k ? "," : "";
+ col += ne_fprintf(f, "%s%s", sep,
+ tcg_get_arg_str(s, buf, sizeof(buf),
+ op->args[k++]));
}
switch (c) {
case INDEX_op_brcond_i32:
@@ -1910,9 +1912,9 @@ static void tcg_dump_ops(TCGContext *s, bool have_prefs)
case INDEX_op_cmpsel_vec:
if (op->args[k] < ARRAY_SIZE(cond_name)
&& cond_name[op->args[k]]) {
- col += qemu_log(",%s", cond_name[op->args[k++]]);
+ col += ne_fprintf(f, ",%s", cond_name[op->args[k++]]);
} else {
- col += qemu_log(",$0x%" TCG_PRIlx, op->args[k++]);
+ col += ne_fprintf(f, ",$0x%" TCG_PRIlx, op->args[k++]);
}
i = 1;
break;
@@ -1927,12 +1929,12 @@ static void tcg_dump_ops(TCGContext *s, bool have_prefs)
unsigned ix = get_mmuidx(oi);
if (op & ~(MO_AMASK | MO_BSWAP | MO_SSIZE)) {
- col += qemu_log(",$0x%x,%u", op, ix);
+ col += ne_fprintf(f, ",$0x%x,%u", op, ix);
} else {
const char *s_al, *s_op;
s_al = alignment_name[(op & MO_AMASK) >> MO_ASHIFT];
s_op = ldst_name[op & (MO_BSWAP | MO_SSIZE)];
- col += qemu_log(",%s%s,%u", s_al, s_op, ix);
+ col += ne_fprintf(f, ",%s%s,%u", s_al, s_op, ix);
}
i = 1;
}
@@ -1950,9 +1952,9 @@ static void tcg_dump_ops(TCGContext *s, bool have_prefs)
name = bswap_flag_name[flags];
}
if (name) {
- col += qemu_log(",%s", name);
+ col += ne_fprintf(f, ",%s", name);
} else {
- col += qemu_log(",$0x%" TCG_PRIlx, flags);
+ col += ne_fprintf(f, ",$0x%" TCG_PRIlx, flags);
}
i = k = 1;
}
@@ -1967,49 +1969,42 @@ static void tcg_dump_ops(TCGContext *s, bool have_prefs)
case INDEX_op_brcond_i32:
case INDEX_op_brcond_i64:
case INDEX_op_brcond2_i32:
- col += qemu_log("%s$L%d", k ? "," : "",
- arg_label(op->args[k])->id);
+ col += ne_fprintf(f, "%s$L%d", k ? "," : "",
+ arg_label(op->args[k])->id);
i++, k++;
break;
default:
break;
}
for (; i < nb_cargs; i++, k++) {
- col += qemu_log("%s$0x%" TCG_PRIlx, k ? "," : "", op->args[k]);
+ col += ne_fprintf(f, "%s$0x%" TCG_PRIlx, k ? "," : "",
+ op->args[k]);
}
}
if (have_prefs || op->life) {
-
- QemuLogFile *logfile;
-
- rcu_read_lock();
- logfile = qatomic_rcu_read(&qemu_logfile);
- if (logfile) {
- for (; col < 40; ++col) {
- putc(' ', logfile->fd);
- }
+ for (; col < 40; ++col) {
+ putc(' ', f);
}
- rcu_read_unlock();
}
if (op->life) {
unsigned life = op->life;
if (life & (SYNC_ARG * 3)) {
- qemu_log(" sync:");
+ ne_fprintf(f, " sync:");
for (i = 0; i < 2; ++i) {
if (life & (SYNC_ARG << i)) {
- qemu_log(" %d", i);
+ ne_fprintf(f, " %d", i);
}
}
}
life /= DEAD_ARG;
if (life) {
- qemu_log(" dead:");
+ ne_fprintf(f, " dead:");
for (i = 0; life; ++i, life >>= 1) {
if (life & 1) {
- qemu_log(" %d", i);
+ ne_fprintf(f, " %d", i);
}
}
}
@@ -2020,28 +2015,28 @@ static void tcg_dump_ops(TCGContext *s, bool have_prefs)
TCGRegSet set = op->output_pref[i];
if (i == 0) {
- qemu_log(" pref=");
+ ne_fprintf(f, " pref=");
} else {
- qemu_log(",");
+ ne_fprintf(f, ",");
}
if (set == 0) {
- qemu_log("none");
+ ne_fprintf(f, "none");
} else if (set == MAKE_64BIT_MASK(0, TCG_TARGET_NB_REGS)) {
- qemu_log("all");
+ ne_fprintf(f, "all");
#ifdef CONFIG_DEBUG_TCG
} else if (tcg_regset_single(set)) {
TCGReg reg = tcg_regset_first(set);
- qemu_log("%s", tcg_target_reg_names[reg]);
+ ne_fprintf(f, "%s", tcg_target_reg_names[reg]);
#endif
} else if (TCG_TARGET_NB_REGS <= 32) {
- qemu_log("%#x", (uint32_t)set);
+ ne_fprintf(f, "%#x", (uint32_t)set);
} else {
- qemu_log("%#" PRIx64, (uint64_t)set);
+ ne_fprintf(f, "%#" PRIx64, (uint64_t)set);
}
}
}
- qemu_log("\n");
+ putc('\n', f);
}
}
@@ -4207,7 +4202,7 @@ int tcg_gen_code(TCGContext *s, TranslationBlock *tb)
FILE *logfile = qemu_log_lock();
if (logfile) {
fprintf(logfile, "OP:\n");
- tcg_dump_ops(s, false);
+ tcg_dump_ops(s, logfile, false);
fprintf(logfile, "\n");
qemu_log_unlock(logfile);
}
@@ -4254,7 +4249,7 @@ int tcg_gen_code(TCGContext *s, TranslationBlock *tb)
FILE *logfile = qemu_log_lock();
if (logfile) {
fprintf(logfile, "OP before indirect lowering:\n");
- tcg_dump_ops(s, false);
+ tcg_dump_ops(s, logfile, false);
fprintf(logfile, "\n");
qemu_log_unlock(logfile);
}
@@ -4277,7 +4272,7 @@ int tcg_gen_code(TCGContext *s, TranslationBlock *tb)
FILE *logfile = qemu_log_lock();
if (logfile) {
fprintf(logfile, "OP after optimization and liveness analysis:\n");
- tcg_dump_ops(s, true);
+ tcg_dump_ops(s, logfile, true);
fprintf(logfile, "\n");
qemu_log_unlock(logfile);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PULL for-7.1 11/36] exec/translator: Pass the locked filepointer to disas_log hook
2022-03-20 17:10 [PULL for-7.1 00/36] Logging cleanup and per-thread logfiles Richard Henderson
` (9 preceding siblings ...)
2022-03-20 17:11 ` [PULL for-7.1 10/36] tcg: Pass the locked filepointer to tcg_dump_ops Richard Henderson
@ 2022-03-20 17:11 ` Richard Henderson
2022-03-20 17:11 ` [PULL for-7.1 12/36] exec/log: Remove log_disas and log_target_disas Richard Henderson
` (25 subsequent siblings)
36 siblings, 0 replies; 60+ messages in thread
From: Richard Henderson @ 2022-03-20 17:11 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
We have fetched and locked the logfile in translator_loop.
Pass the filepointer down to the disas_log hook so that it
need not be fetched and locked again.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/exec/translator.h | 2 +-
accel/tcg/translator.c | 2 +-
target/alpha/translate.c | 7 ++++---
target/arm/translate-a64.c | 6 +++---
target/arm/translate.c | 7 ++++---
target/avr/translate.c | 7 ++++---
target/cris/translate.c | 7 ++++---
target/hexagon/translate.c | 7 ++++---
target/hppa/translate.c | 15 ++++++++-------
target/i386/tcg/translate.c | 6 +++---
target/m68k/translate.c | 7 ++++---
target/microblaze/translate.c | 7 ++++---
target/mips/tcg/translate.c | 7 ++++---
target/nios2/translate.c | 7 ++++---
target/openrisc/translate.c | 7 ++++---
target/ppc/translate.c | 7 ++++---
target/riscv/translate.c | 10 ++++++----
target/rx/translate.c | 7 ++++---
target/s390x/tcg/translate.c | 11 ++++++-----
target/sh4/translate.c | 7 ++++---
target/sparc/translate.c | 7 ++++---
target/tricore/translate.c | 7 ++++---
target/xtensa/translate.c | 7 ++++---
23 files changed, 92 insertions(+), 72 deletions(-)
diff --git a/include/exec/translator.h b/include/exec/translator.h
index 9bc46eda59..31d3fa76ff 100644
--- a/include/exec/translator.h
+++ b/include/exec/translator.h
@@ -118,7 +118,7 @@ typedef struct TranslatorOps {
void (*insn_start)(DisasContextBase *db, CPUState *cpu);
void (*translate_insn)(DisasContextBase *db, CPUState *cpu);
void (*tb_stop)(DisasContextBase *db, CPUState *cpu);
- void (*disas_log)(const DisasContextBase *db, CPUState *cpu);
+ void (*disas_log)(const DisasContextBase *db, CPUState *cpu, FILE *f);
} TranslatorOps;
/**
diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index 90da944dab..7d1505b239 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -142,7 +142,7 @@ void translator_loop(const TranslatorOps *ops, DisasContextBase *db,
FILE *logfile = qemu_log_lock();
if (logfile) {
fprintf(logfile, "----------------\n");
- ops->disas_log(db, cpu);
+ ops->disas_log(db, cpu, logfile);
fprintf(logfile, "\n");
qemu_log_unlock(logfile);
}
diff --git a/target/alpha/translate.c b/target/alpha/translate.c
index 66768ab47a..b1f78cff53 100644
--- a/target/alpha/translate.c
+++ b/target/alpha/translate.c
@@ -3027,10 +3027,11 @@ static void alpha_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
}
}
-static void alpha_tr_disas_log(const DisasContextBase *dcbase, CPUState *cpu)
+static void alpha_tr_disas_log(const DisasContextBase *dcbase,
+ CPUState *cpu, FILE *logfile)
{
- qemu_log("IN: %s\n", lookup_symbol(dcbase->pc_first));
- log_target_disas(cpu, dcbase->pc_first, dcbase->tb->size);
+ fprintf(logfile, "IN: %s\n", lookup_symbol(dcbase->pc_first));
+ target_disas(logfile, cpu, dcbase->pc_first, dcbase->tb->size);
}
static const TranslatorOps alpha_tr_ops = {
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index d1a59fad9c..87ae8b7571 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -14962,12 +14962,12 @@ static void aarch64_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
}
static void aarch64_tr_disas_log(const DisasContextBase *dcbase,
- CPUState *cpu)
+ CPUState *cpu, FILE *logfile)
{
DisasContext *dc = container_of(dcbase, DisasContext, base);
- qemu_log("IN: %s\n", lookup_symbol(dc->base.pc_first));
- log_target_disas(cpu, dc->base.pc_first, dc->base.tb->size);
+ fprintf(logfile, "IN: %s\n", lookup_symbol(dc->base.pc_first));
+ target_disas(logfile, cpu, dc->base.pc_first, dc->base.tb->size);
}
const TranslatorOps aarch64_translator_ops = {
diff --git a/target/arm/translate.c b/target/arm/translate.c
index bf2196b9e2..783a43b4ae 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -9891,12 +9891,13 @@ static void arm_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
}
}
-static void arm_tr_disas_log(const DisasContextBase *dcbase, CPUState *cpu)
+static void arm_tr_disas_log(const DisasContextBase *dcbase,
+ CPUState *cpu, FILE *logfile)
{
DisasContext *dc = container_of(dcbase, DisasContext, base);
- qemu_log("IN: %s\n", lookup_symbol(dc->base.pc_first));
- log_target_disas(cpu, dc->base.pc_first, dc->base.tb->size);
+ fprintf(logfile, "IN: %s\n", lookup_symbol(dc->base.pc_first));
+ target_disas(logfile, cpu, dc->base.pc_first, dc->base.tb->size);
}
static const TranslatorOps arm_translator_ops = {
diff --git a/target/avr/translate.c b/target/avr/translate.c
index af8a3e0f9c..dc9c3d6bcc 100644
--- a/target/avr/translate.c
+++ b/target/avr/translate.c
@@ -3015,10 +3015,11 @@ static void avr_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
}
}
-static void avr_tr_disas_log(const DisasContextBase *dcbase, CPUState *cs)
+static void avr_tr_disas_log(const DisasContextBase *dcbase,
+ CPUState *cs, FILE *logfile)
{
- qemu_log("IN: %s\n", lookup_symbol(dcbase->pc_first));
- log_target_disas(cs, dcbase->pc_first, dcbase->tb->size);
+ fprintf(logfile, "IN: %s\n", lookup_symbol(dcbase->pc_first));
+ target_disas(logfile, cs, dcbase->pc_first, dcbase->tb->size);
}
static const TranslatorOps avr_tr_ops = {
diff --git a/target/cris/translate.c b/target/cris/translate.c
index 3656cd6db1..ac101344a3 100644
--- a/target/cris/translate.c
+++ b/target/cris/translate.c
@@ -3268,11 +3268,12 @@ static void cris_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
}
}
-static void cris_tr_disas_log(const DisasContextBase *dcbase, CPUState *cpu)
+static void cris_tr_disas_log(const DisasContextBase *dcbase,
+ CPUState *cpu, FILE *logfile)
{
if (!DISAS_CRIS) {
- qemu_log("IN: %s\n", lookup_symbol(dcbase->pc_first));
- log_target_disas(cpu, dcbase->pc_first, dcbase->tb->size);
+ fprintf(logfile, "IN: %s\n", lookup_symbol(dcbase->pc_first));
+ target_disas(logfile, cpu, dcbase->pc_first, dcbase->tb->size);
}
}
diff --git a/target/hexagon/translate.c b/target/hexagon/translate.c
index 98f1452cad..d4fc92f7e9 100644
--- a/target/hexagon/translate.c
+++ b/target/hexagon/translate.c
@@ -833,10 +833,11 @@ static void hexagon_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
}
}
-static void hexagon_tr_disas_log(const DisasContextBase *dcbase, CPUState *cpu)
+static void hexagon_tr_disas_log(const DisasContextBase *dcbase,
+ CPUState *cpu, FILE *logfile)
{
- qemu_log("IN: %s\n", lookup_symbol(dcbase->pc_first));
- log_target_disas(cpu, dcbase->pc_first, dcbase->tb->size);
+ fprintf(logfile, "IN: %s\n", lookup_symbol(dcbase->pc_first));
+ target_disas(logfile, cpu, dcbase->pc_first, dcbase->tb->size);
}
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 5c0b1eb274..c24d3ce3b8 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -4305,29 +4305,30 @@ static void hppa_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
}
}
-static void hppa_tr_disas_log(const DisasContextBase *dcbase, CPUState *cs)
+static void hppa_tr_disas_log(const DisasContextBase *dcbase,
+ CPUState *cs, FILE *logfile)
{
target_ulong pc = dcbase->pc_first;
#ifdef CONFIG_USER_ONLY
switch (pc) {
case 0x00:
- qemu_log("IN:\n0x00000000: (null)\n");
+ fprintf(logfile, "IN:\n0x00000000: (null)\n");
return;
case 0xb0:
- qemu_log("IN:\n0x000000b0: light-weight-syscall\n");
+ fprintf(logfile, "IN:\n0x000000b0: light-weight-syscall\n");
return;
case 0xe0:
- qemu_log("IN:\n0x000000e0: set-thread-pointer-syscall\n");
+ fprintf(logfile, "IN:\n0x000000e0: set-thread-pointer-syscall\n");
return;
case 0x100:
- qemu_log("IN:\n0x00000100: syscall\n");
+ fprintf(logfile, "IN:\n0x00000100: syscall\n");
return;
}
#endif
- qemu_log("IN: %s\n", lookup_symbol(pc));
- log_target_disas(cs, pc, dcbase->tb->size);
+ fprintf(logfile, "IN: %s\n", lookup_symbol(pc));
+ target_disas(logfile, cs, pc, dcbase->tb->size);
}
static const TranslatorOps hppa_tr_ops = {
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 7b75e79e2b..f905a34bd7 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -8692,12 +8692,12 @@ static void i386_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
}
static void i386_tr_disas_log(const DisasContextBase *dcbase,
- CPUState *cpu)
+ CPUState *cpu, FILE *logfile)
{
DisasContext *dc = container_of(dcbase, DisasContext, base);
- qemu_log("IN: %s\n", lookup_symbol(dc->base.pc_first));
- log_target_disas(cpu, dc->base.pc_first, dc->base.tb->size);
+ fprintf(logfile, "IN: %s\n", lookup_symbol(dc->base.pc_first));
+ target_disas(logfile, cpu, dc->base.pc_first, dc->base.tb->size);
}
static const TranslatorOps i386_tr_ops = {
diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index af43c8eab8..4026572ed8 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -6257,10 +6257,11 @@ static void m68k_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
}
}
-static void m68k_tr_disas_log(const DisasContextBase *dcbase, CPUState *cpu)
+static void m68k_tr_disas_log(const DisasContextBase *dcbase,
+ CPUState *cpu, FILE *logfile)
{
- qemu_log("IN: %s\n", lookup_symbol(dcbase->pc_first));
- log_target_disas(cpu, dcbase->pc_first, dcbase->tb->size);
+ fprintf(logfile, "IN: %s\n", lookup_symbol(dcbase->pc_first));
+ target_disas(logfile, cpu, dcbase->pc_first, dcbase->tb->size);
}
static const TranslatorOps m68k_tr_ops = {
diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index 2561b904b9..bf01384d33 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -1833,10 +1833,11 @@ static void mb_tr_tb_stop(DisasContextBase *dcb, CPUState *cs)
}
}
-static void mb_tr_disas_log(const DisasContextBase *dcb, CPUState *cs)
+static void mb_tr_disas_log(const DisasContextBase *dcb,
+ CPUState *cs, FILE *logfile)
{
- qemu_log("IN: %s\n", lookup_symbol(dcb->pc_first));
- log_target_disas(cs, dcb->pc_first, dcb->tb->size);
+ fprintf(logfile, "IN: %s\n", lookup_symbol(dcb->pc_first));
+ target_disas(logfile, cs, dcb->pc_first, dcb->tb->size);
}
static const TranslatorOps mb_tr_ops = {
diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
index b82a7ec6ad..6de5b66650 100644
--- a/target/mips/tcg/translate.c
+++ b/target/mips/tcg/translate.c
@@ -16141,10 +16141,11 @@ static void mips_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
}
}
-static void mips_tr_disas_log(const DisasContextBase *dcbase, CPUState *cs)
+static void mips_tr_disas_log(const DisasContextBase *dcbase,
+ CPUState *cs, FILE *logfile)
{
- qemu_log("IN: %s\n", lookup_symbol(dcbase->pc_first));
- log_target_disas(cs, dcbase->pc_first, dcbase->tb->size);
+ fprintf(logfile, "IN: %s\n", lookup_symbol(dcbase->pc_first));
+ target_disas(logfile, cs, dcbase->pc_first, dcbase->tb->size);
}
static const TranslatorOps mips_tr_ops = {
diff --git a/target/nios2/translate.c b/target/nios2/translate.c
index f89271dbed..89b97ef520 100644
--- a/target/nios2/translate.c
+++ b/target/nios2/translate.c
@@ -833,10 +833,11 @@ static void nios2_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
}
}
-static void nios2_tr_disas_log(const DisasContextBase *dcbase, CPUState *cpu)
+static void nios2_tr_disas_log(const DisasContextBase *dcbase,
+ CPUState *cpu, FILE *logfile)
{
- qemu_log("IN: %s\n", lookup_symbol(dcbase->pc_first));
- log_target_disas(cpu, dcbase->pc_first, dcbase->tb->size);
+ fprintf(logfile, "IN: %s\n", lookup_symbol(dcbase->pc_first));
+ target_disas(logfile, cpu, dcbase->pc_first, dcbase->tb->size);
}
static const TranslatorOps nios2_tr_ops = {
diff --git a/target/openrisc/translate.c b/target/openrisc/translate.c
index ca79e609da..7b8ad43d5f 100644
--- a/target/openrisc/translate.c
+++ b/target/openrisc/translate.c
@@ -1687,12 +1687,13 @@ static void openrisc_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
}
}
-static void openrisc_tr_disas_log(const DisasContextBase *dcbase, CPUState *cs)
+static void openrisc_tr_disas_log(const DisasContextBase *dcbase,
+ CPUState *cs, FILE *logfile)
{
DisasContext *s = container_of(dcbase, DisasContext, base);
- qemu_log("IN: %s\n", lookup_symbol(s->base.pc_first));
- log_target_disas(cs, s->base.pc_first, s->base.tb->size);
+ fprintf(logfile, "IN: %s\n", lookup_symbol(s->base.pc_first));
+ target_disas(logfile, cs, s->base.pc_first, s->base.tb->size);
}
static const TranslatorOps openrisc_tr_ops = {
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 408ae26173..e0c9f74da4 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -7769,10 +7769,11 @@ static void ppc_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
}
}
-static void ppc_tr_disas_log(const DisasContextBase *dcbase, CPUState *cs)
+static void ppc_tr_disas_log(const DisasContextBase *dcbase,
+ CPUState *cs, FILE *logfile)
{
- qemu_log("IN: %s\n", lookup_symbol(dcbase->pc_first));
- log_target_disas(cs, dcbase->pc_first, dcbase->tb->size);
+ fprintf(logfile, "IN: %s\n", lookup_symbol(dcbase->pc_first));
+ target_disas(logfile, cs, dcbase->pc_first, dcbase->tb->size);
}
static const TranslatorOps ppc_tr_ops = {
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index fac998a6b5..6495726302 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -1155,18 +1155,20 @@ static void riscv_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
}
}
-static void riscv_tr_disas_log(const DisasContextBase *dcbase, CPUState *cpu)
+static void riscv_tr_disas_log(const DisasContextBase *dcbase,
+ CPUState *cpu, FILE *logfile)
{
#ifndef CONFIG_USER_ONLY
RISCVCPU *rvcpu = RISCV_CPU(cpu);
CPURISCVState *env = &rvcpu->env;
#endif
- qemu_log("IN: %s\n", lookup_symbol(dcbase->pc_first));
+ fprintf(logfile, "IN: %s\n", lookup_symbol(dcbase->pc_first));
#ifndef CONFIG_USER_ONLY
- qemu_log("Priv: "TARGET_FMT_ld"; Virt: "TARGET_FMT_ld"\n", env->priv, env->virt);
+ fprintf(logfile, "Priv: "TARGET_FMT_ld"; Virt: "TARGET_FMT_ld"\n",
+ env->priv, env->virt);
#endif
- log_target_disas(cpu, dcbase->pc_first, dcbase->tb->size);
+ target_disas(logfile, cpu, dcbase->pc_first, dcbase->tb->size);
}
static const TranslatorOps riscv_tr_ops = {
diff --git a/target/rx/translate.c b/target/rx/translate.c
index 5db8f79a82..c8a8991a63 100644
--- a/target/rx/translate.c
+++ b/target/rx/translate.c
@@ -2342,10 +2342,11 @@ static void rx_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
}
}
-static void rx_tr_disas_log(const DisasContextBase *dcbase, CPUState *cs)
+static void rx_tr_disas_log(const DisasContextBase *dcbase,
+ CPUState *cs, FILE *logfile)
{
- qemu_log("IN:\n"); /* , lookup_symbol(dcbase->pc_first)); */
- log_target_disas(cs, dcbase->pc_first, dcbase->tb->size);
+ fprintf(logfile, "IN: %s\n", lookup_symbol(dcbase->pc_first));
+ target_disas(logfile, cs, dcbase->pc_first, dcbase->tb->size);
}
static const TranslatorOps rx_tr_ops = {
diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index 5acfc0ff9b..1e7f470c4b 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -6660,16 +6660,17 @@ static void s390x_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
}
}
-static void s390x_tr_disas_log(const DisasContextBase *dcbase, CPUState *cs)
+static void s390x_tr_disas_log(const DisasContextBase *dcbase,
+ CPUState *cs, FILE *logfile)
{
DisasContext *dc = container_of(dcbase, DisasContext, base);
if (unlikely(dc->ex_value)) {
- /* ??? Unfortunately log_target_disas can't use host memory. */
- qemu_log("IN: EXECUTE %016" PRIx64, dc->ex_value);
+ /* ??? Unfortunately target_disas can't use host memory. */
+ fprintf(logfile, "IN: EXECUTE %016" PRIx64, dc->ex_value);
} else {
- qemu_log("IN: %s\n", lookup_symbol(dc->base.pc_first));
- log_target_disas(cs, dc->base.pc_first, dc->base.tb->size);
+ fprintf(logfile, "IN: %s\n", lookup_symbol(dc->base.pc_first));
+ target_disas(logfile, cs, dc->base.pc_first, dc->base.tb->size);
}
}
diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index 43bc88b7b3..f1b190e7cf 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -2352,10 +2352,11 @@ static void sh4_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
}
}
-static void sh4_tr_disas_log(const DisasContextBase *dcbase, CPUState *cs)
+static void sh4_tr_disas_log(const DisasContextBase *dcbase,
+ CPUState *cs, FILE *logfile)
{
- qemu_log("IN: %s\n", lookup_symbol(dcbase->pc_first));
- log_target_disas(cs, dcbase->pc_first, dcbase->tb->size);
+ fprintf(logfile, "IN: %s\n", lookup_symbol(dcbase->pc_first));
+ target_disas(logfile, cs, dcbase->pc_first, dcbase->tb->size);
}
static const TranslatorOps sh4_tr_ops = {
diff --git a/target/sparc/translate.c b/target/sparc/translate.c
index 4c7c7b5347..2e28222d31 100644
--- a/target/sparc/translate.c
+++ b/target/sparc/translate.c
@@ -5901,10 +5901,11 @@ static void sparc_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
}
}
-static void sparc_tr_disas_log(const DisasContextBase *dcbase, CPUState *cpu)
+static void sparc_tr_disas_log(const DisasContextBase *dcbase,
+ CPUState *cpu, FILE *logfile)
{
- qemu_log("IN: %s\n", lookup_symbol(dcbase->pc_first));
- log_target_disas(cpu, dcbase->pc_first, dcbase->tb->size);
+ fprintf(logfile, "IN: %s\n", lookup_symbol(dcbase->pc_first));
+ target_disas(logfile, cpu, dcbase->pc_first, dcbase->tb->size);
}
static const TranslatorOps sparc_tr_ops = {
diff --git a/target/tricore/translate.c b/target/tricore/translate.c
index 417edbd3f0..d170500fa5 100644
--- a/target/tricore/translate.c
+++ b/target/tricore/translate.c
@@ -8861,10 +8861,11 @@ static void tricore_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
}
}
-static void tricore_tr_disas_log(const DisasContextBase *dcbase, CPUState *cpu)
+static void tricore_tr_disas_log(const DisasContextBase *dcbase,
+ CPUState *cpu, FILE *logfile)
{
- qemu_log("IN: %s\n", lookup_symbol(dcbase->pc_first));
- log_target_disas(cpu, dcbase->pc_first, dcbase->tb->size);
+ fprintf(logfile, "IN: %s\n", lookup_symbol(dcbase->pc_first));
+ target_disas(logfile, cpu, dcbase->pc_first, dcbase->tb->size);
}
static const TranslatorOps tricore_tr_ops = {
diff --git a/target/xtensa/translate.c b/target/xtensa/translate.c
index b1491ed625..a9e69c4153 100644
--- a/target/xtensa/translate.c
+++ b/target/xtensa/translate.c
@@ -1296,10 +1296,11 @@ static void xtensa_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
}
}
-static void xtensa_tr_disas_log(const DisasContextBase *dcbase, CPUState *cpu)
+static void xtensa_tr_disas_log(const DisasContextBase *dcbase,
+ CPUState *cpu, FILE *logfile)
{
- qemu_log("IN: %s\n", lookup_symbol(dcbase->pc_first));
- log_target_disas(cpu, dcbase->pc_first, dcbase->tb->size);
+ fprintf(logfile, "IN: %s\n", lookup_symbol(dcbase->pc_first));
+ target_disas(logfile, cpu, dcbase->pc_first, dcbase->tb->size);
}
static const TranslatorOps xtensa_translator_ops = {
--
2.25.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PULL for-7.1 12/36] exec/log: Remove log_disas and log_target_disas
2022-03-20 17:10 [PULL for-7.1 00/36] Logging cleanup and per-thread logfiles Richard Henderson
` (10 preceding siblings ...)
2022-03-20 17:11 ` [PULL for-7.1 11/36] exec/translator: Pass the locked filepointer to disas_log hook Richard Henderson
@ 2022-03-20 17:11 ` Richard Henderson
2022-03-20 17:11 ` [PULL for-7.1 13/36] accel/tcg: Use cpu_dump_state between qemu_log_lock/unlock Richard Henderson
` (24 subsequent siblings)
36 siblings, 0 replies; 60+ messages in thread
From: Richard Henderson @ 2022-03-20 17:11 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
These functions are no longer used.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/exec/log.h | 24 ------------------------
1 file changed, 24 deletions(-)
diff --git a/include/exec/log.h b/include/exec/log.h
index 6a53199d44..d131a3140d 100644
--- a/include/exec/log.h
+++ b/include/exec/log.h
@@ -43,30 +43,6 @@ static inline void log_cpu_state_mask(int mask, CPUState *cpu, int flags)
}
#ifdef NEED_CPU_H
-/* disas() and target_disas() to qemu_logfile: */
-static inline void log_target_disas(CPUState *cpu, target_ulong start,
- target_ulong len)
-{
- QemuLogFile *logfile;
- rcu_read_lock();
- logfile = qatomic_rcu_read(&qemu_logfile);
- if (logfile) {
- target_disas(logfile->fd, cpu, start, len);
- }
- rcu_read_unlock();
-}
-
-static inline void log_disas(const void *code, unsigned long size)
-{
- QemuLogFile *logfile;
- rcu_read_lock();
- logfile = qatomic_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(const char *operation)
--
2.25.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PULL for-7.1 13/36] accel/tcg: Use cpu_dump_state between qemu_log_lock/unlock
2022-03-20 17:10 [PULL for-7.1 00/36] Logging cleanup and per-thread logfiles Richard Henderson
` (11 preceding siblings ...)
2022-03-20 17:11 ` [PULL for-7.1 12/36] exec/log: Remove log_disas and log_target_disas Richard Henderson
@ 2022-03-20 17:11 ` Richard Henderson
2022-03-20 17:11 ` [PULL for-7.1 14/36] target/nios2: Remove log_cpu_state from reset Richard Henderson
` (23 subsequent siblings)
36 siblings, 0 replies; 60+ messages in thread
From: Richard Henderson @ 2022-03-20 17:11 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
Inside log_cpu_state, we perform qemu_log_lock/unlock, which need
not be done if we have already performed the lock beforehand.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
accel/tcg/cpu-exec.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 7bc489101f..8b7d6ae9b9 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -225,7 +225,7 @@ static inline void log_cpu_exec(target_ulong pc, CPUState *cpu,
#if defined(TARGET_I386)
flags |= CPU_DUMP_CCOP;
#endif
- log_cpu_state(cpu, flags);
+ cpu_dump_state(cpu, logfile, flags);
qemu_log_unlock(logfile);
}
}
--
2.25.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PULL for-7.1 14/36] target/nios2: Remove log_cpu_state from reset
2022-03-20 17:10 [PULL for-7.1 00/36] Logging cleanup and per-thread logfiles Richard Henderson
` (12 preceding siblings ...)
2022-03-20 17:11 ` [PULL for-7.1 13/36] accel/tcg: Use cpu_dump_state between qemu_log_lock/unlock Richard Henderson
@ 2022-03-20 17:11 ` Richard Henderson
2022-03-20 21:54 ` Philippe Mathieu-Daudé
2022-03-20 17:11 ` [PULL for-7.1 15/36] util/log: Use qemu_log_lock/unlock in qemu_log Richard Henderson
` (22 subsequent siblings)
36 siblings, 1 reply; 60+ messages in thread
From: Richard Henderson @ 2022-03-20 17:11 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
This is redundant with the logging done in cpu_common_reset.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/nios2/cpu.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/target/nios2/cpu.c b/target/nios2/cpu.c
index 6975ae4bdb..b0877cb39e 100644
--- a/target/nios2/cpu.c
+++ b/target/nios2/cpu.c
@@ -46,11 +46,6 @@ static void nios2_cpu_reset(DeviceState *dev)
Nios2CPUClass *ncc = NIOS2_CPU_GET_CLASS(cpu);
CPUNios2State *env = &cpu->env;
- if (qemu_loglevel_mask(CPU_LOG_RESET)) {
- qemu_log("CPU Reset (CPU %d)\n", cs->cpu_index);
- log_cpu_state(cs, 0);
- }
-
ncc->parent_reset(dev);
memset(env->regs, 0, sizeof(uint32_t) * NUM_CORE_REGS);
--
2.25.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PULL for-7.1 15/36] util/log: Use qemu_log_lock/unlock in qemu_log
2022-03-20 17:10 [PULL for-7.1 00/36] Logging cleanup and per-thread logfiles Richard Henderson
` (13 preceding siblings ...)
2022-03-20 17:11 ` [PULL for-7.1 14/36] target/nios2: Remove log_cpu_state from reset Richard Henderson
@ 2022-03-20 17:11 ` Richard Henderson
2022-03-23 17:28 ` Alex Bennée
2022-03-20 17:11 ` [PULL for-7.1 16/36] util/log: Remove qemu_log_flush Richard Henderson
` (21 subsequent siblings)
36 siblings, 1 reply; 60+ messages in thread
From: Richard Henderson @ 2022-03-20 17:11 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
Avoid using QemuLogFile and RCU directly.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
util/log.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/util/log.c b/util/log.c
index b1651d8122..3ec6e90575 100644
--- a/util/log.c
+++ b/util/log.c
@@ -62,23 +62,22 @@ void qemu_log_unlock(FILE *fd)
/* Return the number of characters emitted. */
int qemu_log(const char *fmt, ...)
{
+ FILE *f = qemu_log_lock();
int ret = 0;
- QemuLogFile *logfile;
- rcu_read_lock();
- logfile = qatomic_rcu_read(&qemu_logfile);
- if (logfile) {
+ if (f) {
va_list ap;
+
va_start(ap, fmt);
- ret = vfprintf(logfile->fd, fmt, ap);
+ ret = vfprintf(f, fmt, ap);
va_end(ap);
+ qemu_log_unlock(f);
/* Don't pass back error results. */
if (ret < 0) {
ret = 0;
}
}
- rcu_read_unlock();
return ret;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PULL for-7.1 16/36] util/log: Remove qemu_log_flush
2022-03-20 17:10 [PULL for-7.1 00/36] Logging cleanup and per-thread logfiles Richard Henderson
` (14 preceding siblings ...)
2022-03-20 17:11 ` [PULL for-7.1 15/36] util/log: Use qemu_log_lock/unlock in qemu_log Richard Henderson
@ 2022-03-20 17:11 ` Richard Henderson
2022-03-20 21:55 ` Philippe Mathieu-Daudé
2022-03-20 17:11 ` [PULL for-7.1 17/36] util/log: Drop call to setvbuf Richard Henderson
` (20 subsequent siblings)
36 siblings, 1 reply; 60+ messages in thread
From: Richard Henderson @ 2022-03-20 17:11 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
All uses flush output immediately before or after qemu_log_unlock.
Instead of a separate call, move the flush into qemu_log_unlock.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/qemu/log.h | 2 --
accel/tcg/translate-all.c | 1 -
cpu.c | 1 -
hw/net/can/can_sja1000.c | 1 -
hw/xen/xen_pvdev.c | 1 -
net/can/can_socketcan.c | 1 -
tcg/tcg.c | 1 -
util/log.c | 14 +-------------
8 files changed, 1 insertion(+), 21 deletions(-)
diff --git a/include/qemu/log.h b/include/qemu/log.h
index 6e45b31253..10578ba21c 100644
--- a/include/qemu/log.h
+++ b/include/qemu/log.h
@@ -121,8 +121,6 @@ int qemu_str_to_log_mask(const char *str);
*/
void qemu_print_log_usage(FILE *f);
-/* fflush() the log file */
-void qemu_log_flush(void);
/* Close the log file */
void qemu_log_close(void);
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 4af7455522..837f50f5b9 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -1595,7 +1595,6 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
}
}
fprintf(logfile, "\n");
- qemu_log_flush();
qemu_log_unlock(logfile);
}
}
diff --git a/cpu.c b/cpu.c
index 159d0b0d1d..054f21b307 100644
--- a/cpu.c
+++ b/cpu.c
@@ -405,7 +405,6 @@ void cpu_abort(CPUState *cpu, const char *fmt, ...)
vfprintf(logfile, fmt, ap2);
fprintf(logfile, "\n");
cpu_dump_state(cpu, logfile, CPU_DUMP_FPU | CPU_DUMP_CCOP);
- qemu_log_flush();
qemu_log_unlock(logfile);
qemu_log_close();
}
diff --git a/hw/net/can/can_sja1000.c b/hw/net/can/can_sja1000.c
index 919e93664d..3d144508a2 100644
--- a/hw/net/can/can_sja1000.c
+++ b/hw/net/can/can_sja1000.c
@@ -260,7 +260,6 @@ static void can_display_msg(const char *prefix, const qemu_can_frame *msg)
fprintf(logfile, " %02X", msg->data[i]);
}
fprintf(logfile, "\n");
- qemu_log_flush();
qemu_log_unlock(logfile);
}
}
diff --git a/hw/xen/xen_pvdev.c b/hw/xen/xen_pvdev.c
index 355bbda60d..9095080b0d 100644
--- a/hw/xen/xen_pvdev.c
+++ b/hw/xen/xen_pvdev.c
@@ -230,7 +230,6 @@ void xen_pv_printf(struct XenLegacyDevice *xendev, int msg_level,
xen_pv_output_msg(xendev, stderr, fmt, args);
va_end(args);
}
- qemu_log_flush();
}
void xen_pv_evtchn_event(void *opaque)
diff --git a/net/can/can_socketcan.c b/net/can/can_socketcan.c
index ae19d35001..47743e5fd9 100644
--- a/net/can/can_socketcan.c
+++ b/net/can/can_socketcan.c
@@ -88,7 +88,6 @@ static void can_host_socketcan_display_msg(struct qemu_can_frame *msg)
fprintf(logfile, " %02X", msg->data[i]);
}
fprintf(logfile, "\n");
- qemu_log_flush();
qemu_log_unlock(logfile);
}
}
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 1e27367af6..e5d45eca9f 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -783,7 +783,6 @@ void tcg_prologue_init(TCGContext *s)
disas(logfile, s->code_gen_ptr, prologue_size);
}
fprintf(logfile, "\n");
- qemu_log_flush();
qemu_log_unlock(logfile);
}
}
diff --git a/util/log.c b/util/log.c
index 3ec6e90575..73597c40c0 100644
--- a/util/log.c
+++ b/util/log.c
@@ -54,6 +54,7 @@ FILE *qemu_log_lock(void)
void qemu_log_unlock(FILE *fd)
{
if (fd) {
+ fflush(fd);
qemu_funlockfile(fd);
rcu_read_unlock();
}
@@ -277,19 +278,6 @@ out:
g_strfreev(ranges);
}
-/* fflush() the log file */
-void qemu_log_flush(void)
-{
- QemuLogFile *logfile;
-
- rcu_read_lock();
- logfile = qatomic_rcu_read(&qemu_logfile);
- if (logfile) {
- fflush(logfile->fd);
- }
- rcu_read_unlock();
-}
-
/* Close the log file */
void qemu_log_close(void)
{
--
2.25.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* Re: [PULL for-7.1 16/36] util/log: Remove qemu_log_flush
2022-03-20 17:11 ` [PULL for-7.1 16/36] util/log: Remove qemu_log_flush Richard Henderson
@ 2022-03-20 21:55 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 60+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-20 21:55 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: alex.bennee
On 20/3/22 18:11, Richard Henderson wrote:
> All uses flush output immediately before or after qemu_log_unlock.
> Instead of a separate call, move the flush into qemu_log_unlock.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> include/qemu/log.h | 2 --
> accel/tcg/translate-all.c | 1 -
> cpu.c | 1 -
> hw/net/can/can_sja1000.c | 1 -
> hw/xen/xen_pvdev.c | 1 -
> net/can/can_socketcan.c | 1 -
> tcg/tcg.c | 1 -
> util/log.c | 14 +-------------
> 8 files changed, 1 insertion(+), 21 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PULL for-7.1 17/36] util/log: Drop call to setvbuf
2022-03-20 17:10 [PULL for-7.1 00/36] Logging cleanup and per-thread logfiles Richard Henderson
` (15 preceding siblings ...)
2022-03-20 17:11 ` [PULL for-7.1 16/36] util/log: Remove qemu_log_flush Richard Henderson
@ 2022-03-20 17:11 ` Richard Henderson
2022-03-20 17:11 ` [PULL for-7.1 18/36] bsd-user: Expand log_page_dump inline Richard Henderson
` (19 subsequent siblings)
36 siblings, 0 replies; 60+ messages in thread
From: Richard Henderson @ 2022-03-20 17:11 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
Now that the log buffer is flushed after every qemu_log_unlock,
which includes every call to qemu_log, we do not need to force
line buffering (or unbuffering for windows). Block buffer the
entire loggable unit.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
util/log.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/util/log.c b/util/log.c
index 73597c40c0..be20ae6c1c 100644
--- a/util/log.c
+++ b/util/log.c
@@ -145,12 +145,6 @@ void qemu_set_log(int log_flags, Error **errp)
logfile->fd = stderr;
}
-#if defined(_WIN32)
- /* Win32 doesn't support line-buffering, so use unbuffered output. */
- setvbuf(logfile->fd, NULL, _IONBF, 0);
-#else
- setvbuf(logfile->fd, NULL, _IOLBF, 0);
-#endif
log_append = 1;
qatomic_rcu_set(&qemu_logfile, logfile);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PULL for-7.1 18/36] bsd-user: Expand log_page_dump inline
2022-03-20 17:10 [PULL for-7.1 00/36] Logging cleanup and per-thread logfiles Richard Henderson
` (16 preceding siblings ...)
2022-03-20 17:11 ` [PULL for-7.1 17/36] util/log: Drop call to setvbuf Richard Henderson
@ 2022-03-20 17:11 ` Richard Henderson
2022-03-20 17:11 ` [PULL for-7.1 19/36] linux-user: " Richard Henderson
` (18 subsequent siblings)
36 siblings, 0 replies; 60+ messages in thread
From: Richard Henderson @ 2022-03-20 17:11 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
We have extra stuff to log at the same time.
Hoist the qemu_log_lock/unlock to the caller and use fprintf.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
bsd-user/main.c | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/bsd-user/main.c b/bsd-user/main.c
index 0c8aa1ae90..d25d1c6015 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -503,20 +503,25 @@ int main(int argc, char **argv)
g_free(target_environ);
if (qemu_loglevel_mask(CPU_LOG_PAGE)) {
- qemu_log("guest_base %p\n", (void *)guest_base);
- log_page_dump("binary load");
+ FILE *f = qemu_log_lock();
- qemu_log("start_brk 0x" TARGET_ABI_FMT_lx "\n", info->start_brk);
- qemu_log("end_code 0x" TARGET_ABI_FMT_lx "\n", info->end_code);
- qemu_log("start_code 0x" TARGET_ABI_FMT_lx "\n",
+ fprintf(f, "guest_base %p\n", (void *)guest_base);
+ fprintf(f, "page layout changed following binary load\n");
+ page_dump(f);
+
+ fprintf(f, "start_brk 0x" TARGET_ABI_FMT_lx "\n", info->start_brk);
+ fprintf(f, "end_code 0x" TARGET_ABI_FMT_lx "\n", info->end_code);
+ fprintf(f, "start_code 0x" TARGET_ABI_FMT_lx "\n",
info->start_code);
- qemu_log("start_data 0x" TARGET_ABI_FMT_lx "\n",
+ fprintf(f, "start_data 0x" TARGET_ABI_FMT_lx "\n",
info->start_data);
- qemu_log("end_data 0x" TARGET_ABI_FMT_lx "\n", info->end_data);
- qemu_log("start_stack 0x" TARGET_ABI_FMT_lx "\n",
+ fprintf(f, "end_data 0x" TARGET_ABI_FMT_lx "\n", info->end_data);
+ fprintf(f, "start_stack 0x" TARGET_ABI_FMT_lx "\n",
info->start_stack);
- qemu_log("brk 0x" TARGET_ABI_FMT_lx "\n", info->brk);
- qemu_log("entry 0x" TARGET_ABI_FMT_lx "\n", info->entry);
+ fprintf(f, "brk 0x" TARGET_ABI_FMT_lx "\n", info->brk);
+ fprintf(f, "entry 0x" TARGET_ABI_FMT_lx "\n", info->entry);
+
+ qemu_log_unlock(f);
}
/* build Task State */
--
2.25.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PULL for-7.1 19/36] linux-user: Expand log_page_dump inline
2022-03-20 17:10 [PULL for-7.1 00/36] Logging cleanup and per-thread logfiles Richard Henderson
` (17 preceding siblings ...)
2022-03-20 17:11 ` [PULL for-7.1 18/36] bsd-user: Expand log_page_dump inline Richard Henderson
@ 2022-03-20 17:11 ` Richard Henderson
2022-03-20 17:11 ` [PULL for-7.1 20/36] tests/unit: Do not reference QemuLogFile directly Richard Henderson
` (17 subsequent siblings)
36 siblings, 0 replies; 60+ messages in thread
From: Richard Henderson @ 2022-03-20 17:11 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
We have extra stuff to log at the same time.
Hoist the qemu_log_lock/unlock to the caller and use fprintf.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/exec/log.h | 15 ---------------
linux-user/main.c | 43 +++++++++++++++++++++++++++++--------------
linux-user/mmap.c | 7 ++++++-
3 files changed, 35 insertions(+), 30 deletions(-)
diff --git a/include/exec/log.h b/include/exec/log.h
index d131a3140d..ad0a40cfeb 100644
--- a/include/exec/log.h
+++ b/include/exec/log.h
@@ -42,19 +42,4 @@ static inline void log_cpu_state_mask(int mask, CPUState *cpu, int flags)
}
}
-#ifdef NEED_CPU_H
-#if defined(CONFIG_USER_ONLY)
-/* page_dump() output to the log file: */
-static inline void log_page_dump(const char *operation)
-{
- FILE *logfile = qemu_log_lock();
- if (logfile) {
- fprintf(logfile, "page layout changed following %s\n", operation);
- page_dump(logfile);
- }
- qemu_log_unlock(logfile);
-}
-#endif
-#endif
-
#endif
diff --git a/linux-user/main.c b/linux-user/main.c
index 6aed4929ab..2bdee78019 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -857,21 +857,36 @@ int main(int argc, char **argv, char **envp)
g_free(target_environ);
if (qemu_loglevel_mask(CPU_LOG_PAGE)) {
- qemu_log("guest_base %p\n", (void *)guest_base);
- log_page_dump("binary load");
+ FILE *f = qemu_log_lock();
+ if (f) {
+ fprintf(f, "guest_base %p\n", (void *)guest_base);
+ fprintf(f, "page layout changed following binary load\n");
+ page_dump(f);
- qemu_log("start_brk 0x" TARGET_ABI_FMT_lx "\n", info->start_brk);
- qemu_log("end_code 0x" TARGET_ABI_FMT_lx "\n", info->end_code);
- qemu_log("start_code 0x" TARGET_ABI_FMT_lx "\n", info->start_code);
- qemu_log("start_data 0x" TARGET_ABI_FMT_lx "\n", info->start_data);
- qemu_log("end_data 0x" TARGET_ABI_FMT_lx "\n", info->end_data);
- qemu_log("start_stack 0x" TARGET_ABI_FMT_lx "\n", info->start_stack);
- qemu_log("brk 0x" TARGET_ABI_FMT_lx "\n", info->brk);
- qemu_log("entry 0x" TARGET_ABI_FMT_lx "\n", info->entry);
- qemu_log("argv_start 0x" TARGET_ABI_FMT_lx "\n", info->arg_start);
- qemu_log("env_start 0x" TARGET_ABI_FMT_lx "\n",
- info->arg_end + (abi_ulong)sizeof(abi_ulong));
- qemu_log("auxv_start 0x" TARGET_ABI_FMT_lx "\n", info->saved_auxv);
+ fprintf(f, "start_brk 0x" TARGET_ABI_FMT_lx "\n",
+ info->start_brk);
+ fprintf(f, "end_code 0x" TARGET_ABI_FMT_lx "\n",
+ info->end_code);
+ fprintf(f, "start_code 0x" TARGET_ABI_FMT_lx "\n",
+ info->start_code);
+ fprintf(f, "start_data 0x" TARGET_ABI_FMT_lx "\n",
+ info->start_data);
+ fprintf(f, "end_data 0x" TARGET_ABI_FMT_lx "\n",
+ info->end_data);
+ fprintf(f, "start_stack 0x" TARGET_ABI_FMT_lx "\n",
+ info->start_stack);
+ fprintf(f, "brk 0x" TARGET_ABI_FMT_lx "\n",
+ info->brk);
+ fprintf(f, "entry 0x" TARGET_ABI_FMT_lx "\n",
+ info->entry);
+ fprintf(f, "argv_start 0x" TARGET_ABI_FMT_lx "\n",
+ info->arg_start);
+ fprintf(f, "env_start 0x" TARGET_ABI_FMT_lx "\n",
+ info->arg_end + (abi_ulong)sizeof(abi_ulong));
+ fprintf(f, "auxv_start 0x" TARGET_ABI_FMT_lx "\n",
+ info->saved_auxv);
+ qemu_log_unlock(f);
+ }
}
target_set_brk(info->brk);
diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index c125031b90..1aa343a491 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -630,7 +630,12 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot,
the_end:
trace_target_mmap_complete(start);
if (qemu_loglevel_mask(CPU_LOG_PAGE)) {
- log_page_dump(__func__);
+ FILE *f = qemu_log_lock();
+ if (f) {
+ fprintf(f, "page layout changed following mmap\n");
+ page_dump(f);
+ qemu_log_unlock(f);
+ }
}
tb_invalidate_phys_range(start, start + len);
mmap_unlock();
--
2.25.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PULL for-7.1 20/36] tests/unit: Do not reference QemuLogFile directly
2022-03-20 17:10 [PULL for-7.1 00/36] Logging cleanup and per-thread logfiles Richard Henderson
` (18 preceding siblings ...)
2022-03-20 17:11 ` [PULL for-7.1 19/36] linux-user: " Richard Henderson
@ 2022-03-20 17:11 ` Richard Henderson
2022-03-20 17:11 ` [PULL for-7.1 21/36] include/exec/log: " Richard Henderson
` (16 subsequent siblings)
36 siblings, 0 replies; 60+ messages in thread
From: Richard Henderson @ 2022-03-20 17:11 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
Use qemu_log_lock/unlock instead of the raw rcu_read.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
tests/unit/test-logging.c | 33 +++++++++++++++------------------
1 file changed, 15 insertions(+), 18 deletions(-)
diff --git a/tests/unit/test-logging.c b/tests/unit/test-logging.c
index 93c6d666e8..fc22bf40b4 100644
--- a/tests/unit/test-logging.c
+++ b/tests/unit/test-logging.c
@@ -110,12 +110,10 @@ static void test_parse_path(gconstpointer data)
static void test_logfile_write(gconstpointer data)
{
- QemuLogFile *logfile;
- QemuLogFile *logfile2;
+ FILE *logfile0, *logfile1;
gchar const *dir = data;
- g_autofree gchar *file_path = NULL;
+ g_autofree gchar *file_path0 = NULL;
g_autofree gchar *file_path1 = NULL;
- FILE *orig_fd;
/*
* Before starting test, set log flags, to ensure the file gets
@@ -124,29 +122,28 @@ static void test_logfile_write(gconstpointer data)
* this is needed.
*/
qemu_set_log(CPU_LOG_TB_OUT_ASM, &error_abort);
- file_path = g_build_filename(dir, "qemu_test_log_write0.log", NULL);
+ file_path0 = g_build_filename(dir, "qemu_test_log_write0.log", NULL);
file_path1 = g_build_filename(dir, "qemu_test_log_write1.log", NULL);
/*
* Test that even if an open file handle is changed,
* our handle remains valid due to RCU.
*/
- qemu_set_log_filename(file_path, &error_abort);
- rcu_read_lock();
- logfile = qatomic_rcu_read(&qemu_logfile);
- orig_fd = logfile->fd;
- g_assert(logfile && logfile->fd);
- fprintf(logfile->fd, "%s 1st write to file\n", __func__);
- fflush(logfile->fd);
+ qemu_set_log_filename(file_path0, &error_abort);
+ logfile0 = qemu_log_lock();
+ g_assert(logfile0);
+ fprintf(logfile0, "%s 1st write to file\n", __func__);
+ fflush(logfile0);
/* Change the logfile and ensure that the handle is still valid. */
qemu_set_log_filename(file_path1, &error_abort);
- logfile2 = qatomic_rcu_read(&qemu_logfile);
- g_assert(logfile->fd == orig_fd);
- g_assert(logfile2->fd != logfile->fd);
- fprintf(logfile->fd, "%s 2nd write to file\n", __func__);
- fflush(logfile->fd);
- rcu_read_unlock();
+ logfile1 = qemu_log_lock();
+ g_assert(logfile1);
+ g_assert(logfile0 != logfile1);
+ fprintf(logfile0, "%s 2nd write to file\n", __func__);
+ fflush(logfile0);
+ qemu_log_unlock(logfile0);
+ qemu_log_unlock(logfile1);
}
static void test_logfile_lock(gconstpointer data)
--
2.25.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PULL for-7.1 21/36] include/exec/log: Do not reference QemuLogFile directly
2022-03-20 17:10 [PULL for-7.1 00/36] Logging cleanup and per-thread logfiles Richard Henderson
` (19 preceding siblings ...)
2022-03-20 17:11 ` [PULL for-7.1 20/36] tests/unit: Do not reference QemuLogFile directly Richard Henderson
@ 2022-03-20 17:11 ` Richard Henderson
2022-03-20 17:11 ` [PULL for-7.1 22/36] include/qemu/log: Move entire implementation out-of-line Richard Henderson
` (15 subsequent siblings)
36 siblings, 0 replies; 60+ messages in thread
From: Richard Henderson @ 2022-03-20 17:11 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
Use qemu_log_lock/unlock instead of the raw rcu_read.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/exec/log.h | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/include/exec/log.h b/include/exec/log.h
index ad0a40cfeb..10d0e809d4 100644
--- a/include/exec/log.h
+++ b/include/exec/log.h
@@ -15,15 +15,10 @@
*/
static inline void log_cpu_state(CPUState *cpu, int flags)
{
- QemuLogFile *logfile;
-
- if (qemu_log_enabled()) {
- rcu_read_lock();
- logfile = qatomic_rcu_read(&qemu_logfile);
- if (logfile) {
- cpu_dump_state(cpu, logfile->fd, flags);
- }
- rcu_read_unlock();
+ FILE *f = qemu_log_lock();
+ if (f) {
+ cpu_dump_state(cpu, f, flags);
+ qemu_log_unlock(f);
}
}
--
2.25.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PULL for-7.1 22/36] include/qemu/log: Move entire implementation out-of-line
2022-03-20 17:10 [PULL for-7.1 00/36] Logging cleanup and per-thread logfiles Richard Henderson
` (20 preceding siblings ...)
2022-03-20 17:11 ` [PULL for-7.1 21/36] include/exec/log: " Richard Henderson
@ 2022-03-20 17:11 ` Richard Henderson
2022-03-20 17:11 ` [PULL for-7.1 23/36] sysemu/os-win32: Test for and use _lock_file/_unlock_file Richard Henderson
` (14 subsequent siblings)
36 siblings, 0 replies; 60+ messages in thread
From: Richard Henderson @ 2022-03-20 17:11 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
Move QemuLogFile, qemu_logfile, and all inline functions into qemu/log.c.
No need to expose these implementation details in the api.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/qemu/log.h | 38 ++++----------------------------------
tests/unit/test-logging.c | 1 +
util/log.c | 30 +++++++++++++++++++++++++++++-
3 files changed, 34 insertions(+), 35 deletions(-)
diff --git a/include/qemu/log.h b/include/qemu/log.h
index 10578ba21c..c63416c9e0 100644
--- a/include/qemu/log.h
+++ b/include/qemu/log.h
@@ -3,46 +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 QemuLogFile *qemu_logfile;
-
/*
* The new API:
- *
*/
-/* Log settings checking macros: */
+/* Returns true if qemu_log() will really write somewhere. */
+bool qemu_log_enabled(void);
-/* Returns true if qemu_log() will really write somewhere
- */
-static inline bool qemu_log_enabled(void)
-{
- return qemu_logfile != NULL;
-}
-
-/* Returns true if qemu_log() will write somewhere else than stderr
- */
-static inline bool qemu_log_separate(void)
-{
- QemuLogFile *logfile;
- bool res = false;
-
- rcu_read_lock();
- logfile = qatomic_rcu_read(&qemu_logfile);
- if (logfile && logfile->fd != stderr) {
- res = true;
- }
- rcu_read_unlock();
- return res;
-}
+/* Returns true if qemu_log() will write somewhere other than stderr. */
+bool qemu_log_separate(void);
#define CPU_LOG_TB_OUT_ASM (1 << 0)
#define CPU_LOG_TB_IN_ASM (1 << 1)
diff --git a/tests/unit/test-logging.c b/tests/unit/test-logging.c
index fc22bf40b4..7016710a74 100644
--- a/tests/unit/test-logging.c
+++ b/tests/unit/test-logging.c
@@ -30,6 +30,7 @@
#include "qemu-common.h"
#include "qapi/error.h"
#include "qemu/log.h"
+#include "qemu/rcu.h"
static void test_parse_range(void)
{
diff --git a/util/log.c b/util/log.c
index be20ae6c1c..7e8b152742 100644
--- a/util/log.c
+++ b/util/log.c
@@ -26,14 +26,42 @@
#include "trace/control.h"
#include "qemu/thread.h"
#include "qemu/lockable.h"
+#include "qemu/rcu.h"
+
+
+typedef struct QemuLogFile {
+ struct rcu_head rcu;
+ FILE *fd;
+} QemuLogFile;
static char *logfilename;
static QemuMutex qemu_logfile_mutex;
-QemuLogFile *qemu_logfile;
+static QemuLogFile *qemu_logfile;
int qemu_loglevel;
static int log_append = 0;
static GArray *debug_regions;
+/* Returns true if qemu_log() will really write somewhere. */
+bool qemu_log_enabled(void)
+{
+ return qemu_logfile != NULL;
+}
+
+/* Returns true if qemu_log() will write somewhere other than stderr. */
+bool qemu_log_separate(void)
+{
+ QemuLogFile *logfile;
+ bool res = false;
+
+ rcu_read_lock();
+ logfile = qatomic_rcu_read(&qemu_logfile);
+ if (logfile && logfile->fd != stderr) {
+ res = true;
+ }
+ rcu_read_unlock();
+ return res;
+}
+
/* Lock/unlock output. */
FILE *qemu_log_lock(void)
--
2.25.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PULL for-7.1 23/36] sysemu/os-win32: Test for and use _lock_file/_unlock_file
2022-03-20 17:10 [PULL for-7.1 00/36] Logging cleanup and per-thread logfiles Richard Henderson
` (21 preceding siblings ...)
2022-03-20 17:11 ` [PULL for-7.1 22/36] include/qemu/log: Move entire implementation out-of-line Richard Henderson
@ 2022-03-20 17:11 ` Richard Henderson
2022-03-20 17:11 ` [PULL for-7.1 24/36] util/log: Introduce qemu_set_log_filename_flags Richard Henderson
` (13 subsequent siblings)
36 siblings, 0 replies; 60+ messages in thread
From: Richard Henderson @ 2022-03-20 17:11 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
The bug referenced in os-win32.h was fixed in mingw-w64 v6.
According to repology, version 5 used by ubuntu 18, which is
not yet out of support, so provide a meson link test for it.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
meson.build | 12 ++++++++++++
include/sysemu/os-win32.h | 16 +++++++++-------
2 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/meson.build b/meson.build
index 282e7c4650..f22a09dac5 100644
--- a/meson.build
+++ b/meson.build
@@ -2018,6 +2018,18 @@ foreach k, v: config_host
endif
endforeach
+# Older versions of MinGW do not import _lock_file and _unlock_file properly.
+# This was fixed for v6.0.0 with commit b48e3ac8969d.
+if targetos == 'windows'
+ config_host_data.set('HAVE__LOCK_FILE', cc.links('''
+ #include <stdio.h>
+ int main(void) {
+ _lock_file(NULL);
+ _unlock_file(NULL);
+ return 0;
+ }''', name: '_lock_file and _unlock_file'))
+endif
+
########################
# Target configuration #
########################
diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
index 770752222a..649f417d39 100644
--- a/include/sysemu/os-win32.h
+++ b/include/sysemu/os-win32.h
@@ -113,20 +113,22 @@ static inline char *realpath(const char *path, char *resolved_path)
return resolved_path;
}
-/* ??? Mingw appears to export _lock_file and _unlock_file as the functions
- * with which to lock a stdio handle. But something is wrong in the markup,
- * either in the header or the library, such that we get undefined references
- * to "_imp___lock_file" etc when linking. Since we seem to have no other
- * alternative, and the usage within the logging functions isn't critical,
- * ignore FILE locking.
+/*
+ * Older versions of MinGW do not import _lock_file and _unlock_file properly.
+ * This was fixed for v6.0.0 with commit b48e3ac8969d.
*/
-
static inline void qemu_flockfile(FILE *f)
{
+#ifdef HAVE__LOCK_FILE
+ _lock_file(f);
+#endif
}
static inline void qemu_funlockfile(FILE *f)
{
+#ifdef HAVE__LOCK_FILE
+ _unlock_file(f);
+#endif
}
/* We wrap all the sockets functions so that we can
--
2.25.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PULL for-7.1 24/36] util/log: Introduce qemu_set_log_filename_flags
2022-03-20 17:10 [PULL for-7.1 00/36] Logging cleanup and per-thread logfiles Richard Henderson
` (22 preceding siblings ...)
2022-03-20 17:11 ` [PULL for-7.1 23/36] sysemu/os-win32: Test for and use _lock_file/_unlock_file Richard Henderson
@ 2022-03-20 17:11 ` Richard Henderson
2022-03-20 17:11 ` [PULL for-7.1 25/36] bsd-user: Use qemu_set_log_filename_flags Richard Henderson
` (12 subsequent siblings)
36 siblings, 0 replies; 60+ messages in thread
From: Richard Henderson @ 2022-03-20 17:11 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
Provide a function to set both filename and flags at
the same time. This is the common case at startup.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/qemu/log.h | 1 +
util/log.c | 122 ++++++++++++++++++++++++++++-----------------
2 files changed, 77 insertions(+), 46 deletions(-)
diff --git a/include/qemu/log.h b/include/qemu/log.h
index c63416c9e0..ef247776cf 100644
--- a/include/qemu/log.h
+++ b/include/qemu/log.h
@@ -82,6 +82,7 @@ extern const QEMULogItem qemu_log_items[];
void qemu_set_log(int log_flags, Error **errp);
void qemu_set_log_filename(const char *filename, Error **errp);
+void qemu_set_log_filename_flags(const char *name, int flags, Error **errp);
void qemu_set_dfilter_ranges(const char *ranges, Error **errp);
bool qemu_log_in_addr_range(uint64_t addr);
int qemu_str_to_log_mask(const char *str);
diff --git a/util/log.c b/util/log.c
index 7e8b152742..46a0d38d41 100644
--- a/util/log.c
+++ b/util/log.c
@@ -126,15 +126,58 @@ static void qemu_logfile_free(QemuLogFile *logfile)
}
/* enable or disable low levels log */
-void qemu_set_log(int log_flags, Error **errp)
+static void qemu_set_log_internal(const char *filename, bool changed_name,
+ int log_flags, Error **errp)
{
- bool need_to_open_file = false;
+ bool need_to_open_file;
QemuLogFile *logfile;
- qemu_loglevel = log_flags;
+ QEMU_LOCK_GUARD(&qemu_logfile_mutex);
+ logfile = qemu_logfile;
+
+ if (changed_name) {
+ char *newname = NULL;
+
+ /*
+ * Allow the user to include %d in their logfile which will be
+ * substituted with the current PID. This is useful for debugging many
+ * nested linux-user tasks but will result in lots of logs.
+ *
+ * filename may be NULL. In that case, log output is sent to stderr
+ */
+ if (filename) {
+ char *pidstr = strstr(filename, "%");
+
+ if (pidstr) {
+ /* We only accept one %d, no other format strings */
+ if (pidstr[1] != 'd' || strchr(pidstr + 2, '%')) {
+ error_setg(errp, "Bad logfile format: %s", filename);
+ return;
+ }
+ newname = g_strdup_printf(filename, getpid());
+ } else {
+ newname = g_strdup(filename);
+ }
+ }
+
+ g_free(logfilename);
+ logfilename = newname;
+ filename = newname;
+
+ if (logfile) {
+ qatomic_rcu_set(&qemu_logfile, NULL);
+ call_rcu(logfile, qemu_logfile_free, rcu);
+ logfile = NULL;
+ }
+ } else {
+ filename = logfilename;
+ }
+
#ifdef CONFIG_TRACE_LOG
- qemu_loglevel |= LOG_TRACE;
+ log_flags |= LOG_TRACE;
#endif
+ qemu_loglevel = log_flags;
+
/*
* In all cases we only log if qemu_loglevel is set.
* Also:
@@ -143,70 +186,57 @@ void qemu_set_log(int log_flags, Error **errp)
* If we are daemonized,
* we will only log if there is a logfilename.
*/
- if (qemu_loglevel && (!is_daemonized() || logfilename)) {
- need_to_open_file = true;
- }
- QEMU_LOCK_GUARD(&qemu_logfile_mutex);
- if (qemu_logfile && !need_to_open_file) {
- logfile = qemu_logfile;
+ need_to_open_file = log_flags && (!is_daemonized() || filename);
+
+ if (logfile && !need_to_open_file) {
qatomic_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) {
- logfile->fd = fopen(logfilename, log_append ? "a" : "w");
- if (!logfile->fd) {
+ return;
+ }
+ if (!logfile && need_to_open_file) {
+ FILE *fd;
+
+ if (filename) {
+ fd = fopen(filename, log_append ? "a" : "w");
+ if (!fd) {
error_setg_errno(errp, errno, "Error opening logfile %s",
- logfilename);
+ filename);
return;
}
/* In case we are a daemon redirect stderr to logfile */
if (is_daemonized()) {
- dup2(fileno(logfile->fd), STDERR_FILENO);
- fclose(logfile->fd);
+ dup2(fileno(fd), STDERR_FILENO);
+ fclose(fd);
/* This will skip closing logfile in qemu_log_close() */
- logfile->fd = stderr;
+ fd = stderr;
}
} else {
/* Default to stderr if no log file specified */
assert(!is_daemonized());
- logfile->fd = stderr;
+ fd = stderr;
}
log_append = 1;
+
+ logfile = g_new0(QemuLogFile, 1);
+ logfile->fd = fd;
qatomic_rcu_set(&qemu_logfile, logfile);
}
}
-/*
- * Allow the user to include %d in their logfile which will be
- * substituted with the current PID. This is useful for debugging many
- * nested linux-user tasks but will result in lots of logs.
- *
- * filename may be NULL. In that case, log output is sent to stderr
- */
+void qemu_set_log(int log_flags, Error **errp)
+{
+ qemu_set_log_internal(NULL, false, log_flags, errp);
+}
+
void qemu_set_log_filename(const char *filename, Error **errp)
{
- g_free(logfilename);
- logfilename = NULL;
+ qemu_set_log_internal(filename, true, qemu_loglevel, errp);
+}
- if (filename) {
- char *pidstr = strstr(filename, "%");
- if (pidstr) {
- /* We only accept one %d, no other format strings */
- if (pidstr[1] != 'd' || strchr(pidstr + 2, '%')) {
- error_setg(errp, "Bad logfile format: %s", filename);
- return;
- } else {
- logfilename = g_strdup_printf(filename, getpid());
- }
- } else {
- logfilename = g_strdup(filename);
- }
- }
-
- qemu_log_close();
- qemu_set_log(qemu_loglevel, errp);
+void qemu_set_log_filename_flags(const char *name, int flags, Error **errp)
+{
+ qemu_set_log_internal(name, true, flags, errp);
}
/* Returns true if addr is in our debug filter or no filter defined
--
2.25.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PULL for-7.1 25/36] bsd-user: Use qemu_set_log_filename_flags
2022-03-20 17:10 [PULL for-7.1 00/36] Logging cleanup and per-thread logfiles Richard Henderson
` (23 preceding siblings ...)
2022-03-20 17:11 ` [PULL for-7.1 24/36] util/log: Introduce qemu_set_log_filename_flags Richard Henderson
@ 2022-03-20 17:11 ` Richard Henderson
2022-03-20 17:11 ` [PULL for-7.1 26/36] linux-user: " Richard Henderson
` (11 subsequent siblings)
36 siblings, 0 replies; 60+ messages in thread
From: Richard Henderson @ 2022-03-20 17:11 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
Perform all logfile setup in one step.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
bsd-user/main.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/bsd-user/main.c b/bsd-user/main.c
index d25d1c6015..2c8f8d1c19 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -405,16 +405,16 @@ int main(int argc, char **argv)
}
/* init debug */
- qemu_set_log_filename(log_file, &error_fatal);
- if (log_mask) {
- int mask;
-
- mask = qemu_str_to_log_mask(log_mask);
- if (!mask) {
- qemu_print_log_usage(stdout);
- exit(1);
+ {
+ int mask = 0;
+ if (log_mask) {
+ mask = qemu_str_to_log_mask(log_mask);
+ if (!mask) {
+ qemu_print_log_usage(stdout);
+ exit(1);
+ }
}
- qemu_set_log(mask, &error_fatal);
+ qemu_set_log_filename_flags(log_file, mask, &error_fatal);
}
if (optind >= argc) {
--
2.25.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PULL for-7.1 26/36] linux-user: Use qemu_set_log_filename_flags
2022-03-20 17:10 [PULL for-7.1 00/36] Logging cleanup and per-thread logfiles Richard Henderson
` (24 preceding siblings ...)
2022-03-20 17:11 ` [PULL for-7.1 25/36] bsd-user: Use qemu_set_log_filename_flags Richard Henderson
@ 2022-03-20 17:11 ` Richard Henderson
2022-03-20 17:11 ` [PULL for-7.1 27/36] softmmu: " Richard Henderson
` (10 subsequent siblings)
36 siblings, 0 replies; 60+ messages in thread
From: Richard Henderson @ 2022-03-20 17:11 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
Perform all logfile setup in one step.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
linux-user/main.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/linux-user/main.c b/linux-user/main.c
index 2bdee78019..75d13eff97 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -85,6 +85,7 @@ static bool enable_strace;
* Used to support command line arguments overriding environment variables.
*/
static int last_log_mask;
+static const char *last_log_filename;
/*
* When running 32-on-64 we should make sure we can fit all of the possible
@@ -257,7 +258,7 @@ static void handle_arg_dfilter(const char *arg)
static void handle_arg_log_filename(const char *arg)
{
- qemu_set_log_filename(arg, &error_fatal);
+ last_log_filename = arg;
}
static void handle_arg_set_env(const char *arg)
@@ -643,7 +644,6 @@ int main(int argc, char **argv, char **envp)
int i;
int ret;
int execfd;
- int log_mask;
unsigned long max_reserved_va;
bool preserve_argv0;
@@ -677,10 +677,9 @@ int main(int argc, char **argv, char **envp)
optind = parse_args(argc, argv);
- log_mask = last_log_mask | (enable_strace ? LOG_STRACE : 0);
- if (log_mask) {
- qemu_set_log(log_mask, &error_fatal);
- }
+ qemu_set_log_filename_flags(last_log_filename,
+ last_log_mask | (enable_strace * LOG_STRACE),
+ &error_fatal);
if (!trace_init_backends()) {
exit(1);
--
2.25.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PULL for-7.1 27/36] softmmu: Use qemu_set_log_filename_flags
2022-03-20 17:10 [PULL for-7.1 00/36] Logging cleanup and per-thread logfiles Richard Henderson
` (25 preceding siblings ...)
2022-03-20 17:11 ` [PULL for-7.1 26/36] linux-user: " Richard Henderson
@ 2022-03-20 17:11 ` Richard Henderson
2022-03-20 17:11 ` [PULL for-7.1 28/36] util/log: Remove qemu_log_close Richard Henderson
` (9 subsequent siblings)
36 siblings, 0 replies; 60+ messages in thread
From: Richard Henderson @ 2022-03-20 17:11 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
Perform all logfile setup at startup in one step.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
softmmu/vl.c | 21 +++++++++------------
1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 73296015af..81c91ec7b0 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2559,19 +2559,16 @@ static void qemu_process_early_options(void)
#endif
/* Open the logfile at this point and set the log mask if necessary. */
- if (log_file) {
- qemu_set_log_filename(log_file, &error_fatal);
- }
- if (log_mask) {
- int mask;
- mask = qemu_str_to_log_mask(log_mask);
- if (!mask) {
- qemu_print_log_usage(stdout);
- exit(1);
+ {
+ int mask = 0;
+ if (log_mask) {
+ mask = qemu_str_to_log_mask(log_mask);
+ if (!mask) {
+ qemu_print_log_usage(stdout);
+ exit(1);
+ }
}
- qemu_set_log(mask, &error_fatal);
- } else {
- qemu_set_log(0, &error_fatal);
+ qemu_set_log_filename_flags(log_file, mask, &error_fatal);
}
qemu_add_default_firmwarepath();
--
2.25.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PULL for-7.1 28/36] util/log: Remove qemu_log_close
2022-03-20 17:10 [PULL for-7.1 00/36] Logging cleanup and per-thread logfiles Richard Henderson
` (26 preceding siblings ...)
2022-03-20 17:11 ` [PULL for-7.1 27/36] softmmu: " Richard Henderson
@ 2022-03-20 17:11 ` Richard Henderson
2022-03-20 17:11 ` [PULL for-7.1 29/36] util/log: Rename logfilename to global_filename Richard Henderson
` (8 subsequent siblings)
36 siblings, 0 replies; 60+ messages in thread
From: Richard Henderson @ 2022-03-20 17:11 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
The only real use is in cpu_abort, where we have just
flushed the file via qemu_log_unlock, and are just about
to force-crash the application via abort. We do not
really need to close the FILE before the abort.
The two uses in test-logging.c can be handled with
qemu_set_log_filename_flags.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/qemu/log.h | 3 ---
cpu.c | 1 -
tests/unit/test-logging.c | 4 ++--
util/log.c | 17 +----------------
4 files changed, 3 insertions(+), 22 deletions(-)
diff --git a/include/qemu/log.h b/include/qemu/log.h
index ef247776cf..783faec309 100644
--- a/include/qemu/log.h
+++ b/include/qemu/log.h
@@ -92,7 +92,4 @@ int qemu_str_to_log_mask(const char *str);
*/
void qemu_print_log_usage(FILE *f);
-/* Close the log file */
-void qemu_log_close(void);
-
#endif
diff --git a/cpu.c b/cpu.c
index 054f21b307..3eb324c1c9 100644
--- a/cpu.c
+++ b/cpu.c
@@ -406,7 +406,6 @@ void cpu_abort(CPUState *cpu, const char *fmt, ...)
fprintf(logfile, "\n");
cpu_dump_state(cpu, logfile, CPU_DUMP_FPU | CPU_DUMP_CCOP);
qemu_log_unlock(logfile);
- qemu_log_close();
}
va_end(ap2);
va_end(ap);
diff --git a/tests/unit/test-logging.c b/tests/unit/test-logging.c
index 7016710a74..4e649e44c5 100644
--- a/tests/unit/test-logging.c
+++ b/tests/unit/test-logging.c
@@ -170,7 +170,7 @@ static void test_logfile_lock(gconstpointer data)
* Initiate a close file and make sure our handle remains
* valid since we still have the logfile lock.
*/
- qemu_log_close();
+ qemu_set_log_filename_flags(NULL, 0, &error_abort);
fprintf(logfile, "%s 2nd write to file\n", __func__);
fflush(logfile);
qemu_log_unlock(logfile);
@@ -208,7 +208,7 @@ int main(int argc, char **argv)
tmp_path, test_logfile_lock);
rc = g_test_run();
- qemu_log_close();
+ qemu_set_log_filename_flags(NULL, 0, &error_abort);
drain_call_rcu();
rmdir_full(tmp_path);
diff --git a/util/log.c b/util/log.c
index 46a0d38d41..691d6d7930 100644
--- a/util/log.c
+++ b/util/log.c
@@ -207,7 +207,7 @@ static void qemu_set_log_internal(const char *filename, bool changed_name,
if (is_daemonized()) {
dup2(fileno(fd), STDERR_FILENO);
fclose(fd);
- /* This will skip closing logfile in qemu_log_close() */
+ /* This will skip closing logfile in qemu_logfile_free. */
fd = stderr;
}
} else {
@@ -330,21 +330,6 @@ out:
g_strfreev(ranges);
}
-/* Close the log file */
-void qemu_log_close(void)
-{
- QemuLogFile *logfile;
-
- qemu_mutex_lock(&qemu_logfile_mutex);
- logfile = qemu_logfile;
-
- if (logfile) {
- qatomic_rcu_set(&qemu_logfile, NULL);
- call_rcu(logfile, qemu_logfile_free, rcu);
- }
- qemu_mutex_unlock(&qemu_logfile_mutex);
-}
-
const QEMULogItem qemu_log_items[] = {
{ CPU_LOG_TB_OUT_ASM, "out_asm",
"show generated host assembly code for each compiled TB" },
--
2.25.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PULL for-7.1 29/36] util/log: Rename logfilename to global_filename
2022-03-20 17:10 [PULL for-7.1 00/36] Logging cleanup and per-thread logfiles Richard Henderson
` (27 preceding siblings ...)
2022-03-20 17:11 ` [PULL for-7.1 28/36] util/log: Remove qemu_log_close Richard Henderson
@ 2022-03-20 17:11 ` Richard Henderson
2022-03-20 21:59 ` Philippe Mathieu-Daudé
2022-03-20 17:11 ` [PULL for-7.1 30/36] util/log: Rename qemu_logfile to global_file Richard Henderson
` (7 subsequent siblings)
36 siblings, 1 reply; 60+ messages in thread
From: Richard Henderson @ 2022-03-20 17:11 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
Rename to emphasize this is the file-scope global variable.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
util/log.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/util/log.c b/util/log.c
index 691d6d7930..e559d37369 100644
--- a/util/log.c
+++ b/util/log.c
@@ -34,7 +34,7 @@ typedef struct QemuLogFile {
FILE *fd;
} QemuLogFile;
-static char *logfilename;
+static char *global_filename;
static QemuMutex qemu_logfile_mutex;
static QemuLogFile *qemu_logfile;
int qemu_loglevel;
@@ -160,8 +160,8 @@ static void qemu_set_log_internal(const char *filename, bool changed_name,
}
}
- g_free(logfilename);
- logfilename = newname;
+ g_free(global_filename);
+ global_filename = newname;
filename = newname;
if (logfile) {
@@ -170,7 +170,7 @@ static void qemu_set_log_internal(const char *filename, bool changed_name,
logfile = NULL;
}
} else {
- filename = logfilename;
+ filename = global_filename;
}
#ifdef CONFIG_TRACE_LOG
@@ -182,9 +182,8 @@ static void qemu_set_log_internal(const char *filename, bool changed_name,
* In all cases we only log if qemu_loglevel is set.
* Also:
* If not daemonized we will always log either to stderr
- * or to a file (if there is a logfilename).
- * If we are daemonized,
- * we will only log if there is a logfilename.
+ * or to a file (if there is a filename).
+ * If we are daemonized, we will only log if there is a filename.
*/
need_to_open_file = log_flags && (!is_daemonized() || filename);
--
2.25.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PULL for-7.1 30/36] util/log: Rename qemu_logfile to global_file
2022-03-20 17:10 [PULL for-7.1 00/36] Logging cleanup and per-thread logfiles Richard Henderson
` (28 preceding siblings ...)
2022-03-20 17:11 ` [PULL for-7.1 29/36] util/log: Rename logfilename to global_filename Richard Henderson
@ 2022-03-20 17:11 ` Richard Henderson
2022-03-20 22:00 ` Philippe Mathieu-Daudé
2022-03-20 17:11 ` [PULL for-7.1 31/36] util/log: Rename qemu_logfile_mutex to global_mutex Richard Henderson
` (6 subsequent siblings)
36 siblings, 1 reply; 60+ messages in thread
From: Richard Henderson @ 2022-03-20 17:11 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
Rename to emphasize this is the file-scope global variable.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
util/log.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/util/log.c b/util/log.c
index e559d37369..0676ce5bd6 100644
--- a/util/log.c
+++ b/util/log.c
@@ -36,7 +36,7 @@ typedef struct QemuLogFile {
static char *global_filename;
static QemuMutex qemu_logfile_mutex;
-static QemuLogFile *qemu_logfile;
+static QemuLogFile *global_file;
int qemu_loglevel;
static int log_append = 0;
static GArray *debug_regions;
@@ -44,7 +44,7 @@ static GArray *debug_regions;
/* Returns true if qemu_log() will really write somewhere. */
bool qemu_log_enabled(void)
{
- return qemu_logfile != NULL;
+ return global_file != NULL;
}
/* Returns true if qemu_log() will write somewhere other than stderr. */
@@ -54,7 +54,7 @@ bool qemu_log_separate(void)
bool res = false;
rcu_read_lock();
- logfile = qatomic_rcu_read(&qemu_logfile);
+ logfile = qatomic_rcu_read(&global_file);
if (logfile && logfile->fd != stderr) {
res = true;
}
@@ -69,7 +69,7 @@ FILE *qemu_log_lock(void)
QemuLogFile *logfile;
rcu_read_lock();
- logfile = qatomic_rcu_read(&qemu_logfile);
+ logfile = qatomic_rcu_read(&global_file);
if (logfile) {
qemu_flockfile(logfile->fd);
return logfile->fd;
@@ -133,7 +133,7 @@ static void qemu_set_log_internal(const char *filename, bool changed_name,
QemuLogFile *logfile;
QEMU_LOCK_GUARD(&qemu_logfile_mutex);
- logfile = qemu_logfile;
+ logfile = global_file;
if (changed_name) {
char *newname = NULL;
@@ -165,7 +165,7 @@ static void qemu_set_log_internal(const char *filename, bool changed_name,
filename = newname;
if (logfile) {
- qatomic_rcu_set(&qemu_logfile, NULL);
+ qatomic_rcu_set(&global_file, NULL);
call_rcu(logfile, qemu_logfile_free, rcu);
logfile = NULL;
}
@@ -188,7 +188,7 @@ static void qemu_set_log_internal(const char *filename, bool changed_name,
need_to_open_file = log_flags && (!is_daemonized() || filename);
if (logfile && !need_to_open_file) {
- qatomic_rcu_set(&qemu_logfile, NULL);
+ qatomic_rcu_set(&global_file, NULL);
call_rcu(logfile, qemu_logfile_free, rcu);
return;
}
@@ -219,7 +219,7 @@ static void qemu_set_log_internal(const char *filename, bool changed_name,
logfile = g_new0(QemuLogFile, 1);
logfile->fd = fd;
- qatomic_rcu_set(&qemu_logfile, logfile);
+ qatomic_rcu_set(&global_file, logfile);
}
}
--
2.25.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PULL for-7.1 31/36] util/log: Rename qemu_logfile_mutex to global_mutex
2022-03-20 17:10 [PULL for-7.1 00/36] Logging cleanup and per-thread logfiles Richard Henderson
` (29 preceding siblings ...)
2022-03-20 17:11 ` [PULL for-7.1 30/36] util/log: Rename qemu_logfile to global_file Richard Henderson
@ 2022-03-20 17:11 ` Richard Henderson
2022-03-20 17:11 ` [PULL for-7.1 32/36] util/log: Hoist the eval of is_daemonized in qemu_set_log_internal Richard Henderson
` (5 subsequent siblings)
36 siblings, 0 replies; 60+ messages in thread
From: Richard Henderson @ 2022-03-20 17:11 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
Rename to emphasize this covers the file-scope global variables.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
util/log.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/util/log.c b/util/log.c
index 0676ce5bd6..42b13e6bf1 100644
--- a/util/log.c
+++ b/util/log.c
@@ -34,9 +34,11 @@ typedef struct QemuLogFile {
FILE *fd;
} QemuLogFile;
+/* Mutex covering the other global_* variables. */
+static QemuMutex global_mutex;
static char *global_filename;
-static QemuMutex qemu_logfile_mutex;
static QemuLogFile *global_file;
+
int qemu_loglevel;
static int log_append = 0;
static GArray *debug_regions;
@@ -110,9 +112,9 @@ int qemu_log(const char *fmt, ...)
return ret;
}
-static void __attribute__((__constructor__)) qemu_logfile_init(void)
+static void __attribute__((__constructor__)) startup(void)
{
- qemu_mutex_init(&qemu_logfile_mutex);
+ qemu_mutex_init(&global_mutex);
}
static void qemu_logfile_free(QemuLogFile *logfile)
@@ -132,7 +134,7 @@ static void qemu_set_log_internal(const char *filename, bool changed_name,
bool need_to_open_file;
QemuLogFile *logfile;
- QEMU_LOCK_GUARD(&qemu_logfile_mutex);
+ QEMU_LOCK_GUARD(&global_mutex);
logfile = global_file;
if (changed_name) {
--
2.25.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PULL for-7.1 32/36] util/log: Hoist the eval of is_daemonized in qemu_set_log_internal
2022-03-20 17:10 [PULL for-7.1 00/36] Logging cleanup and per-thread logfiles Richard Henderson
` (30 preceding siblings ...)
2022-03-20 17:11 ` [PULL for-7.1 31/36] util/log: Rename qemu_logfile_mutex to global_mutex Richard Henderson
@ 2022-03-20 17:11 ` Richard Henderson
2022-03-20 21:58 ` Philippe Mathieu-Daudé
2022-03-20 17:11 ` [PULL for-7.1 33/36] util/log: Combine two logfile closes Richard Henderson
` (4 subsequent siblings)
36 siblings, 1 reply; 60+ messages in thread
From: Richard Henderson @ 2022-03-20 17:11 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
Only call is_daemonized once.
We require the result on all paths after this point.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
util/log.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/util/log.c b/util/log.c
index 42b13e6bf1..f1f6d09c90 100644
--- a/util/log.c
+++ b/util/log.c
@@ -132,6 +132,7 @@ static void qemu_set_log_internal(const char *filename, bool changed_name,
int log_flags, Error **errp)
{
bool need_to_open_file;
+ bool daemonized;
QemuLogFile *logfile;
QEMU_LOCK_GUARD(&global_mutex);
@@ -187,7 +188,8 @@ static void qemu_set_log_internal(const char *filename, bool changed_name,
* or to a file (if there is a filename).
* If we are daemonized, we will only log if there is a filename.
*/
- need_to_open_file = log_flags && (!is_daemonized() || filename);
+ daemonized = is_daemonized();
+ need_to_open_file = log_flags && (!daemonized || filename);
if (logfile && !need_to_open_file) {
qatomic_rcu_set(&global_file, NULL);
@@ -205,7 +207,7 @@ static void qemu_set_log_internal(const char *filename, bool changed_name,
return;
}
/* In case we are a daemon redirect stderr to logfile */
- if (is_daemonized()) {
+ if (daemonized) {
dup2(fileno(fd), STDERR_FILENO);
fclose(fd);
/* This will skip closing logfile in qemu_logfile_free. */
@@ -213,7 +215,7 @@ static void qemu_set_log_internal(const char *filename, bool changed_name,
}
} else {
/* Default to stderr if no log file specified */
- assert(!is_daemonized());
+ assert(!daemonized);
fd = stderr;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PULL for-7.1 33/36] util/log: Combine two logfile closes
2022-03-20 17:10 [PULL for-7.1 00/36] Logging cleanup and per-thread logfiles Richard Henderson
` (31 preceding siblings ...)
2022-03-20 17:11 ` [PULL for-7.1 32/36] util/log: Hoist the eval of is_daemonized in qemu_set_log_internal Richard Henderson
@ 2022-03-20 17:11 ` Richard Henderson
2022-03-20 17:11 ` [PULL for-7.1 34/36] util/log: Rename QemuLogFile to RCUCloseFILE Richard Henderson
` (3 subsequent siblings)
36 siblings, 0 replies; 60+ messages in thread
From: Richard Henderson @ 2022-03-20 17:11 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
Merge the close from the changed_name block with the close
from the !need_to_open_file block.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
util/log.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/util/log.c b/util/log.c
index f1f6d09c90..596de0050f 100644
--- a/util/log.c
+++ b/util/log.c
@@ -166,12 +166,6 @@ static void qemu_set_log_internal(const char *filename, bool changed_name,
g_free(global_filename);
global_filename = newname;
filename = newname;
-
- if (logfile) {
- qatomic_rcu_set(&global_file, NULL);
- call_rcu(logfile, qemu_logfile_free, rcu);
- logfile = NULL;
- }
} else {
filename = global_filename;
}
@@ -191,11 +185,12 @@ static void qemu_set_log_internal(const char *filename, bool changed_name,
daemonized = is_daemonized();
need_to_open_file = log_flags && (!daemonized || filename);
- if (logfile && !need_to_open_file) {
+ if (logfile && (!need_to_open_file || changed_name)) {
qatomic_rcu_set(&global_file, NULL);
call_rcu(logfile, qemu_logfile_free, rcu);
- return;
+ logfile = NULL;
}
+
if (!logfile && need_to_open_file) {
FILE *fd;
--
2.25.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PULL for-7.1 34/36] util/log: Rename QemuLogFile to RCUCloseFILE
2022-03-20 17:10 [PULL for-7.1 00/36] Logging cleanup and per-thread logfiles Richard Henderson
` (32 preceding siblings ...)
2022-03-20 17:11 ` [PULL for-7.1 33/36] util/log: Combine two logfile closes Richard Henderson
@ 2022-03-20 17:11 ` Richard Henderson
2022-03-20 21:58 ` Philippe Mathieu-Daudé
2022-03-20 17:11 ` [PULL for-7.1 35/36] util/log: Limit RCUCloseFILE to file closing Richard Henderson
` (2 subsequent siblings)
36 siblings, 1 reply; 60+ messages in thread
From: Richard Henderson @ 2022-03-20 17:11 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
s/QemuLogFile/RCUCloseFILE/
s/qemu_logfile_free/rcu_close_file/
Emphasize that this is only a carrier for passing a pointer
to call_rcu for closing, and not the real logfile.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
util/log.c | 28 +++++++++++++---------------
1 file changed, 13 insertions(+), 15 deletions(-)
diff --git a/util/log.c b/util/log.c
index 596de0050f..e966d1091d 100644
--- a/util/log.c
+++ b/util/log.c
@@ -29,15 +29,15 @@
#include "qemu/rcu.h"
-typedef struct QemuLogFile {
+typedef struct RCUCloseFILE {
struct rcu_head rcu;
FILE *fd;
-} QemuLogFile;
+} RCUCloseFILE;
/* Mutex covering the other global_* variables. */
static QemuMutex global_mutex;
static char *global_filename;
-static QemuLogFile *global_file;
+static RCUCloseFILE *global_file;
int qemu_loglevel;
static int log_append = 0;
@@ -52,7 +52,7 @@ bool qemu_log_enabled(void)
/* Returns true if qemu_log() will write somewhere other than stderr. */
bool qemu_log_separate(void)
{
- QemuLogFile *logfile;
+ RCUCloseFILE *logfile;
bool res = false;
rcu_read_lock();
@@ -68,7 +68,7 @@ bool qemu_log_separate(void)
FILE *qemu_log_lock(void)
{
- QemuLogFile *logfile;
+ RCUCloseFILE *logfile;
rcu_read_lock();
logfile = qatomic_rcu_read(&global_file);
@@ -117,14 +117,12 @@ static void __attribute__((__constructor__)) startup(void)
qemu_mutex_init(&global_mutex);
}
-static void qemu_logfile_free(QemuLogFile *logfile)
+static void rcu_close_file(RCUCloseFILE *r)
{
- g_assert(logfile);
-
- if (logfile->fd != stderr) {
- fclose(logfile->fd);
+ if (r->fd != stderr) {
+ fclose(r->fd);
}
- g_free(logfile);
+ g_free(r);
}
/* enable or disable low levels log */
@@ -133,7 +131,7 @@ static void qemu_set_log_internal(const char *filename, bool changed_name,
{
bool need_to_open_file;
bool daemonized;
- QemuLogFile *logfile;
+ RCUCloseFILE *logfile;
QEMU_LOCK_GUARD(&global_mutex);
logfile = global_file;
@@ -187,7 +185,7 @@ static void qemu_set_log_internal(const char *filename, bool changed_name,
if (logfile && (!need_to_open_file || changed_name)) {
qatomic_rcu_set(&global_file, NULL);
- call_rcu(logfile, qemu_logfile_free, rcu);
+ call_rcu(logfile, rcu_close_file, rcu);
logfile = NULL;
}
@@ -205,7 +203,7 @@ static void qemu_set_log_internal(const char *filename, bool changed_name,
if (daemonized) {
dup2(fileno(fd), STDERR_FILENO);
fclose(fd);
- /* This will skip closing logfile in qemu_logfile_free. */
+ /* This will skip closing logfile in rcu_close_file. */
fd = stderr;
}
} else {
@@ -216,7 +214,7 @@ static void qemu_set_log_internal(const char *filename, bool changed_name,
log_append = 1;
- logfile = g_new0(QemuLogFile, 1);
+ logfile = g_new0(RCUCloseFILE, 1);
logfile->fd = fd;
qatomic_rcu_set(&global_file, logfile);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* Re: [PULL for-7.1 34/36] util/log: Rename QemuLogFile to RCUCloseFILE
2022-03-20 17:11 ` [PULL for-7.1 34/36] util/log: Rename QemuLogFile to RCUCloseFILE Richard Henderson
@ 2022-03-20 21:58 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 60+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-20 21:58 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: alex.bennee
On 20/3/22 18:11, Richard Henderson wrote:
> s/QemuLogFile/RCUCloseFILE/
> s/qemu_logfile_free/rcu_close_file/
>
> Emphasize that this is only a carrier for passing a pointer
> to call_rcu for closing, and not the real logfile.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> util/log.c | 28 +++++++++++++---------------
> 1 file changed, 13 insertions(+), 15 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PULL for-7.1 35/36] util/log: Limit RCUCloseFILE to file closing
2022-03-20 17:10 [PULL for-7.1 00/36] Logging cleanup and per-thread logfiles Richard Henderson
` (33 preceding siblings ...)
2022-03-20 17:11 ` [PULL for-7.1 34/36] util/log: Rename QemuLogFile to RCUCloseFILE Richard Henderson
@ 2022-03-20 17:11 ` Richard Henderson
2022-03-20 17:11 ` [PULL for-7.1 36/36] util/log: Support per-thread log files Richard Henderson
2022-03-20 17:14 ` [PULL for-7.1 00/36] Logging cleanup and per-thread logfiles Richard Henderson
36 siblings, 0 replies; 60+ messages in thread
From: Richard Henderson @ 2022-03-20 17:11 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
Use FILE* for global_file. We can perform an rcu_read on that
just as easily as RCUCloseFILE*. This simplifies a couple of
places, where previously we required taking the rcu_read_lock
simply to avoid racing to dereference RCUCloseFile->fd.
Only allocate the RCUCloseFile prior to call_rcu.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
util/log.c | 67 +++++++++++++++++++++++-------------------------------
1 file changed, 28 insertions(+), 39 deletions(-)
diff --git a/util/log.c b/util/log.c
index e966d1091d..713a0744be 100644
--- a/util/log.c
+++ b/util/log.c
@@ -37,7 +37,7 @@ typedef struct RCUCloseFILE {
/* Mutex covering the other global_* variables. */
static QemuMutex global_mutex;
static char *global_filename;
-static RCUCloseFILE *global_file;
+static FILE *global_file;
int qemu_loglevel;
static int log_append = 0;
@@ -46,46 +46,37 @@ static GArray *debug_regions;
/* Returns true if qemu_log() will really write somewhere. */
bool qemu_log_enabled(void)
{
- return global_file != NULL;
+ return qatomic_read(&global_file) != NULL;
}
/* Returns true if qemu_log() will write somewhere other than stderr. */
bool qemu_log_separate(void)
{
- RCUCloseFILE *logfile;
- bool res = false;
-
- rcu_read_lock();
- logfile = qatomic_rcu_read(&global_file);
- if (logfile && logfile->fd != stderr) {
- res = true;
- }
- rcu_read_unlock();
- return res;
+ FILE *logfile = qatomic_read(&global_file);
+ return logfile && logfile != stderr;
}
/* Lock/unlock output. */
FILE *qemu_log_lock(void)
{
- RCUCloseFILE *logfile;
+ FILE *logfile;
rcu_read_lock();
logfile = qatomic_rcu_read(&global_file);
if (logfile) {
- qemu_flockfile(logfile->fd);
- return logfile->fd;
+ qemu_flockfile(logfile);
} else {
rcu_read_unlock();
- return NULL;
}
+ return logfile;
}
-void qemu_log_unlock(FILE *fd)
+void qemu_log_unlock(FILE *logfile)
{
- if (fd) {
- fflush(fd);
- qemu_funlockfile(fd);
+ if (logfile) {
+ fflush(logfile);
+ qemu_funlockfile(logfile);
rcu_read_unlock();
}
}
@@ -93,16 +84,16 @@ void qemu_log_unlock(FILE *fd)
/* Return the number of characters emitted. */
int qemu_log(const char *fmt, ...)
{
- FILE *f = qemu_log_lock();
+ FILE *logfile = qemu_log_lock();
int ret = 0;
- if (f) {
+ if (logfile) {
va_list ap;
va_start(ap, fmt);
- ret = vfprintf(f, fmt, ap);
+ ret = vfprintf(logfile, fmt, ap);
va_end(ap);
- qemu_log_unlock(f);
+ qemu_log_unlock(logfile);
/* Don't pass back error results. */
if (ret < 0) {
@@ -119,9 +110,7 @@ static void __attribute__((__constructor__)) startup(void)
static void rcu_close_file(RCUCloseFILE *r)
{
- if (r->fd != stderr) {
- fclose(r->fd);
- }
+ fclose(r->fd);
g_free(r);
}
@@ -131,7 +120,7 @@ static void qemu_set_log_internal(const char *filename, bool changed_name,
{
bool need_to_open_file;
bool daemonized;
- RCUCloseFILE *logfile;
+ FILE *logfile;
QEMU_LOCK_GUARD(&global_mutex);
logfile = global_file;
@@ -185,37 +174,37 @@ static void qemu_set_log_internal(const char *filename, bool changed_name,
if (logfile && (!need_to_open_file || changed_name)) {
qatomic_rcu_set(&global_file, NULL);
- call_rcu(logfile, rcu_close_file, rcu);
+ if (logfile != stderr) {
+ RCUCloseFILE *r = g_new0(RCUCloseFILE, 1);
+ r->fd = logfile;
+ call_rcu(r, rcu_close_file, rcu);
+ }
logfile = NULL;
}
if (!logfile && need_to_open_file) {
- FILE *fd;
-
if (filename) {
- fd = fopen(filename, log_append ? "a" : "w");
- if (!fd) {
+ logfile = fopen(filename, log_append ? "a" : "w");
+ if (!logfile) {
error_setg_errno(errp, errno, "Error opening logfile %s",
filename);
return;
}
/* In case we are a daemon redirect stderr to logfile */
if (daemonized) {
- dup2(fileno(fd), STDERR_FILENO);
- fclose(fd);
+ dup2(fileno(logfile), STDERR_FILENO);
+ fclose(logfile);
/* This will skip closing logfile in rcu_close_file. */
- fd = stderr;
+ logfile = stderr;
}
} else {
/* Default to stderr if no log file specified */
assert(!daemonized);
- fd = stderr;
+ logfile = stderr;
}
log_append = 1;
- logfile = g_new0(RCUCloseFILE, 1);
- logfile->fd = fd;
qatomic_rcu_set(&global_file, logfile);
}
}
--
2.25.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PULL for-7.1 36/36] util/log: Support per-thread log files
2022-03-20 17:10 [PULL for-7.1 00/36] Logging cleanup and per-thread logfiles Richard Henderson
` (34 preceding siblings ...)
2022-03-20 17:11 ` [PULL for-7.1 35/36] util/log: Limit RCUCloseFILE to file closing Richard Henderson
@ 2022-03-20 17:11 ` Richard Henderson
2022-03-20 17:14 ` [PULL for-7.1 00/36] Logging cleanup and per-thread logfiles Richard Henderson
36 siblings, 0 replies; 60+ messages in thread
From: Richard Henderson @ 2022-03-20 17:11 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
Add a new log flag, tid, to turn this feature on.
Require the log filename to be set, and to contain %d.
Do not allow tid to be turned off once it is on, nor let
the filename be change thereafter. This avoids the need
for signalling each thread to re-open on a name change.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/qemu/log.h | 1 +
util/log.c | 130 +++++++++++++++++++++++++++++++++++----------
2 files changed, 103 insertions(+), 28 deletions(-)
diff --git a/include/qemu/log.h b/include/qemu/log.h
index 783faec309..6455e47010 100644
--- a/include/qemu/log.h
+++ b/include/qemu/log.h
@@ -34,6 +34,7 @@ bool qemu_log_separate(void);
#define CPU_LOG_PLUGIN (1 << 18)
/* LOG_STRACE is used for user-mode strace logging. */
#define LOG_STRACE (1 << 19)
+#define LOG_PER_THREAD (1 << 20)
/* Lock/unlock output. */
diff --git a/util/log.c b/util/log.c
index 713a0744be..0da67c55de 100644
--- a/util/log.c
+++ b/util/log.c
@@ -38,22 +38,34 @@ typedef struct RCUCloseFILE {
static QemuMutex global_mutex;
static char *global_filename;
static FILE *global_file;
+static __thread FILE *thread_file;
int qemu_loglevel;
-static int log_append = 0;
+static bool log_append;
+static bool log_per_thread;
static GArray *debug_regions;
/* Returns true if qemu_log() will really write somewhere. */
bool qemu_log_enabled(void)
{
- return qatomic_read(&global_file) != NULL;
+ return log_per_thread || qatomic_read(&global_file) != NULL;
}
/* Returns true if qemu_log() will write somewhere other than stderr. */
bool qemu_log_separate(void)
{
FILE *logfile = qatomic_read(&global_file);
- return logfile && logfile != stderr;
+ return log_per_thread || (logfile && logfile != stderr);
+}
+
+static int log_thread_id(void)
+{
+#ifdef __linux__
+ return gettid(); /* ??? need syscall before glibc 2.30 */
+#else
+ static int counter;
+ return qatomic_fetch_inc(&counter);
+#endif
}
/* Lock/unlock output. */
@@ -62,13 +74,27 @@ FILE *qemu_log_lock(void)
{
FILE *logfile;
- rcu_read_lock();
- logfile = qatomic_rcu_read(&global_file);
- if (logfile) {
- qemu_flockfile(logfile);
- } else {
- rcu_read_unlock();
+ logfile = thread_file;
+ if (!logfile) {
+ if (log_per_thread) {
+ g_autofree char *filename
+ = g_strdup_printf(global_filename, log_thread_id());
+ logfile = fopen(filename, "w");
+ if (!logfile) {
+ return NULL;
+ }
+ thread_file = logfile;
+ } else {
+ rcu_read_lock();
+ logfile = qatomic_rcu_read(&global_file);
+ if (!logfile) {
+ rcu_read_unlock();
+ return NULL;
+ }
+ }
}
+
+ qemu_flockfile(logfile);
return logfile;
}
@@ -77,7 +103,9 @@ void qemu_log_unlock(FILE *logfile)
if (logfile) {
fflush(logfile);
qemu_funlockfile(logfile);
- rcu_read_unlock();
+ if (!log_per_thread) {
+ rcu_read_unlock();
+ }
}
}
@@ -114,40 +142,74 @@ static void rcu_close_file(RCUCloseFILE *r)
g_free(r);
}
+/**
+ * valid_filename_template:
+ *
+ * Validate the filename template. Require %d if per_thread, allow it
+ * otherwise; require no other % within the template.
+ * Return 0 if invalid, 1 if stderr, 2 if strdup, 3 if pid printf.
+ */
+static int valid_filename_template(const char *filename,
+ bool per_thread, Error **errp)
+{
+ if (filename) {
+ char *pidstr = strstr(filename, "%");
+
+ if (pidstr) {
+ /* We only accept one %d, no other format strings */
+ if (pidstr[1] != 'd' || strchr(pidstr + 2, '%')) {
+ error_setg(errp, "Bad logfile template: %s", filename);
+ return 0;
+ }
+ return per_thread ? 2 : 3;
+ }
+ }
+ if (per_thread) {
+ error_setg(errp, "Filename template with '%%d' required for 'tid'");
+ return 0;
+ }
+ return filename ? 2 : 1;
+}
+
/* enable or disable low levels log */
static void qemu_set_log_internal(const char *filename, bool changed_name,
int log_flags, Error **errp)
{
bool need_to_open_file;
bool daemonized;
+ bool per_thread;
FILE *logfile;
QEMU_LOCK_GUARD(&global_mutex);
logfile = global_file;
+ per_thread = log_flags & LOG_PER_THREAD;
+
if (changed_name) {
char *newname = NULL;
/*
- * Allow the user to include %d in their logfile which will be
- * substituted with the current PID. This is useful for debugging many
- * nested linux-user tasks but will result in lots of logs.
- *
- * filename may be NULL. In that case, log output is sent to stderr
+ * Once threads start opening their own log files, we have no
+ * easy mechanism to tell them all to close and re-open.
+ * There seems little cause to do so either -- this option
+ * will most often be used at user-only startup.
*/
- if (filename) {
- char *pidstr = strstr(filename, "%");
+ if (log_per_thread) {
+ error_setg(errp, "Cannot change log filename after setting 'tid'");
+ return;
+ }
- if (pidstr) {
- /* We only accept one %d, no other format strings */
- if (pidstr[1] != 'd' || strchr(pidstr + 2, '%')) {
- error_setg(errp, "Bad logfile format: %s", filename);
- return;
- }
- newname = g_strdup_printf(filename, getpid());
- } else {
- newname = g_strdup(filename);
- }
+ switch (valid_filename_template(filename, per_thread, errp)) {
+ case 0:
+ return; /* error */
+ case 1:
+ break; /* stderr */
+ case 2:
+ newname = g_strdup(filename);
+ break;
+ case 3:
+ newname = g_strdup_printf(filename, getpid());
+ break;
}
g_free(global_filename);
@@ -155,8 +217,17 @@ static void qemu_set_log_internal(const char *filename, bool changed_name,
filename = newname;
} else {
filename = global_filename;
+ if (per_thread && !valid_filename_template(filename, true, errp)) {
+ return; /* error */
+ }
}
+ /* Once the per-thread flag is set, it cannot be unset. */
+ if (per_thread) {
+ log_per_thread = true;
+ }
+ /* The flag itself is not relevant for need_to_open_file. */
+ log_flags &= ~LOG_PER_THREAD;
#ifdef CONFIG_TRACE_LOG
log_flags |= LOG_TRACE;
#endif
@@ -165,12 +236,13 @@ static void qemu_set_log_internal(const char *filename, bool changed_name,
/*
* In all cases we only log if qemu_loglevel is set.
* Also:
+ * If per-thread, open the file for each thread in qemu_log_lock.
* If not daemonized we will always log either to stderr
* or to a file (if there is a filename).
* If we are daemonized, we will only log if there is a filename.
*/
daemonized = is_daemonized();
- need_to_open_file = log_flags && (!daemonized || filename);
+ need_to_open_file = log_flags && !per_thread && (!daemonized || filename);
if (logfile && (!need_to_open_file || changed_name)) {
qatomic_rcu_set(&global_file, NULL);
@@ -355,6 +427,8 @@ const QEMULogItem qemu_log_items[] = {
#endif
{ LOG_STRACE, "strace",
"log every user-mode syscall, its input, and its result" },
+ { LOG_PER_THREAD, "tid",
+ "open a separate log file per thread; filename must contain '%d'" },
{ 0, NULL, NULL },
};
--
2.25.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* Re: [PULL for-7.1 00/36] Logging cleanup and per-thread logfiles
2022-03-20 17:10 [PULL for-7.1 00/36] Logging cleanup and per-thread logfiles Richard Henderson
` (35 preceding siblings ...)
2022-03-20 17:11 ` [PULL for-7.1 36/36] util/log: Support per-thread log files Richard Henderson
@ 2022-03-20 17:14 ` Richard Henderson
36 siblings, 0 replies; 60+ messages in thread
From: Richard Henderson @ 2022-03-20 17:14 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
On 3/20/22 10:10, Richard Henderson wrote:
> Most of the changes here reduce the amount of locking involved
> in logging, due to repeated qemu_log calls, each of which takes
> and releases the rcu_read_lock.
>
> This makes more use of qemu_log_lock/unlock around code blocks,
> which both keeps the output together in the face of threads and
> also plays the rcu_read_lock game only once for the block.
>
> Finally, add a -d tid option to open per-thread logfiles.
> This can be extremely helpful in debugging user-only threads.
Gah. I didn't mean to write 'PULL' here, just PATCH.
r~
^ permalink raw reply [flat|nested] 60+ messages in thread