| Message ID | 20181220152030.28035-1-remy.noel@blade-group.com |
|---|---|
| Headers | show |
| Series | Fix concurrent aio_poll/set_fd_handler. | expand |
On Thu, Dec 20, 2018 at 04:20:28PM +0100, remy.noel@blade-group.com wrote: > From: Remy Noel <remy.noel@blade-group.com> > > It is possible for an io_poll/read/write callback to be concurrently executed along > with an aio_set_fd_handlers. This can cause all sorts of problems, like > a NULL callback or a bad opaque pointer. > > V2: > * Do not use RCU anymore as it inccurs a performance loss > V3: > * Don't drop revents when a handler is modified [Stefan] > V4: > * Unregister fd from ctx epoll when removing fd_handler [Paolo] > > Remy Noel (2): > aio-posix: Unregister fd from ctx epoll when removing fd_handler. > aio-posix: Fix concurrent aio_poll/set_fd_handler. > > util/aio-posix.c | 90 +++++++++++++++++++++++++++++------------------- > util/aio-win32.c | 67 ++++++++++++++++------------------- > 2 files changed, 84 insertions(+), 73 deletions(-) > > -- > 2.19.2 > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
On 20/12/18 17:37, Stefan Hajnoczi wrote: > On Thu, Dec 20, 2018 at 04:20:28PM +0100, remy.noel@blade-group.com wrote: >> From: Remy Noel <remy.noel@blade-group.com> >> >> It is possible for an io_poll/read/write callback to be concurrently executed along >> with an aio_set_fd_handlers. This can cause all sorts of problems, like >> a NULL callback or a bad opaque pointer. >> >> V2: >> * Do not use RCU anymore as it inccurs a performance loss >> V3: >> * Don't drop revents when a handler is modified [Stefan] >> V4: >> * Unregister fd from ctx epoll when removing fd_handler [Paolo] >> >> Remy Noel (2): >> aio-posix: Unregister fd from ctx epoll when removing fd_handler. >> aio-posix: Fix concurrent aio_poll/set_fd_handler. >> >> util/aio-posix.c | 90 +++++++++++++++++++++++++++++------------------- >> util/aio-win32.c | 67 ++++++++++++++++------------------- >> 2 files changed, 84 insertions(+), 73 deletions(-) >> >> -- >> 2.19.2 >> > > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> FWIW, I had missed the early version that used RCU, but lockcnt is already very RCU-like, so not using RCU is the right thing to do. The difference between lockcnt and RCU is that cleanup is done by the reader instead of a separate thread. Because we know that reader/writer concurrency is very rare for AioContext handlers, the tradeoffs favor lockcnt over RCU. Paolo
On 12/21/18 12:34 PM, Paolo Bonzini wrote: > FWIW, I had missed the early version that used RCU, but lockcnt is > already very RCU-like, so not using RCU is the right thing to do. The > difference between lockcnt and RCU is that cleanup is done by the reader > instead of a separate thread. Because we know that reader/writer > concurrency is very rare for AioContext handlers, the tradeoffs favor > lockcnt over RCU. Indeed, i forgot to CC you in the first batch (get_maintainer.pl was not finding you). Thanks for the RCU hints, i though the performance hit was due to the RCU being global to qemu. Remy
On Thu, Dec 20, 2018 at 04:20:28PM +0100, Remy Noel wrote: >From: Remy Noel <remy.noel@blade-group.com> > >It is possible for an io_poll/read/write callback to be concurrently executed along >with an aio_set_fd_handlers. This can cause all sorts of problems, like >a NULL callback or a bad opaque pointer. > >V2: > * Do not use RCU anymore as it inccurs a performance loss >V3: > * Don't drop revents when a handler is modified [Stefan] >V4: > * Unregister fd from ctx epoll when removing fd_handler [Paolo] > >Remy Noel (2): > aio-posix: Unregister fd from ctx epoll when removing fd_handler. > aio-posix: Fix concurrent aio_poll/set_fd_handler. > > util/aio-posix.c | 90 +++++++++++++++++++++++++++++------------------- > util/aio-win32.c | 67 ++++++++++++++++------------------- > 2 files changed, 84 insertions(+), 73 deletions(-) > >-- >2.19.2 > ping. Does it needs anything for getting queued ? Thanks. Remy.
On Thu, Dec 20, 2018 at 04:20:28PM +0100, remy.noel@blade-group.com wrote: > From: Remy Noel <remy.noel@blade-group.com> > > It is possible for an io_poll/read/write callback to be concurrently executed along > with an aio_set_fd_handlers. This can cause all sorts of problems, like > a NULL callback or a bad opaque pointer. > > V2: > * Do not use RCU anymore as it inccurs a performance loss > V3: > * Don't drop revents when a handler is modified [Stefan] > V4: > * Unregister fd from ctx epoll when removing fd_handler [Paolo] > > Remy Noel (2): > aio-posix: Unregister fd from ctx epoll when removing fd_handler. > aio-posix: Fix concurrent aio_poll/set_fd_handler. > > util/aio-posix.c | 90 +++++++++++++++++++++++++++++------------------- > util/aio-win32.c | 67 ++++++++++++++++------------------- > 2 files changed, 84 insertions(+), 73 deletions(-) > > -- > 2.19.2 > Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan
On Thu, Dec 20, 2018 at 04:20:28PM +0100, remy.noel@blade-group.com wrote: > From: Remy Noel <remy.noel@blade-group.com> > > It is possible for an io_poll/read/write callback to be concurrently executed along > with an aio_set_fd_handlers. This can cause all sorts of problems, like > a NULL callback or a bad opaque pointer. > > V2: > * Do not use RCU anymore as it inccurs a performance loss > V3: > * Don't drop revents when a handler is modified [Stefan] > V4: > * Unregister fd from ctx epoll when removing fd_handler [Paolo] > > Remy Noel (2): > aio-posix: Unregister fd from ctx epoll when removing fd_handler. > aio-posix: Fix concurrent aio_poll/set_fd_handler. > > util/aio-posix.c | 90 +++++++++++++++++++++++++++++------------------- > util/aio-win32.c | 67 ++++++++++++++++------------------- > 2 files changed, 84 insertions(+), 73 deletions(-) > > -- > 2.19.2 > Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan
On Sat, Jan 12, 2019 at 08:30:08AM +0000, Stefan Hajnoczi wrote: >Thanks, applied to my block tree: >https://github.com/stefanha/qemu/commits/block > Thanks ! Remy
From: Remy Noel <remy.noel@blade-group.com> It is possible for an io_poll/read/write callback to be concurrently executed along with an aio_set_fd_handlers. This can cause all sorts of problems, like a NULL callback or a bad opaque pointer. V2: * Do not use RCU anymore as it inccurs a performance loss V3: * Don't drop revents when a handler is modified [Stefan] V4: * Unregister fd from ctx epoll when removing fd_handler [Paolo] Remy Noel (2): aio-posix: Unregister fd from ctx epoll when removing fd_handler. aio-posix: Fix concurrent aio_poll/set_fd_handler. util/aio-posix.c | 90 +++++++++++++++++++++++++++++------------------- util/aio-win32.c | 67 ++++++++++++++++------------------- 2 files changed, 84 insertions(+), 73 deletions(-)