diff mbox

[16/22] tcg: keep a list of TCGContext's

Message ID 1499586614-20507-17-git-send-email-cota@braap.org (mailing list archive)
State New, archived
Headers show

Commit Message

Emilio Cota July 9, 2017, 7:50 a.m. UTC
Before we make TCGContext thread-local. Once that is done, iterating
over all TCG contexts will be quite useful; for instance we
will need it to gather profiling info from each TCGContext.

A possible alternative would be to keep an array of TCGContext pointers.
However this option however is not that trivial, because vCPUs are spawned in
parallel. So let's just keep it simple and use a list protected by a lock.

Note that this lock will soon be used for other purposes, hence the
generic "tcg_lock" name.

Signed-off-by: Emilio G. Cota <cota@braap.org>
---
 tcg/tcg.h |  3 +++
 tcg/tcg.c | 23 +++++++++++++++++++++++
 2 files changed, 26 insertions(+)

Comments

Richard Henderson July 9, 2017, 8:43 p.m. UTC | #1
On 07/08/2017 09:50 PM, Emilio G. Cota wrote:
> Before we make TCGContext thread-local. Once that is done, iterating
> over all TCG contexts will be quite useful; for instance we
> will need it to gather profiling info from each TCGContext.
> 
> A possible alternative would be to keep an array of TCGContext pointers.
> However this option however is not that trivial, because vCPUs are spawned in
> parallel. So let's just keep it simple and use a list protected by a lock.

Do we not know the number of cpus?  I would have thought that we did, via -smp. 
  Even cpu-hotplug would still have some sort of bounds?

Which really suggests an array would be better.


r~
Alex Bennée July 12, 2017, 3:32 p.m. UTC | #2
Emilio G. Cota <cota@braap.org> writes:

> Before we make TCGContext thread-local.

Really? Not allocated for each thread that needs it?

> Once that is done, iterating
> over all TCG contexts will be quite useful; for instance we
> will need it to gather profiling info from each TCGContext.

How often will we need to do this and how performance sensitive will it be?

>
> A possible alternative would be to keep an array of TCGContext pointers.
> However this option however is not that trivial, because vCPUs are spawned in
> parallel. So let's just keep it simple and use a list protected by a
> lock.

Given we are protecting with a lock I don't see why it wouldn't be as
equally trivial.

>
> Note that this lock will soon be used for other purposes, hence the
> generic "tcg_lock" name.
>
> Signed-off-by: Emilio G. Cota <cota@braap.org>
> ---
>  tcg/tcg.h |  3 +++
>  tcg/tcg.c | 23 +++++++++++++++++++++++
>  2 files changed, 26 insertions(+)
>
> diff --git a/tcg/tcg.h b/tcg/tcg.h
> index 534ead5..8e1cd45 100644
> --- a/tcg/tcg.h
> +++ b/tcg/tcg.h
> @@ -725,6 +725,8 @@ struct TCGContext {
>
>      uint16_t gen_insn_end_off[TCG_MAX_INSNS];
>      target_ulong gen_insn_data[TCG_MAX_INSNS][TARGET_INSN_START_WORDS];
> +
> +    QSIMPLEQ_ENTRY(TCGContext) entry;
>  };
>
>  extern TCGContext tcg_ctx;
> @@ -773,6 +775,7 @@ static inline void *tcg_malloc(int size)
>
>  void tcg_context_init(TCGContext *s);
>  void tcg_prologue_init(TCGContext *s);
> +void tcg_register_thread(void);
>  void tcg_func_start(TCGContext *s);
>
>  int tcg_gen_code(TCGContext *s, TranslationBlock *tb);
> diff --git a/tcg/tcg.c b/tcg/tcg.c
> index d9b083a..0da7c61 100644
> --- a/tcg/tcg.c
> +++ b/tcg/tcg.c
> @@ -115,7 +115,16 @@ static int tcg_target_const_match(tcg_target_long val, TCGType type,
>  static void tcg_out_tb_init(TCGContext *s);
>  static bool tcg_out_tb_finalize(TCGContext *s);
>
> +static QemuMutex tcg_lock;
>
> +/*
> + * List of TCGContext's in the system. Protected by tcg_lock.
> + * Once vcpu threads have been inited, there will be no further modifications
> + * to the list (vcpu threads never return) so we can safely traverse the list
> + * without synchronization.
> + */
> +static QSIMPLEQ_HEAD(, TCGContext) ctx_list =
> +    QSIMPLEQ_HEAD_INITIALIZER(ctx_list);
>
>  static TCGRegSet tcg_target_available_regs[2];
>  static TCGRegSet tcg_target_call_clobber_regs;
> @@ -324,6 +333,17 @@ static GHashTable *helper_table;
>  static int indirect_reg_alloc_order[ARRAY_SIZE(tcg_target_reg_alloc_order)];
>  static void process_op_defs(TCGContext *s);
>
> +/*
> + * Child TCG threads, i.e. the ones that do not call tcg_context_init, must call
> + * this function before initiating translation.
> + */
> +void tcg_register_thread(void)
> +{
> +    qemu_mutex_lock(&tcg_lock);
> +    QSIMPLEQ_INSERT_TAIL(&ctx_list, &tcg_ctx, entry);
> +    qemu_mutex_unlock(&tcg_lock);
> +}
> +
>  void tcg_context_init(TCGContext *s)
>  {
>      int op, total_args, n, i;
> @@ -381,6 +401,9 @@ void tcg_context_init(TCGContext *s)
>      for (; i < ARRAY_SIZE(tcg_target_reg_alloc_order); ++i) {
>          indirect_reg_alloc_order[i] = tcg_target_reg_alloc_order[i];
>      }
> +
> +    qemu_mutex_init(&tcg_lock);
> +    tcg_register_thread();
>  }
>
>  /*


--
Alex Bennée
diff mbox

Patch

diff --git a/tcg/tcg.h b/tcg/tcg.h
index 534ead5..8e1cd45 100644
--- a/tcg/tcg.h
+++ b/tcg/tcg.h
@@ -725,6 +725,8 @@  struct TCGContext {
 
     uint16_t gen_insn_end_off[TCG_MAX_INSNS];
     target_ulong gen_insn_data[TCG_MAX_INSNS][TARGET_INSN_START_WORDS];
+
+    QSIMPLEQ_ENTRY(TCGContext) entry;
 };
 
 extern TCGContext tcg_ctx;
@@ -773,6 +775,7 @@  static inline void *tcg_malloc(int size)
 
 void tcg_context_init(TCGContext *s);
 void tcg_prologue_init(TCGContext *s);
+void tcg_register_thread(void);
 void tcg_func_start(TCGContext *s);
 
 int tcg_gen_code(TCGContext *s, TranslationBlock *tb);
diff --git a/tcg/tcg.c b/tcg/tcg.c
index d9b083a..0da7c61 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -115,7 +115,16 @@  static int tcg_target_const_match(tcg_target_long val, TCGType type,
 static void tcg_out_tb_init(TCGContext *s);
 static bool tcg_out_tb_finalize(TCGContext *s);
 
+static QemuMutex tcg_lock;
 
+/*
+ * List of TCGContext's in the system. Protected by tcg_lock.
+ * Once vcpu threads have been inited, there will be no further modifications
+ * to the list (vcpu threads never return) so we can safely traverse the list
+ * without synchronization.
+ */
+static QSIMPLEQ_HEAD(, TCGContext) ctx_list =
+    QSIMPLEQ_HEAD_INITIALIZER(ctx_list);
 
 static TCGRegSet tcg_target_available_regs[2];
 static TCGRegSet tcg_target_call_clobber_regs;
@@ -324,6 +333,17 @@  static GHashTable *helper_table;
 static int indirect_reg_alloc_order[ARRAY_SIZE(tcg_target_reg_alloc_order)];
 static void process_op_defs(TCGContext *s);
 
+/*
+ * Child TCG threads, i.e. the ones that do not call tcg_context_init, must call
+ * this function before initiating translation.
+ */
+void tcg_register_thread(void)
+{
+    qemu_mutex_lock(&tcg_lock);
+    QSIMPLEQ_INSERT_TAIL(&ctx_list, &tcg_ctx, entry);
+    qemu_mutex_unlock(&tcg_lock);
+}
+
 void tcg_context_init(TCGContext *s)
 {
     int op, total_args, n, i;
@@ -381,6 +401,9 @@  void tcg_context_init(TCGContext *s)
     for (; i < ARRAY_SIZE(tcg_target_reg_alloc_order); ++i) {
         indirect_reg_alloc_order[i] = tcg_target_reg_alloc_order[i];
     }
+
+    qemu_mutex_init(&tcg_lock);
+    tcg_register_thread();
 }
 
 /*