Re: [PATCH RFC 4/9] migration/rdma: Change io_create_watch() to return immediately

[Fabiano Rosas <farosas@suse.de>]

Peter Xu <peterx@redhat.com> writes:

> The old RDMA's io_create_watch() isn't really doing much work anyway.  For
> G_IO_OUT, it already does return immediately.  For G_IO_IN, it will try to
> detect some RDMA context length however normally nobody will be able to set
> it at all.
>
> Simplify the code so that RDMA iochannels simply always rely on synchronous
> reads and writes.  It is highly likely what 6ddd2d76ca6f86f was talking
> about, that the async model isn't really working well.
>
> This helps because this is almost the only dependency that the migration
> core would need a coroutine for rdma channels.
>

I don't understand this. How does this code require a coroutine? Isn't
the io_watch exactly the strategy used when there is no coroutine?

> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  migration/rdma.c | 69 +++---------------------------------------------
>  1 file changed, 3 insertions(+), 66 deletions(-)
>
> diff --git a/migration/rdma.c b/migration/rdma.c
> index ed4e20b988..bcd7aae2f2 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -2789,56 +2789,14 @@ static gboolean
>  qio_channel_rdma_source_prepare(GSource *source,
>                                  gint *timeout)
>  {
> -    QIOChannelRDMASource *rsource = (QIOChannelRDMASource *)source;
> -    RDMAContext *rdma;
> -    GIOCondition cond = 0;
>      *timeout = -1;
> -
> -    RCU_READ_LOCK_GUARD();
> -    if (rsource->condition == G_IO_IN) {
> -        rdma = qatomic_rcu_read(&rsource->rioc->rdmain);
> -    } else {
> -        rdma = qatomic_rcu_read(&rsource->rioc->rdmaout);
> -    }
> -
> -    if (!rdma) {
> -        error_report("RDMAContext is NULL when prepare Gsource");
> -        return FALSE;
> -    }
> -
> -    if (rdma->wr_data[0].control_len) {
> -        cond |= G_IO_IN;
> -    }
> -    cond |= G_IO_OUT;
> -
> -    return cond & rsource->condition;
> +    return TRUE;
>  }
>  
>  static gboolean
>  qio_channel_rdma_source_check(GSource *source)
>  {
> -    QIOChannelRDMASource *rsource = (QIOChannelRDMASource *)source;
> -    RDMAContext *rdma;
> -    GIOCondition cond = 0;
> -
> -    RCU_READ_LOCK_GUARD();
> -    if (rsource->condition == G_IO_IN) {
> -        rdma = qatomic_rcu_read(&rsource->rioc->rdmain);
> -    } else {
> -        rdma = qatomic_rcu_read(&rsource->rioc->rdmaout);
> -    }
> -
> -    if (!rdma) {
> -        error_report("RDMAContext is NULL when check Gsource");
> -        return FALSE;
> -    }
> -
> -    if (rdma->wr_data[0].control_len) {
> -        cond |= G_IO_IN;
> -    }
> -    cond |= G_IO_OUT;
> -
> -    return cond & rsource->condition;
> +    return TRUE;

These are fine if we want the source to run as soon as possible, I
think. But then...

>  }
>  
>  static gboolean
> @@ -2848,29 +2806,8 @@ qio_channel_rdma_source_dispatch(GSource *source,
>  {
>      QIOChannelFunc func = (QIOChannelFunc)callback;
>      QIOChannelRDMASource *rsource = (QIOChannelRDMASource *)source;
> -    RDMAContext *rdma;
> -    GIOCondition cond = 0;
> -
> -    RCU_READ_LOCK_GUARD();
> -    if (rsource->condition == G_IO_IN) {
> -        rdma = qatomic_rcu_read(&rsource->rioc->rdmain);
> -    } else {
> -        rdma = qatomic_rcu_read(&rsource->rioc->rdmaout);
> -    }
> -
> -    if (!rdma) {
> -        error_report("RDMAContext is NULL when dispatch Gsource");
> -        return FALSE;
> -    }
> -
> -    if (rdma->wr_data[0].control_len) {
> -        cond |= G_IO_IN;
> -    }
> -    cond |= G_IO_OUT;
>  
> -    return (*func)(QIO_CHANNEL(rsource->rioc),
> -                   (cond & rsource->condition),
> -                   user_data);
> +    return (*func)(QIO_CHANNEL(rsource->rioc), rsource->condition, user_data);

No idea who even calls g_source_set_callback() in this case. What is func?

>  }
>  
>  static void