qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 3/4] qemu_log_lock/unlock now preserves the qemu_logfile hand


From: Alex Bennée
Subject: Re: [PATCH 3/4] qemu_log_lock/unlock now preserves the qemu_logfile handle.
Date: Thu, 07 Nov 2019 16:25:22 +0000
User-agent: mu4e 1.3.5; emacs 27.0.50

Robert Foley <address@hidden> 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 <address@hidden>
> ---
>  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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]