Re: [Qemu-devel] [RFC PATCH V7 09/19] Drop global lock during TCG code execution

[Frederic Konrad]

On 10/08/2015 18:15, Paolo Bonzini wrote:

On 10/08/2015 17:27, fred.kon...@greensocs.com wrote:

  void qemu_mutex_lock_iothread(void)
  {
-    atomic_inc(&iothread_requesting_mutex);
-    /* In the simple case there is no need to bump the VCPU thread out of
-     * TCG code execution.
-     */
-    if (!tcg_enabled() || qemu_in_vcpu_thread() ||
-        !first_cpu || !first_cpu->thread) {
-        qemu_mutex_lock(&qemu_global_mutex);
-        atomic_dec(&iothread_requesting_mutex);
-    } else {
-        if (qemu_mutex_trylock(&qemu_global_mutex)) {
-            qemu_cpu_kick_thread(first_cpu);
-            qemu_mutex_lock(&qemu_global_mutex);
-        }
-        atomic_dec(&iothread_requesting_mutex);
-        qemu_cond_broadcast(&qemu_io_proceeded_cond);
-    }
-    iothread_locked = true;

"iothread_locked = true" must be kept.  Otherwise... yay! :)

oops :).

@@ -125,8 +128,10 @@ void tlb_flush_page(CPUState *cpu, target_ulong addr)
     can be detected */
  void tlb_protect_code(ram_addr_t ram_addr)
  {
+    qemu_mutex_lock_iothread();
      cpu_physical_memory_test_and_clear_dirty(ram_addr, TARGET_PAGE_SIZE,
                                               DIRTY_MEMORY_CODE);
+    qemu_mutex_unlock_iothread();
  }

Not needed anymore.

diff --git a/target-i386/misc_helper.c b/target-i386/misc_helper.c
index 52c5d65..55f63bf 100644
--- a/target-i386/misc_helper.c
+++ b/target-i386/misc_helper.c

None of this is needed anymore either! :)

+    /*
+     * Some device's reset needs to grab the global_mutex. So just release it
+     * here.
+     */
+    qemu_mutex_unlock_iothread();
      /* reset all devices */
      QTAILQ_FOREACH_SAFE(re, &reset_handlers, entry, nre) {
          re->func(re->opaque);
      }
+    qemu_mutex_lock_iothread();

Should never have been true?  (And, I think, it was pointed out in a
previous version too).

I had a double lock with the reset handler from vexpress-a15. I don't really
remember why. But I hacked that. It's fixed now :)

Thanks,
Fred

Paolo