[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v7 13/13] block: Convert 'block_resize' to coroutine
From: |
Kevin Wolf |
Subject: |
Re: [PATCH v7 13/13] block: Convert 'block_resize' to coroutine |
Date: |
Fri, 25 Sep 2020 18:07:50 +0200 |
Am 15.09.2020 um 16:57 hat Stefan Hajnoczi geschrieben:
> On Wed, Sep 09, 2020 at 05:11:49PM +0200, Kevin Wolf wrote:
> > @@ -2456,8 +2456,7 @@ void qmp_block_resize(bool has_device, const char
> > *device,
> > return;
> > }
> >
> > - aio_context = bdrv_get_aio_context(bs);
> > - aio_context_acquire(aio_context);
> > + old_ctx = bdrv_co_move_to_aio_context(bs);
> >
> > if (size < 0) {
> > error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "size", "a >0
> > size");
>
> Is it safe to call blk_new() outside the BQL since it mutates global state?
>
> In other words, could another thread race with us?
Hm, probably not.
Would it be safer to have the bdrv_co_move_to_aio_context() call only
immediately before the drain?
> > @@ -2479,8 +2478,8 @@ void qmp_block_resize(bool has_device, const char
> > *device,
> > bdrv_drained_end(bs);
> >
> > out:
> > + aio_co_reschedule_self(old_ctx);
> > blk_unref(blk);
> > - aio_context_release(aio_context);
>
> The following precondition is violated by the blk_unref -> bdrv_drain ->
> AIO_WAIT_WHILE() call if blk->refcnt is 1 here:
>
> * The caller's thread must be the IOThread that owns @ctx or the main loop
> * thread (with @ctx acquired exactly once).
>
> blk_unref() is called from the main loop thread without having acquired
> blk's AioContext.
>
> Normally blk->refcnt will be > 1 so bdrv_drain() won't be called, but
> I'm not sure if that can be guaranteed.
>
> The following seems safer although it's uglier:
>
> aio_context = bdrv_get_aio_context(bs);
> aio_context_acquire(aio_context);
> blk_unref(blk);
> aio_context_release(aio_context);
May we actually acquire aio_context if blk is in the main thread? I
think we must only do this if it's in a different iothread because we'd
end up with a recursive lock and drain would hang.
Kevin
signature.asc
Description: PGP signature
- Re: [PATCH v7 11/13] util/async: Add aio_co_reschedule_self(), (continued)
- [PATCH v7 10/13] hmp: Add support for coroutine command handlers, Kevin Wolf, 2020/09/09
- [PATCH v7 12/13] block: Add bdrv_co_move_to_aio_context(), Kevin Wolf, 2020/09/09
- [PATCH v7 13/13] block: Convert 'block_resize' to coroutine, Kevin Wolf, 2020/09/09
- Re: [PATCH v7 00/13] monitor: Optionally run handlers in coroutines, no-reply, 2020/09/09
- Re: [PATCH v7 00/13] monitor: Optionally run handlers in coroutines, Stefan Hajnoczi, 2020/09/10