[PATCH 0/2] aio-posix: do not nest poll handlers

Stefan Hajnoczi posted 2 patches 1 year ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20230502184134.534703-1-stefanha@redhat.com
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>, Fam Zheng <fam@euphon.net>
tests/unit/test-nested-aio-poll.c | 130 ++++++++++++++++++++++++++++++
util/aio-posix.c                  |  11 +++
tests/unit/meson.build            |   1 +
3 files changed, 142 insertions(+)
create mode 100644 tests/unit/test-nested-aio-poll.c
[PATCH 0/2] aio-posix: do not nest poll handlers
Posted by Stefan Hajnoczi 1 year ago
The following stack exhaustion was reported in
https://bugzilla.redhat.com/show_bug.cgi?id=2186181:

  ...
  #51 0x000055884fca7451 aio_poll (qemu-kvm + 0x9d6451)
  #52 0x000055884fab9cbd bdrv_poll_co (qemu-kvm + 0x7e8cbd)
  #53 0x000055884fab654b blk_io_plug (qemu-kvm + 0x7e554b)
  #54 0x000055884f927fef virtio_blk_handle_vq (qemu-kvm + 0x656fef)
  #55 0x000055884f96d384 virtio_queue_host_notifier_aio_poll_ready (qemu-kvm + 0x69c384)
  #56 0x000055884fca671b aio_dispatch_handler (qemu-kvm + 0x9d571b)
  #57 0x000055884fca7451 aio_poll (qemu-kvm + 0x9d6451)
  #58 0x000055884fab9cbd bdrv_poll_co (qemu-kvm + 0x7e8cbd)
  #59 0x000055884fab654b blk_io_plug (qemu-kvm + 0x7e554b)
  #60 0x000055884f927fef virtio_blk_handle_vq (qemu-kvm + 0x656fef)
  #61 0x000055884f96d384 virtio_queue_host_notifier_aio_poll_ready (qemu-kvm + 0x69c384)
  #62 0x000055884fca671b aio_dispatch_handler (qemu-kvm + 0x9d571b)
  #63 0x000055884fca7451 aio_poll (qemu-kvm + 0x9d6451)
  ...

This happens because some block layer APIs in QEMU 8.0 run in coroutines in
order to take the graph rdlock. Existing virtqueue handler functions weren't
written with this in mind.

A simplified example of the problem is:

  void my_fd_handler(void *opaque)
  {
      do_something();
      event_notifier_test_and_clear(opaque);
      do_something_else();
  }

When do_something() calls aio_poll(), my_fd_handler() will be entered again
immediately because the fd is still readable and stack exhaustion will occur.

When do_something_else() calls aio_poll(), there is no stack exhaustion since
the event notifier has been cleared and the fd is not readable.

The actual bug is more involved. The handler in question is a poll handler, not
an fd handler, but the principle is the same.

I haven't been able to reproduce the bug, but I have included a test case that
demonstrates the problem.

Stefan Hajnoczi (2):
  aio-posix: do not nest poll handlers
  tested: add test for nested aio_poll() in poll handlers

 tests/unit/test-nested-aio-poll.c | 130 ++++++++++++++++++++++++++++++
 util/aio-posix.c                  |  11 +++
 tests/unit/meson.build            |   1 +
 3 files changed, 142 insertions(+)
 create mode 100644 tests/unit/test-nested-aio-poll.c

-- 
2.40.1
Re: [PATCH 0/2] aio-posix: do not nest poll handlers
Posted by Kevin Wolf 1 year ago
Am 02.05.2023 um 20:41 hat Stefan Hajnoczi geschrieben:
> The following stack exhaustion was reported in
> https://bugzilla.redhat.com/show_bug.cgi?id=2186181:
> 
>   ...
>   #51 0x000055884fca7451 aio_poll (qemu-kvm + 0x9d6451)
>   #52 0x000055884fab9cbd bdrv_poll_co (qemu-kvm + 0x7e8cbd)
>   #53 0x000055884fab654b blk_io_plug (qemu-kvm + 0x7e554b)
>   #54 0x000055884f927fef virtio_blk_handle_vq (qemu-kvm + 0x656fef)
>   #55 0x000055884f96d384 virtio_queue_host_notifier_aio_poll_ready (qemu-kvm + 0x69c384)
>   #56 0x000055884fca671b aio_dispatch_handler (qemu-kvm + 0x9d571b)
>   #57 0x000055884fca7451 aio_poll (qemu-kvm + 0x9d6451)
>   #58 0x000055884fab9cbd bdrv_poll_co (qemu-kvm + 0x7e8cbd)
>   #59 0x000055884fab654b blk_io_plug (qemu-kvm + 0x7e554b)
>   #60 0x000055884f927fef virtio_blk_handle_vq (qemu-kvm + 0x656fef)
>   #61 0x000055884f96d384 virtio_queue_host_notifier_aio_poll_ready (qemu-kvm + 0x69c384)
>   #62 0x000055884fca671b aio_dispatch_handler (qemu-kvm + 0x9d571b)
>   #63 0x000055884fca7451 aio_poll (qemu-kvm + 0x9d6451)
>   ...
> 
> This happens because some block layer APIs in QEMU 8.0 run in coroutines in
> order to take the graph rdlock. Existing virtqueue handler functions weren't
> written with this in mind.
> 
> A simplified example of the problem is:
> 
>   void my_fd_handler(void *opaque)
>   {
>       do_something();
>       event_notifier_test_and_clear(opaque);
>       do_something_else();
>   }
> 
> When do_something() calls aio_poll(), my_fd_handler() will be entered again
> immediately because the fd is still readable and stack exhaustion will occur.
> 
> When do_something_else() calls aio_poll(), there is no stack exhaustion since
> the event notifier has been cleared and the fd is not readable.
> 
> The actual bug is more involved. The handler in question is a poll handler, not
> an fd handler, but the principle is the same.
> 
> I haven't been able to reproduce the bug, but I have included a test case that
> demonstrates the problem.

Thanks, applied to the block branch.

Kevin