Re: [RFC PATCH 3/3] block: Assert we're running in the right thread

[Philippe Mathieu-Daudé]

On 5/14/20 4:30 PM, Kevin Wolf wrote:
> Am 14.05.2020 um 15:52 hat Stefan Reiter geschrieben:
> > On 5/12/20 4:43 PM, Kevin Wolf wrote:
> > > tracked_request_begin() is called for most I/O operations, so it's a
> > > good place to assert that we're indeed running in the home thread of the
> > > node's AioContext.
> >
> > Is this patch supposed to be always correct or only together with nr. 2?
> >
> > I changed our code to call bdrv_flush_all from the main AIO context and it
> > certainly works just fine (even without this series, so I suppose that would
> > be the 'correct' way to fix it you mention on the cover), though of course
> > it trips this assert without patch 2.
>
> Yes, I think this is a basic assumption that should always be true.
> This series shouldn't fix anything for upstream QEMU (at least I'm not
> aware of anything that needs it), so the assertion could be added even
> without the other patches.
>
> In fact, I'm currently thinking that committing just patch 1 (because
> it's a nice cleanup anyway) and patch 3 (because it will let us know
> when we mess up) might make sense.

FWIW applying 1+3 as no-RFC LGTM.

Reviewed-by: Philippe Mathieu-Daudé <address@hidden>

> Kevin
>
> > > Signed-off-by: Kevin Wolf <address@hidden>
> > > ---
> > >    block/io.c | 5 ++++-
> > >    1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/block/io.c b/block/io.c
> > > index 7808e8bdc0..924bf5ba46 100644
> > > --- a/block/io.c
> > > +++ b/block/io.c
> > > @@ -695,14 +695,17 @@ static void tracked_request_begin(BdrvTrackedRequest *req,
> > >                                      uint64_t bytes,
> > >                                      enum BdrvTrackedRequestType type)
> > >    {
> > > +    Coroutine *self = qemu_coroutine_self();
> > > +
> > >        assert(bytes <= INT64_MAX && offset <= INT64_MAX - bytes);
> > > +    assert(bs->aio_context == qemu_coroutine_get_aio_context(self));
> > >        *req = (BdrvTrackedRequest){
> > >            .bs = bs,
> > >            .offset         = offset,
> > >            .bytes          = bytes,
> > >            .type           = type,
> > > -        .co             = qemu_coroutine_self(),
> > > +        .co             = self,
> > >            .serialising    = false,
> > >            .overlap_offset = offset,
> > >            .overlap_bytes  = bytes,