On Fri 18 Nov 2016 01:21:12 PM CET, Kevin Wolf wrote:
>> > + /* one less rewrite to do */
>> > + acb->rewrite_count--;
>> > + qemu_coroutine_enter_if_inactive(acb->co);
>>
>> I think you should only enter acb->co when acb->rewrite_count reaches
>> zero.
>>
>> In all other cases the main coroutine simply iterates inside the
>> while() loop, verifies that the number is still positive and yields
>> again.
>>
>> The same applies to all other cases of
>> qemu_coroutine_enter_if_inactive, by the way (I failed to notice it
>> in patch #5).
>
> I think I like it better this way because it keeps the loop condition
> local to the caller instead of spreading it across the caller and the
> places that reenter. On the other hand, I can see that not doing the
> extra context switch might be a little more efficient.
Apart from the efficiency, I actually find it a bit easier to understand
if it's made explicit:
/* Once all rewrites are done we can wake up the caller */
if (--acb->rewrite_count == 0) {
qemu_coroutine_enter_if_inactive(acb->co);
}
But I can understand that some people may find your way easier. I don't
feel strongly about it, so no need to change it.
Berto
