Re: bdrv_co_pwritev: Assertion `!waited || !use_local_qiov' failed.

[Kevin Wolf]

Am 17.12.2019 um 15:14 hat Peter Lieven geschrieben:
> I have a vserver running Qemu 4.0 that seems to reproducibly hit the
> following assertion:
> 
>  bdrv_co_pwritev: Assertion `!waited || !use_local_qiov' failed.
> 
> I noticed that the padding code was recently reworked in commit
> 2e2ad02f2c. In the new code I cannot find a similar assertion. Was
> the assertion wrong or why was it dropped?

No, the assertion in the old version makes sense to me. The code goes
basically like this:

    if (head unaligned) {
        /* Make sure no new conflicting request will be started */
        mark_request_serialising()

        /* Wait if a conflicting request is already in flight */
        wait_serialising_requests()

        adjust start of the request
    }

    if (tail unaligned) {
        /* Make sure no new conflicting request will be started */
        mark_request_serialising()

        /* Wait if a conflicting request is already in flight. If we
         * already had an unaligned head, we already waited for
         * conflicting requests and no new requests may have been
         * started, so in this case this must be a no-op. */
        wait_serialising_requests()

        adjust end of the request
    }

If the assertion fails, we already waited in the "head unaligned" case,
but a new request snuck in even though we marked this request as
serialising, so the other request should have waited.

This might mean that a wait_serialising_requests() is missing somewhere.

> I try to add some debugging code to find out what is exactly happing.
> Especially the requests that are in flight when the assertion is
> triggered.

You can just have a look at bs->tracked_requests in gdb (if you can
trigger the bug in a debugging environment). However, at the time of the
assertion it's too late, we have waited for the bad request to complete,
so it's gone now. You probably need to tell
bdrv_wait_serialising_requests() that it's not supposed to wait, so you
can assert the condition already there.

(In the new code, I think the situation is different because both head
and tail are handled at once, so we don't even have two instances of
mark_request_serialising() and wait_serialising_requests() any more. But
if a bug existed previously, it probably still exists.)

Kevin