On 2018-02-28 15:13, Max Reitz wrote:
> On 2018-02-27 08:44, Fam Zheng wrote:
>> On Mon, 01/22 23:07, Max Reitz wrote:
>>> @@ -101,7 +105,7 @@ static BlockErrorAction
>>> mirror_error_action(MirrorBlockJob *s, bool read,
>>> }
>>> }
>>>
>>> -static void mirror_iteration_done(MirrorOp *op, int ret)
>>> +static void coroutine_fn mirror_iteration_done(MirrorOp *op, int ret)
>>> {
>>> MirrorBlockJob *s = op->s;
>>> struct iovec *iov;
>>
>> I think we want s/qemu_coroutine_enter/aio_co_wake/ in
>> mirror_iteration_done().
>> As an AIO callback before, this didn't matter, but now we are in an
>> terminating
>> coroutine, so it is pointless to defer the termination, or even risky in
>> that we
>> are in a aio_context_acquire/release section, but have already decremented
>> s->in_flight, which is fishy.
>
> I guess I'll still do the replacement, regardless of whether the next
> patch overwrites it again...Maybe I don't. Doing this breaks iotest 041 because the assert(data.done) in bdrv_co_yield_to_drain() fails. Not sure why that is, but under the circumstance I guess it's best to just pretend this never happened, continue to use qemu_coroutine_enter() and just replace it in the next patch. As for in_flight: What is the issue there? We mostly need that to know how many I/O requests are actually running, that is, how much buffer space is used, how many I/O is done concurrently, etc. (and later we need the in-flight information so that we don't access the target in overlapping areas concurrently). But it doesn't seem to be about how many coroutines there are. So as long as the s->in_flight decrement is done in the same critical section as the op is deleted, we should be good...? Max
signature.asc
Description: OpenPGP digital signature
