On Thu, Apr 26, 2018 at 05:40:11PM +0100, Dr. David Alan Gilbert wrote:
> * Lidong Chen (jemmy858...@gmail.com) wrote:
> > Because qio_channel_rdma_writev and qio_channel_rdma_readv maybe invoked
> > by different threads concurrently, this patch removes unnecessary variables
> > len in QIOChannelRDMA and use local variable instead.
> >
> > Signed-off-by: Lidong Chen <lidongc...@tencent.com>
>
> I'm OK with this patch as is; but now you're making me worried that I
> don't quite understand what thrads are accessing it at the same time; we
> need to document/comment what's accessed concurrently.
FWIW, I have always intended that QIOChannel impls be able to support
bidirectional I/O from multiple threads, provided they each are using
it in a different direction. So this is a mistake in my original RDMA
impl.
>
> Reviewed-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
>
> > ---
> > migration/rdma.c | 15 +++++++--------
> > 1 file changed, 7 insertions(+), 8 deletions(-)
> >
> > diff --git a/migration/rdma.c b/migration/rdma.c
> > index c745427..f5c1d02 100644
> > --- a/migration/rdma.c
> > +++ b/migration/rdma.c
> > @@ -404,7 +404,6 @@ struct QIOChannelRDMA {
> > QIOChannel parent;
> > RDMAContext *rdma;
> > QEMUFile *file;
> > - size_t len;
> > bool blocking; /* XXX we don't actually honour this yet */
> > };
> >
> > @@ -2640,6 +2639,7 @@ static ssize_t qio_channel_rdma_writev(QIOChannel
> > *ioc,
> > int ret;
> > ssize_t done = 0;
> > size_t i;
> > + size_t len = 0;
> >
> > CHECK_ERROR_STATE();
> >
> > @@ -2659,10 +2659,10 @@ static ssize_t qio_channel_rdma_writev(QIOChannel
> > *ioc,
> > while (remaining) {
> > RDMAControlHeader head;
> >
> > - rioc->len = MIN(remaining, RDMA_SEND_INCREMENT);
> > - remaining -= rioc->len;
> > + len = MIN(remaining, RDMA_SEND_INCREMENT);
> > + remaining -= len;
> >
> > - head.len = rioc->len;
> > + head.len = len;
> > head.type = RDMA_CONTROL_QEMU_FILE;
> >
> > ret = qemu_rdma_exchange_send(rdma, &head, data, NULL, NULL,
> > NULL);
> > @@ -2672,8 +2672,8 @@ static ssize_t qio_channel_rdma_writev(QIOChannel
> > *ioc,
> > return ret;
> > }
> >
> > - data += rioc->len;
> > - done += rioc->len;
> > + data += len;
> > + done += len;
> > }
> > }
> >
> > @@ -2768,8 +2768,7 @@ static ssize_t qio_channel_rdma_readv(QIOChannel *ioc,
> > }
> > }
> > }
> > - rioc->len = done;
> > - return rioc->len;
> > + return done;
> > }
> >
> > /*
> > --
> > 1.8.3.1
> >
> --
> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
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 :|
