From: "Alex Bennée" <alex.bennee@linaro.org>
To: Robert Foley <robert.foley@linaro.org>
Cc: peter.puhov@linaro.org, qemu-devel@nongnu.org
Subject: Re: [PATCH 3/4] qemu_log_lock/unlock now preserves the qemu_logfile handle.
Date: Thu, 07 Nov 2019 16:25:22 +0000 [thread overview]
Message-ID: <87wocbhcwd.fsf@linaro.org> (raw)
In-Reply-To: <20191107142613.2379-4-robert.foley@linaro.org>
Robert Foley <robert.foley@linaro.org> writes:
> qemu_log_lock() now returns a handle and qemu_log_unlock() receives a
> handle to unlock. This allows for changing the handle during logging
> and ensures the lock() and unlock() are for the same file.
Ahh there it is!
We probably want to put the API change through before we add the RCU
support - so swap the patch order around.
>
> Signed-off-by: Robert Foley <robert.foley@linaro.org>
> ---
> include/qemu/log.h | 14 +++++++-------
> accel/tcg/cpu-exec.c | 4 ++--
> accel/tcg/translate-all.c | 4 ++--
> accel/tcg/translator.c | 4 ++--
> exec.c | 4 ++--
> hw/net/can/can_sja1000.c | 4 ++--
> net/can/can_socketcan.c | 5 ++---
> target/cris/translate.c | 4 ++--
> target/i386/translate.c | 5 +++--
> target/lm32/translate.c | 4 ++--
> target/microblaze/translate.c | 4 ++--
> target/nios2/translate.c | 4 ++--
> target/tilegx/translate.c | 7 ++++---
> target/unicore32/translate.c | 4 ++--
> tcg/tcg.c | 16 ++++++++--------
> 15 files changed, 44 insertions(+), 43 deletions(-)
>
> diff --git a/include/qemu/log.h b/include/qemu/log.h
> index 975de18e23..3d0f47a479 100644
> --- a/include/qemu/log.h
> +++ b/include/qemu/log.h
> @@ -71,25 +71,25 @@ static inline bool qemu_log_separate(void)
> * qemu_loglevel is never set when qemu_logfile is unset.
> */
>
> -static inline void qemu_log_lock(void)
> +static inline FILE *qemu_log_lock(void)
> {
> QemuLogFile *logfile;
> rcu_read_lock();
> logfile = atomic_rcu_read(&qemu_logfile);
> if (logfile) {
> qemu_flockfile(logfile->fd);
> + return logfile->fd;
> }
> rcu_read_unlock();
> + return NULL;
> }
>
> -static inline void qemu_log_unlock(void)
> +static inline void qemu_log_unlock(FILE *fd)
> {
> - QemuLogFile *logfile;
> - logfile = atomic_rcu_read(&qemu_logfile);
> - if (logfile) {
> - qemu_funlockfile(logfile->fd);
> + if (fd) {
> + qemu_funlockfile(fd);
> + rcu_read_unlock();
> }
> - rcu_read_unlock();
> }
>
> /* Logging functions: */
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index c01f59c743..62068d10c3 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -156,7 +156,7 @@ static inline tcg_target_ulong cpu_tb_exec(CPUState *cpu, TranslationBlock *itb)
> #if defined(DEBUG_DISAS)
> if (qemu_loglevel_mask(CPU_LOG_TB_CPU)
> && qemu_log_in_addr_range(itb->pc)) {
> - qemu_log_lock();
> + FILE *logfile = qemu_log_lock();
> int flags = 0;
> if (qemu_loglevel_mask(CPU_LOG_TB_FPU)) {
> flags |= CPU_DUMP_FPU;
> @@ -165,7 +165,7 @@ static inline tcg_target_ulong cpu_tb_exec(CPUState *cpu, TranslationBlock *itb)
> flags |= CPU_DUMP_CCOP;
> #endif
> log_cpu_state(cpu, flags);
> - qemu_log_unlock();
> + qemu_log_unlock(logfile);
> }
> #endif /* DEBUG_DISAS */
>
> diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
> index 9f48da9472..bb325a2bc4 100644
> --- a/accel/tcg/translate-all.c
> +++ b/accel/tcg/translate-all.c
> @@ -1804,7 +1804,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
> #ifdef DEBUG_DISAS
> if (qemu_loglevel_mask(CPU_LOG_TB_OUT_ASM) &&
> qemu_log_in_addr_range(tb->pc)) {
> - qemu_log_lock();
> + FILE *logfile = qemu_log_lock();
> qemu_log("OUT: [size=%d]\n", gen_code_size);
> if (tcg_ctx->data_gen_ptr) {
> size_t code_size = tcg_ctx->data_gen_ptr - tb->tc.ptr;
> @@ -1829,7 +1829,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
> }
> qemu_log("\n");
> qemu_log_flush();
> - qemu_log_unlock();
> + qemu_log_unlock(logfile);
> }
> #endif
>
> diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
> index f977682be7..603d17ff83 100644
> --- a/accel/tcg/translator.c
> +++ b/accel/tcg/translator.c
> @@ -138,11 +138,11 @@ void translator_loop(const TranslatorOps *ops, DisasContextBase *db,
> #ifdef DEBUG_DISAS
> if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)
> && qemu_log_in_addr_range(db->pc_first)) {
> - qemu_log_lock();
> + FILE *logfile = qemu_log_lock();
> qemu_log("----------------\n");
> ops->disas_log(db, cpu);
> qemu_log("\n");
> - qemu_log_unlock();
> + qemu_log_unlock(logfile);
> }
> #endif
> }
> diff --git a/exec.c b/exec.c
> index ffdb518535..c994a00f10 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1223,13 +1223,13 @@ void cpu_abort(CPUState *cpu, const char *fmt, ...)
> fprintf(stderr, "\n");
> cpu_dump_state(cpu, stderr, CPU_DUMP_FPU | CPU_DUMP_CCOP);
> if (qemu_log_separate()) {
> - qemu_log_lock();
> + 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);
> qemu_log_flush();
> - qemu_log_unlock();
> + qemu_log_unlock(logfile);
> qemu_log_close();
> }
> va_end(ap2);
> diff --git a/hw/net/can/can_sja1000.c b/hw/net/can/can_sja1000.c
> index 1f81341554..39c78faf9b 100644
> --- a/hw/net/can/can_sja1000.c
> +++ b/hw/net/can/can_sja1000.c
> @@ -247,8 +247,8 @@ 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_lock();
> qemu_log("%s%03X [%01d] %s %s",
> prefix,
> msg->can_id & QEMU_CAN_EFF_MASK,
> @@ -261,7 +261,7 @@ static void can_display_msg(const char *prefix, const qemu_can_frame *msg)
> }
> qemu_log("\n");
> qemu_log_flush();
> - qemu_log_unlock();
> + 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 8a6ffad40c..29bfacd4f8 100644
> --- a/net/can/can_socketcan.c
> +++ b/net/can/can_socketcan.c
> @@ -76,8 +76,7 @@ QEMU_BUILD_BUG_ON(offsetof(qemu_can_frame, data)
> static void can_host_socketcan_display_msg(struct qemu_can_frame *msg)
> {
> int i;
> -
> - qemu_log_lock();
> + FILE *logfile = qemu_log_lock();
> qemu_log("[cansocketcan]: %03X [%01d] %s %s",
> msg->can_id & QEMU_CAN_EFF_MASK,
> msg->can_dlc,
> @@ -89,7 +88,7 @@ static void can_host_socketcan_display_msg(struct qemu_can_frame *msg)
> }
> qemu_log("\n");
> qemu_log_flush();
> - qemu_log_unlock();
> + qemu_log_unlock(logfile);
> }
>
> static void can_host_socketcan_read(void *opaque)
> diff --git a/target/cris/translate.c b/target/cris/translate.c
> index e752bd0609..cb57516a44 100644
> --- a/target/cris/translate.c
> +++ b/target/cris/translate.c
> @@ -3273,11 +3273,11 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
> #if !DISAS_CRIS
> if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)
> && qemu_log_in_addr_range(pc_start)) {
> - qemu_log_lock();
> + FILE *logfile = qemu_log_lock();
> qemu_log("--------------\n");
> qemu_log("IN: %s\n", lookup_symbol(pc_start));
> log_target_disas(cs, pc_start, dc->pc - pc_start);
> - qemu_log_unlock();
> + qemu_log_unlock(logfile);
> }
> #endif
> #endif
> diff --git a/target/i386/translate.c b/target/i386/translate.c
> index 77e932d827..7c99ef1385 100644
> --- a/target/i386/translate.c
> +++ b/target/i386/translate.c
> @@ -2502,14 +2502,15 @@ static void gen_unknown_opcode(CPUX86State *env, DisasContext *s)
> gen_illegal_opcode(s);
>
> if (qemu_loglevel_mask(LOG_UNIMP)) {
> + FILE *logfile = qemu_log_lock();
> target_ulong pc = s->pc_start, end = s->pc;
> - qemu_log_lock();
> +
> qemu_log("ILLOPC: " TARGET_FMT_lx ":", pc);
> for (; pc < end; ++pc) {
> qemu_log(" %02x", cpu_ldub_code(env, pc));
> }
> qemu_log("\n");
> - qemu_log_unlock();
> + qemu_log_unlock(logfile);
> }
> }
>
> diff --git a/target/lm32/translate.c b/target/lm32/translate.c
> index 778cae1e81..73db9654d6 100644
> --- a/target/lm32/translate.c
> +++ b/target/lm32/translate.c
> @@ -1137,10 +1137,10 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
> #ifdef DEBUG_DISAS
> if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)
> && qemu_log_in_addr_range(pc_start)) {
> - qemu_log_lock();
> + FILE *logfile = qemu_log_lock();
> qemu_log("\n");
> log_target_disas(cs, pc_start, dc->pc - pc_start);
> - qemu_log_unlock();
> + qemu_log_unlock(logfile);
> }
> #endif
> }
> diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
> index 761f535357..cc1ad15656 100644
> --- a/target/microblaze/translate.c
> +++ b/target/microblaze/translate.c
> @@ -1766,10 +1766,10 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
> #if !SIM_COMPAT
> if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)
> && qemu_log_in_addr_range(pc_start)) {
> - qemu_log_lock();
> + FILE *logfile = qemu_log_lock();
> qemu_log("--------------\n");
> log_target_disas(cs, pc_start, dc->pc - pc_start);
> - qemu_log_unlock();
> + qemu_log_unlock(logfile);
> }
> #endif
> #endif
> diff --git a/target/nios2/translate.c b/target/nios2/translate.c
> index e17656e66f..82107bf270 100644
> --- a/target/nios2/translate.c
> +++ b/target/nios2/translate.c
> @@ -892,11 +892,11 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
> #ifdef DEBUG_DISAS
> if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)
> && qemu_log_in_addr_range(tb->pc)) {
> - qemu_log_lock();
> + FILE *logfile = qemu_log_lock();
> qemu_log("IN: %s\n", lookup_symbol(tb->pc));
> log_target_disas(cs, tb->pc, dc->pc - tb->pc);
> qemu_log("\n");
> - qemu_log_unlock();
> + qemu_log_unlock(logfile);
> }
> #endif
> }
> diff --git a/target/tilegx/translate.c b/target/tilegx/translate.c
> index 68dd4aa2d8..fd406f4f71 100644
> --- a/target/tilegx/translate.c
> +++ b/target/tilegx/translate.c
> @@ -2377,6 +2377,7 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
> uint64_t pc_start = tb->pc;
> uint64_t page_start = pc_start & TARGET_PAGE_MASK;
> int num_insns = 0;
> + FILE *logfile = NULL;
>
> dc->pc = pc_start;
> dc->mmuidx = 0;
> @@ -2388,7 +2389,7 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
> dc->zero = NULL;
>
> if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
> - qemu_log_lock();
> + logfile = qemu_log_lock();
> qemu_log("IN: %s\n", lookup_symbol(pc_start));
> }
> gen_tb_start(tb);
> @@ -2418,9 +2419,9 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
> tb->size = dc->pc - pc_start;
> tb->icount = num_insns;
>
> - if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
> + if (logfile) {
> qemu_log("\n");
> - qemu_log_unlock();
> + qemu_log_unlock(logfile);
> }
> }
>
> diff --git a/target/unicore32/translate.c b/target/unicore32/translate.c
> index 0e01f35856..0f6891b8aa 100644
> --- a/target/unicore32/translate.c
> +++ b/target/unicore32/translate.c
> @@ -1994,12 +1994,12 @@ done_generating:
> #ifdef DEBUG_DISAS
> if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)
> && qemu_log_in_addr_range(pc_start)) {
> - qemu_log_lock();
> + FILE *logfile = qemu_log_lock();
> qemu_log("----------------\n");
> qemu_log("IN: %s\n", lookup_symbol(pc_start));
> log_target_disas(cs, pc_start, dc->pc - pc_start);
> qemu_log("\n");
> - qemu_log_unlock();
> + qemu_log_unlock(logfile);
> }
> #endif
> tb->size = dc->pc - pc_start;
> diff --git a/tcg/tcg.c b/tcg/tcg.c
> index 220eaac7c7..4f616ba38b 100644
> --- a/tcg/tcg.c
> +++ b/tcg/tcg.c
> @@ -1085,7 +1085,7 @@ void tcg_prologue_init(TCGContext *s)
>
> #ifdef DEBUG_DISAS
> if (qemu_loglevel_mask(CPU_LOG_TB_OUT_ASM)) {
> - qemu_log_lock();
> + 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 - buf0;
> @@ -1110,7 +1110,7 @@ void tcg_prologue_init(TCGContext *s)
> }
> qemu_log("\n");
> qemu_log_flush();
> - qemu_log_unlock();
> + qemu_log_unlock(logfile);
> }
> #endif
>
> @@ -4049,11 +4049,11 @@ int tcg_gen_code(TCGContext *s, TranslationBlock *tb)
> #ifdef DEBUG_DISAS
> if (unlikely(qemu_loglevel_mask(CPU_LOG_TB_OP)
> && qemu_log_in_addr_range(tb->pc))) {
> - qemu_log_lock();
> + FILE *logfile = qemu_log_lock();
> qemu_log("OP:\n");
> tcg_dump_ops(s, false);
> qemu_log("\n");
> - qemu_log_unlock();
> + qemu_log_unlock(logfile);
> }
> #endif
>
> @@ -4094,11 +4094,11 @@ int tcg_gen_code(TCGContext *s, TranslationBlock *tb)
> #ifdef DEBUG_DISAS
> if (unlikely(qemu_loglevel_mask(CPU_LOG_TB_OP_IND)
> && qemu_log_in_addr_range(tb->pc))) {
> - qemu_log_lock();
> + FILE *logfile = qemu_log_lock();
> qemu_log("OP before indirect lowering:\n");
> tcg_dump_ops(s, false);
> qemu_log("\n");
> - qemu_log_unlock();
> + qemu_log_unlock(logfile);
> }
> #endif
> /* Replace indirect temps with direct temps. */
> @@ -4115,11 +4115,11 @@ int tcg_gen_code(TCGContext *s, TranslationBlock *tb)
> #ifdef DEBUG_DISAS
> if (unlikely(qemu_loglevel_mask(CPU_LOG_TB_OP_OPT)
> && qemu_log_in_addr_range(tb->pc))) {
> - qemu_log_lock();
> + 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();
> + qemu_log_unlock(logfile);
> }
> #endif
--
Alex Bennée
next prev parent reply other threads:[~2019-11-07 16:26 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-07 14:26 [PATCH 0/4] Make the qemu_logfile handle thread safe Robert Foley
2019-11-07 14:26 ` [PATCH 1/4] Add a mutex to guarantee single writer to qemu_logfile handle Robert Foley
2019-11-07 16:53 ` Alex Bennée
2019-11-07 21:54 ` Robert Foley
2019-11-07 14:26 ` [PATCH 2/4] Add use of RCU for qemu_logfile Robert Foley
2019-11-07 16:23 ` Alex Bennée
2019-11-07 21:13 ` Robert Foley
2019-11-07 14:26 ` [PATCH 3/4] qemu_log_lock/unlock now preserves the qemu_logfile handle Robert Foley
2019-11-07 16:25 ` Alex Bennée [this message]
2019-11-07 21:22 ` Robert Foley
2019-11-07 14:26 ` [PATCH 4/4] Added tests for close and change of logfile Robert Foley
2019-11-07 16:32 ` Alex Bennée
2019-11-07 17:26 ` Alex Bennée
2019-11-07 19:38 ` Robert Foley
2019-11-07 20:12 ` Alex Bennée
2019-11-07 22:11 ` Robert Foley
2019-11-07 21:33 ` Robert Foley
2019-11-07 14:46 ` [PATCH 0/4] Make the qemu_logfile handle thread safe no-reply
2019-11-07 16:35 ` Alex Bennée
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87wocbhcwd.fsf@linaro.org \
--to=alex.bennee@linaro.org \
--cc=peter.puhov@linaro.org \
--cc=qemu-devel@nongnu.org \
--cc=robert.foley@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.