Skip to content
Snippets Groups Projects
  • Roman Kagan's avatar
    e533f45d
    cpus-common: nuke finish_safe_work · e533f45d
    Roman Kagan authored
    
    It was introduced in commit ab129972,
    with the following motivation:
    
      Because start_exclusive uses CPU_FOREACH, merge exclusive_lock with
      qemu_cpu_list_lock: together with a call to exclusive_idle (via
      cpu_exec_start/end) in cpu_list_add, this protects exclusive work
      against concurrent CPU addition and removal.
    
    However, it seems to be redundant, because the cpu-exclusive
    infrastructure provides suffificent protection against the newly added
    CPU starting execution while the cpu-exclusive work is running, and the
    aforementioned traversing of the cpu list is protected by
    qemu_cpu_list_lock.
    
    Besides, this appears to be the only place where the cpu-exclusive
    section is entered with the BQL taken, which has been found to trigger
    AB-BA deadlock as follows:
    
        vCPU thread                             main thread
        -----------                             -----------
    async_safe_run_on_cpu(self,
                          async_synic_update)
    ...                                         [cpu hot-add]
    process_queued_cpu_work()
      qemu_mutex_unlock_iothread()
                                                [grab BQL]
      start_exclusive()                         cpu_list_add()
      async_synic_update()                        finish_safe_work()
        qemu_mutex_lock_iothread()                  cpu_exec_start()
    
    So remove it.  This paves the way to establishing a strict nesting rule
    of never entering the exclusive section with the BQL taken.
    
    Signed-off-by: default avatarRoman Kagan <rkagan@virtuozzo.com>
    Message-Id: <20190523105440.27045-2-rkagan@virtuozzo.com>
    e533f45d
    History
    cpus-common: nuke finish_safe_work
    Roman Kagan authored
    
    It was introduced in commit ab129972,
    with the following motivation:
    
      Because start_exclusive uses CPU_FOREACH, merge exclusive_lock with
      qemu_cpu_list_lock: together with a call to exclusive_idle (via
      cpu_exec_start/end) in cpu_list_add, this protects exclusive work
      against concurrent CPU addition and removal.
    
    However, it seems to be redundant, because the cpu-exclusive
    infrastructure provides suffificent protection against the newly added
    CPU starting execution while the cpu-exclusive work is running, and the
    aforementioned traversing of the cpu list is protected by
    qemu_cpu_list_lock.
    
    Besides, this appears to be the only place where the cpu-exclusive
    section is entered with the BQL taken, which has been found to trigger
    AB-BA deadlock as follows:
    
        vCPU thread                             main thread
        -----------                             -----------
    async_safe_run_on_cpu(self,
                          async_synic_update)
    ...                                         [cpu hot-add]
    process_queued_cpu_work()
      qemu_mutex_unlock_iothread()
                                                [grab BQL]
      start_exclusive()                         cpu_list_add()
      async_synic_update()                        finish_safe_work()
        qemu_mutex_lock_iothread()                  cpu_exec_start()
    
    So remove it.  This paves the way to establishing a strict nesting rule
    of never entering the exclusive section with the BQL taken.
    
    Signed-off-by: default avatarRoman Kagan <rkagan@virtuozzo.com>
    Message-Id: <20190523105440.27045-2-rkagan@virtuozzo.com>