qemu-commits
[Top][All Lists]
Advanced

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

[Qemu-commits] [qemu/qemu] 178bd4: block: Walk bs->children carefully in


From: GitHub
Subject: [Qemu-commits] [qemu/qemu] 178bd4: block: Walk bs->children carefully in bdrv_drain_r...
Date: Tue, 18 Apr 2017 09:15:11 -0700

  Branch: refs/heads/master
  Home:   https://github.com/qemu/qemu
  Commit: 178bd438af5c95deef5073416c60396f88e97ec9
      
https://github.com/qemu/qemu/commit/178bd438af5c95deef5073416c60396f88e97ec9
  Author: Fam Zheng <address@hidden>
  Date:   2017-04-18 (Tue, 18 Apr 2017)

  Changed paths:
    M block/io.c

  Log Message:
  -----------
  block: Walk bs->children carefully in bdrv_drain_recurse

The recursive bdrv_drain_recurse may run a block job completion BH that
drops nodes. The coming changes will make that more likely and use-after-free
would happen without this patch

Stash the bs pointer and use bdrv_ref/bdrv_unref in addition to
QLIST_FOREACH_SAFE to prevent such a case from happening.

Since bdrv_unref accesses global state that is not protected by the AioContext
lock, we cannot use bdrv_ref/bdrv_unref unconditionally.  Fortunately the
protection is not needed in IOThread because only main loop can modify a graph
with the AioContext lock held.

Signed-off-by: Fam Zheng <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Jeff Cody <address@hidden>
Tested-by: Jeff Cody <address@hidden>
Signed-off-by: Fam Zheng <address@hidden>


  Commit: 91af091f92358c2ff828fa1def1a7bea9b701cdf
      
https://github.com/qemu/qemu/commit/91af091f92358c2ff828fa1def1a7bea9b701cdf
  Author: Fam Zheng <address@hidden>
  Date:   2017-04-18 (Tue, 18 Apr 2017)

  Changed paths:
    M include/block/block.h

  Log Message:
  -----------
  block: Drain BH in bdrv_drained_begin

During block job completion, nothing is preventing
block_job_defer_to_main_loop_bh from being called in a nested
aio_poll(), which is a trouble, such as in this code path:

    qmp_block_commit
      commit_active_start
  bdrv_reopen
    bdrv_reopen_multiple
      bdrv_reopen_prepare
        bdrv_flush
          aio_poll
            aio_bh_poll
              aio_bh_call
                block_job_defer_to_main_loop_bh
                  stream_complete
                    bdrv_reopen

block_job_defer_to_main_loop_bh is the last step of the stream job,
which should have been "paused" by the bdrv_drained_begin/end in
bdrv_reopen_multiple, but it is not done because it's in the form of a
main loop BH.

Similar to why block jobs should be paused between drained_begin and
drained_end, BHs they schedule must be excluded as well.  To achieve
this, this patch forces draining the BH in BDRV_POLL_WHILE.

As a side effect this fixes a hang in block_job_detach_aio_context
during system_reset when a block job is ready:

    #0  0x0000555555aa79f3 in bdrv_drain_recurse
    #1  0x0000555555aa825d in bdrv_drained_begin
    #2  0x0000555555aa8449 in bdrv_drain
    #3  0x0000555555a9c356 in blk_drain
    #4  0x0000555555aa3cfd in mirror_drain
    #5  0x0000555555a66e11 in block_job_detach_aio_context
    #6  0x0000555555a62f4d in bdrv_detach_aio_context
    #7  0x0000555555a63116 in bdrv_set_aio_context
    #8  0x0000555555a9d326 in blk_set_aio_context
    #9  0x00005555557e38da in virtio_blk_data_plane_stop
    #10 0x00005555559f9d5f in virtio_bus_stop_ioeventfd
    #11 0x00005555559fa49b in virtio_bus_stop_ioeventfd
    #12 0x00005555559f6a18 in virtio_pci_stop_ioeventfd
    #13 0x00005555559f6a18 in virtio_pci_reset
    #14 0x00005555559139a9 in qdev_reset_one
    #15 0x0000555555916738 in qbus_walk_children
    #16 0x0000555555913318 in qdev_walk_children
    #17 0x0000555555916738 in qbus_walk_children
    #18 0x00005555559168ca in qemu_devices_reset
    #19 0x000055555581fcbb in pc_machine_reset
    #20 0x00005555558a4d96 in qemu_system_reset
    #21 0x000055555577157a in main_loop_should_exit
    #22 0x000055555577157a in main_loop
    #23 0x000055555577157a in main

The rationale is that the loop in block_job_detach_aio_context cannot
make any progress in pausing/completing the job, because bs->in_flight
is 0, so bdrv_drain doesn't process the block_job_defer_to_main_loop
BH. With this patch, it does.

Reported-by: Jeff Cody <address@hidden>
Signed-off-by: Fam Zheng <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Jeff Cody <address@hidden>
Tested-by: Jeff Cody <address@hidden>
Signed-off-by: Fam Zheng <address@hidden>


  Commit: d1263f8f1805ffe6a9da8520a6aa96062bc794f4
      
https://github.com/qemu/qemu/commit/d1263f8f1805ffe6a9da8520a6aa96062bc794f4
  Author: Peter Maydell <address@hidden>
  Date:   2017-04-18 (Tue, 18 Apr 2017)

  Changed paths:
    M block/io.c
    M include/block/block.h

  Log Message:
  -----------
  Merge remote-tracking branch 'remotes/famz/tags/block-pull-request' into 
staging

# gpg: Signature made Tue 18 Apr 2017 15:58:32 BST
# gpg:                using RSA key 0xCA35624C6A9171C6
# gpg: Good signature from "Fam Zheng <address@hidden>"
# gpg: WARNING: This key is not certified with sufficiently trusted signatures!
# gpg:          It is not certain that the signature belongs to the owner.
# Primary key fingerprint: 5003 7CB7 9706 0F76 F021  AD56 CA35 624C 6A91 71C6

* remotes/famz/tags/block-pull-request:
  block: Drain BH in bdrv_drained_begin
  block: Walk bs->children carefully in bdrv_drain_recurse

Signed-off-by: Peter Maydell <address@hidden>


  Commit: ca55019dacb821cc675273237a5173fc67bf3230
      
https://github.com/qemu/qemu/commit/ca55019dacb821cc675273237a5173fc67bf3230
  Author: Peter Maydell <address@hidden>
  Date:   2017-04-18 (Tue, 18 Apr 2017)

  Changed paths:
    M VERSION

  Log Message:
  -----------
  Update version for v2.9.0-rc5 release

Signed-off-by: Peter Maydell <address@hidden>


Compare: https://github.com/qemu/qemu/compare/9c6b899f7a46...ca55019dacb8

reply via email to

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