qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
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



      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).