| Message ID | 1439397664-70734-7-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 12 August 2015 at 17:40, Paolo Bonzini <pbonzini@redhat.com> wrote: > More #ifdefs are not nice, but this clarifies why its usage is not > protected by tb_lock. Does it? I thought the idea of this series was to add locking which we needed for adding multi-threading to softmmu, in which case presumably we need to protect this with a lock of some kind whether we're using softmmu or usermode. thanks -- PMM
On 28/08/2015 16:57, Peter Maydell wrote: >> > More #ifdefs are not nice, but this clarifies why its usage is not >> > protected by tb_lock. > Does it? I thought the idea of this series was to add locking > which we needed for adding multi-threading to softmmu, in > which case presumably we need to protect this with a lock > of some kind whether we're using softmmu or usermode. No, the idea of the series was to base softmmu multi-threading on user-mode multi-threading, and document what else needs to be done with respect to common code. Adding the necessary locks for softmmu only comes in the "extra" patches 11 and 12, and protecting code_bitmap with a lock (presumably tb_lock, or its own lock, I don't know) would belong there. However I left this job to Fred, and similarly for breakpoints. I'm not sure if it makes sense to add locking (like patches 11 and 12) in the absence of actual multi-threading. On one hand it would make it simpler to commit the MTTCG work in steps. On the other hand it risks bitrotting. The possibility of bitrot is another point in favor of making more code #ifdef CONFIG_SOFTMMU; it would clearly identify areas where locking correctness never matters in practice, and thus bugs may be hidden. Paolo
diff --git a/translate-all.c b/translate-all.c index a6bff72..7aa5664 100644 --- a/translate-all.c +++ b/translate-all.c @@ -765,11 +765,15 @@ void tb_free(TranslationBlock *tb) static inline void invalidate_page_bitmap(PageDesc *p) { +#ifdef CONFIG_SOFTMMU if (p->code_bitmap) { g_free(p->code_bitmap); p->code_bitmap = NULL; } p->code_write_count = 0; +#else + assert(p->code_bitmap == NULL); +#endif } /* Set to NULL all the 'first_tb' fields in all PageDescs. */ @@ -1001,6 +1005,7 @@ void tb_phys_invalidate(TranslationBlock *tb, tb_page_addr_t page_addr) tcg_ctx.tb_ctx.tb_phys_invalidate_count++; } +#ifdef CONFIG_SOFTMMU static void build_page_bitmap(PageDesc *p) { int n, tb_start, tb_end; @@ -1029,6 +1034,7 @@ static void build_page_bitmap(PageDesc *p) tb = tb->page_next[n]; } } +#endif TranslationBlock *tb_gen_code(CPUState *cpu, target_ulong pc, target_ulong cs_base, @@ -1202,6 +1208,7 @@ void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end, #endif } +#ifdef CONFIG_SOFTMMU /* len must be <= 8 and start must be a multiple of len */ void tb_invalidate_phys_page_fast(tb_page_addr_t start, int len) { @@ -1239,8 +1246,7 @@ void tb_invalidate_phys_page_fast(tb_page_addr_t start, int len) tb_invalidate_phys_page_range(start, start + len, 1); } } - -#if !defined(CONFIG_SOFTMMU) +#else static void tb_invalidate_phys_page(tb_page_addr_t addr, uintptr_t pc, void *puc, bool locked)
More #ifdefs are not nice, but this clarifies why its usage is not protected by tb_lock. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- translate-all.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)