[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH 6/8] quorum: Avoid bdrv_aio_writev() for rew
From: |
Alberto Garcia |
Subject: |
Re: [Qemu-devel] [RFC PATCH 6/8] quorum: Avoid bdrv_aio_writev() for rewrites |
Date: |
Fri, 18 Nov 2016 13:33:52 +0100 |
User-agent: |
Notmuch/0.18.2 (http://notmuchmail.org) Emacs/24.4.1 (i586-pc-linux-gnu) |
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
Re: [Qemu-devel] [RFC PATCH 0/8] quorum: Implement .bdrv_co_preadv/pwritev(), Paolo Bonzini, 2016/11/11
Re: [Qemu-devel] [RFC PATCH 0/8] quorum: Implement .bdrv_co_preadv/pwritev(), no-reply, 2016/11/12