From: Paolo Bonzini <pbonzini@redhat.com>
To: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
Emanuele Giuseppe Esposito <eesposit@redhat.com>,
qemu-block@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>, John Snow <jsnow@redhat.com>,
qemu-devel@nongnu.org, Max Reitz <mreitz@redhat.com>
Subject: Re: [RFC PATCH 0/3] block-copy: lock tasks and calls list
Date: Wed, 21 Apr 2021 14:13:59 +0200 [thread overview]
Message-ID: <9e8a7846-a2a6-fd98-f7c4-eacf29d18a29@redhat.com> (raw)
In-Reply-To: <27d07471-76f0-5c3a-79f8-824663ec14a2@virtuozzo.com>
On 21/04/21 10:53, Vladimir Sementsov-Ogievskiy wrote:
>>
>> Good point. Emanuele, can you work on ProgressMeter and
>> SharedResource? AioTaskPool can also be converted to just use CoQueue
>> instead of manually waking up coroutines.
>>
>
> That would be great.
>
> I have one more question in mind:
>
> Is it effective to use CoMutex here? We are protecting only some fast
> manipulations with data, not io path or something like that. Will simple
> QemuMutex work better? Even if CoMutex doesn't have any overhead, I
> don't think than if thread A wants to modify task list, but mutex is
> held by thread B (for similar thing), there is a reason for thread A to
> yield and do some other things: it can just wait several moments on
> mutex while B is modifying task list..
Indeed even CoQueue primitives count as simple manipulation of data,
because they unlock/lock the mutex while the coroutine sleeps. So
you're right that it would be okay to use QemuMutex as well
The block copy code that Emanuele has touched so far is all coroutine
based. I like using CoMutex when that is the case, because it says
implicitly "the monitor is not involved". But we need to see what it
will be like when the patches are complete.
Rate limiting ends up being called by the monitor, but it will have its
own QemuMutex so it's fine. What's left is cancellation and
block_copy_kick; I think that we can make qemu_co_sleep thread-safe with
an API similar to Linux's prepare_to_wait, so a QemuMutex wouldn't be
needed there either.
Paolo
prev parent reply other threads:[~2021-04-21 12:15 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-20 10:04 [RFC PATCH 0/3] block-copy: lock tasks and calls list Emanuele Giuseppe Esposito
2021-04-20 10:04 ` [RFC PATCH 1/3] block-copy: improve documentation for BlockCopyTask type and functions Emanuele Giuseppe Esposito
2021-04-20 12:03 ` Vladimir Sementsov-Ogievskiy
2021-04-20 12:51 ` Emanuele Giuseppe Esposito
2021-04-20 13:11 ` Vladimir Sementsov-Ogievskiy
2021-04-20 10:04 ` [RFC PATCH 2/3] block-copy: add a CoMutex to the BlockCopyTask list Emanuele Giuseppe Esposito
2021-04-20 10:04 ` [RFC PATCH 3/3] block-copy: add CoMutex lock for BlockCopyCallState list Emanuele Giuseppe Esposito
2021-04-20 13:12 ` [RFC PATCH 0/3] block-copy: lock tasks and calls list Vladimir Sementsov-Ogievskiy
2021-04-21 8:38 ` Paolo Bonzini
2021-04-21 8:53 ` Vladimir Sementsov-Ogievskiy
2021-04-21 12:13 ` Paolo Bonzini [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=9e8a7846-a2a6-fd98-f7c4-eacf29d18a29@redhat.com \
--to=pbonzini@redhat.com \
--cc=eesposit@redhat.com \
--cc=jsnow@redhat.com \
--cc=kwolf@redhat.com \
--cc=mreitz@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=vsementsov@virtuozzo.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).