Re: [Qemu-devel] [PATCH 1/3] io: Yield rather than wait when already in coroutine

[Eric Blake]

On 09/06/2017 03:50 AM, Daniel P. Berrange wrote:
> On Tue, Sep 05, 2017 at 02:11:12PM -0500, Eric Blake wrote:
>> The new qio_channel_{read,write}{,v}_all functions are documented
>> as yielding until data is available.  When used on a blocking
>> channel, this yield is done via qio_channel_wait() which spawns
>> a new coroutine under the hood (so it is the new entry point that
>> yields as needed); but if we are already in a coroutine (at which
> 
> qio_channel_wait doesn't spawn any coroutine - it simply rnus a
> nested event loop to wait for the channel...
> 
>> point QIO_CHANNEL_ERR_BLOCK is only possible if we are a
>> non-blocking channel), we want to yield the current coroutine
>> instead of spawning a new one.
> 
> ...none the less, I think this is ok.

Okay, tweaking the commit message:

The new qio_channel_{read,write}{,v}_all functions are documented
as yielding until data is available.  When used on a blocking
channel, this yield is done via qio_channel_wait() which spawns
a nested event loop under the hood (so it is that secondary loop
which yields as needed); but if we are already in a coroutine (at
which point QIO_CHANNEL_ERR_BLOCK is only possible if we are a
non-blocking channel), we want to yield the current coroutine
instead of spawning a nested event loop.

> 
> Acked-by: Daniel P. Berrange 
> 
> Regards,
> Daniel
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 1/3] io: Yield rather than wait when already in coroutine

[Paolo Bonzini]

Il 06 set 2017 10:51 AM, "Daniel P. Berrange"  ha
scritto:

On Tue, Sep 05, 2017 at 02:11:12PM -0500, Eric Blake wrote:
> The new qio_channel_{read,write}{,v}_all functions are documented
> as yielding until data is available.  When used on a blocking
> channel, this yield is done via qio_channel_wait() which spawns
> a new coroutine under the hood (so it is the new entry point that
> yields as needed); but if we are already in a coroutine (at which

qio_channel_wait doesn't spawn any coroutine - it simply rnus a
nested event loop to wait for the channel...

> point QIO_CHANNEL_ERR_BLOCK is only possible if we are a
> non-blocking channel), we want to yield the current coroutine
> instead of spawning a new one.

...none the less, I think this is ok.


Great, this works for me on the pr-helper too.

Paolo


>
> Signed-off-by: Eric Blake 
> ---
>  io/channel.c | 12 ++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/io/channel.c b/io/channel.c
> index 5e8c2f0a91..9e62794cab 100644
> --- a/io/channel.c
> +++ b/io/channel.c
> @@ -105,7 +105,11 @@ int qio_channel_readv_all(QIOChannel *ioc,
>  ssize_t len;
>  len = qio_channel_readv(ioc, local_iov, nlocal_iov, errp);
>  if (len == QIO_CHANNEL_ERR_BLOCK) {
> -qio_channel_wait(ioc, G_IO_IN);
> +if (qemu_in_coroutine()) {
> +qio_channel_yield(ioc, G_IO_IN);
> +} else {
> +qio_channel_wait(ioc, G_IO_IN);
> +}
>  continue;
>  } else if (len < 0) {
>  goto cleanup;
> @@ -143,7 +147,11 @@ int qio_channel_writev_all(QIOChannel *ioc,
>  ssize_t len;
>  len = qio_channel_writev(ioc, local_iov, nlocal_iov, errp);
>  if (len == QIO_CHANNEL_ERR_BLOCK) {
> -qio_channel_wait(ioc, G_IO_OUT);
> +if (qemu_in_coroutine()) {
> +qio_channel_yield(ioc, G_IO_OUT);
> +} else {
> +qio_channel_wait(ioc, G_IO_OUT);
> +}
>  continue;
>  }
>  if (len < 0) {

Acked-by: Daniel P. Berrange 

Regards,
Daniel
--
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange
:|
|: https://libvirt.org -o-https://fstop138.berrange.com
:|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange
:|

Re: [Qemu-devel] [PATCH 1/3] io: Yield rather than wait when already in coroutine

[Daniel P. Berrange]

On Tue, Sep 05, 2017 at 02:11:12PM -0500, Eric Blake wrote:
> The new qio_channel_{read,write}{,v}_all functions are documented
> as yielding until data is available.  When used on a blocking
> channel, this yield is done via qio_channel_wait() which spawns
> a new coroutine under the hood (so it is the new entry point that
> yields as needed); but if we are already in a coroutine (at which

qio_channel_wait doesn't spawn any coroutine - it simply rnus a
nested event loop to wait for the channel...

> point QIO_CHANNEL_ERR_BLOCK is only possible if we are a
> non-blocking channel), we want to yield the current coroutine
> instead of spawning a new one.

...none the less, I think this is ok.

> 
> Signed-off-by: Eric Blake 
> ---
>  io/channel.c | 12 ++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/io/channel.c b/io/channel.c
> index 5e8c2f0a91..9e62794cab 100644
> --- a/io/channel.c
> +++ b/io/channel.c
> @@ -105,7 +105,11 @@ int qio_channel_readv_all(QIOChannel *ioc,
>  ssize_t len;
>  len = qio_channel_readv(ioc, local_iov, nlocal_iov, errp);
>  if (len == QIO_CHANNEL_ERR_BLOCK) {
> -qio_channel_wait(ioc, G_IO_IN);
> +if (qemu_in_coroutine()) {
> +qio_channel_yield(ioc, G_IO_IN);
> +} else {
> +qio_channel_wait(ioc, G_IO_IN);
> +}
>  continue;
>  } else if (len < 0) {
>  goto cleanup;
> @@ -143,7 +147,11 @@ int qio_channel_writev_all(QIOChannel *ioc,
>  ssize_t len;
>  len = qio_channel_writev(ioc, local_iov, nlocal_iov, errp);
>  if (len == QIO_CHANNEL_ERR_BLOCK) {
> -qio_channel_wait(ioc, G_IO_OUT);
> +if (qemu_in_coroutine()) {
> +qio_channel_yield(ioc, G_IO_OUT);
> +} else {
> +qio_channel_wait(ioc, G_IO_OUT);
> +}
>  continue;
>  }
>  if (len < 0) {

Acked-by: Daniel P. Berrange 

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

[Qemu-devel] [PATCH 1/3] io: Yield rather than wait when already in coroutine

[Eric Blake]

The new qio_channel_{read,write}{,v}_all functions are documented
as yielding until data is available.  When used on a blocking
channel, this yield is done via qio_channel_wait() which spawns
a new coroutine under the hood (so it is the new entry point that
yields as needed); but if we are already in a coroutine (at which
point QIO_CHANNEL_ERR_BLOCK is only possible if we are a
non-blocking channel), we want to yield the current coroutine
instead of spawning a new one.

Signed-off-by: Eric Blake 
---
 io/channel.c | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/io/channel.c b/io/channel.c
index 5e8c2f0a91..9e62794cab 100644
--- a/io/channel.c
+++ b/io/channel.c
@@ -105,7 +105,11 @@ int qio_channel_readv_all(QIOChannel *ioc,
 ssize_t len;
 len = qio_channel_readv(ioc, local_iov, nlocal_iov, errp);
 if (len == QIO_CHANNEL_ERR_BLOCK) {
-qio_channel_wait(ioc, G_IO_IN);
+if (qemu_in_coroutine()) {
+qio_channel_yield(ioc, G_IO_IN);
+} else {
+qio_channel_wait(ioc, G_IO_IN);
+}
 continue;
 } else if (len < 0) {
 goto cleanup;
@@ -143,7 +147,11 @@ int qio_channel_writev_all(QIOChannel *ioc,
 ssize_t len;
 len = qio_channel_writev(ioc, local_iov, nlocal_iov, errp);
 if (len == QIO_CHANNEL_ERR_BLOCK) {
-qio_channel_wait(ioc, G_IO_OUT);
+if (qemu_in_coroutine()) {
+qio_channel_yield(ioc, G_IO_OUT);
+} else {
+qio_channel_wait(ioc, G_IO_OUT);
+}
 continue;
 }
 if (len < 0) {
-- 
2.13.5