CC hans about this one as well. Btw, this was tested on windows client (related to rhbz#1201387) and it seems to improve well. https://bugzilla.redhat.com/show_bug.cgi?id=1201387 Thanks for any input, On Fri, Oct 09, 2015 at 02:59:05PM +0200, Victor Toso wrote: > When channel wants to send much more data then the wire can handle, the > queue grows fast. This patch does not limit the queue growth but > introduces an internal API to check if queue size is too big. > > In the case of usbredir, video devices on high latency can easily > trigger this situation. > > An easy way to test locally is sharing and webcam and simulating high > latency with tc: > tc qdisc add dev lo root netem delay 100ms > tc qdisc change dev lo root netem delay 1000ms > tc qdisc del dev lo root netem > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1264156 > --- > src/channel-usbredir.c | 7 +++++++ > src/spice-channel-priv.h | 2 ++ > src/spice-channel.c | 27 ++++++++++++++++++++++++++- > 3 files changed, 35 insertions(+), 1 deletion(-) > > diff --git a/src/channel-usbredir.c b/src/channel-usbredir.c > index 89f5c9d..fb7bd07 100644 > --- a/src/channel-usbredir.c > +++ b/src/channel-usbredir.c > @@ -475,6 +475,13 @@ static void usbredir_write_flush_callback(void *user_data) > if (!priv->host) > return; > > + if (spice_channel_has_full_queue (SPICE_CHANNEL(channel))) { > + g_warning ("device %04x:%04x is loosing data due high traffic", > + spice_usb_device_get_vid(priv->spice_device), > + spice_usb_device_get_pid(priv->spice_device)); > + return; > + } > + > usbredirhost_write_guest_data(priv->host); > } > > diff --git a/src/spice-channel-priv.h b/src/spice-channel-priv.h > index 436a521..ae8d01e 100644 > --- a/src/spice-channel-priv.h > +++ b/src/spice-channel-priv.h > @@ -111,6 +111,7 @@ struct _SpiceChannelPrivate { > gboolean xmit_queue_blocked; > STATIC_MUTEX xmit_queue_lock; > guint xmit_queue_wakeup_id; > + guint64 queue_size; > > char name[16]; > enum spice_channel_state state; > @@ -171,6 +172,7 @@ void spice_channel_wakeup(SpiceChannel *channel, gboolean cancel); > > SpiceSession* spice_channel_get_session(SpiceChannel *channel); > enum spice_channel_state spice_channel_get_state(SpiceChannel *channel); > +gboolean spice_channel_has_full_queue (SpiceChannel *channel); > > /* coroutine context */ > typedef void (*handler_msg_in)(SpiceChannel *channel, SpiceMsgIn *msg, gpointer data); > diff --git a/src/spice-channel.c b/src/spice-channel.c > index 2ce52c7..d574b90 100644 > --- a/src/spice-channel.c > +++ b/src/spice-channel.c > @@ -697,10 +697,12 @@ void spice_msg_out_send(SpiceMsgOut *out) > { > SpiceChannelPrivate *c; > gboolean was_empty; > + guint32 size; > > g_return_if_fail(out != NULL); > g_return_if_fail(out->channel != NULL); > c = out->channel->priv; > + size = spice_marshaller_get_total_size(out->marshaller); > > STATIC_MUTEX_LOCK(c->xmit_queue_lock); > if (c->xmit_queue_blocked) { > @@ -710,6 +712,7 @@ void spice_msg_out_send(SpiceMsgOut *out) > > was_empty = g_queue_is_empty(&c->xmit_queue); > g_queue_push_tail(&c->xmit_queue, out); > + c->queue_size = (was_empty) ? size : c->queue_size + size; > > /* One wakeup is enough to empty the entire queue -> only do a wakeup > if the queue was empty, and there isn't one pending already. */ > @@ -2104,8 +2107,11 @@ static void spice_channel_iterate_write(SpiceChannel *channel) > STATIC_MUTEX_LOCK(c->xmit_queue_lock); > out = g_queue_pop_head(&c->xmit_queue); > STATIC_MUTEX_UNLOCK(c->xmit_queue_lock); > - if (out) > + if (out) { > + guint32 size = spice_marshaller_get_total_size(out->marshaller); > + c->queue_size = (c->queue_size < size) ? 0 : c->queue_size - size; > spice_channel_write_msg(channel, out); > + } > } while (out); > > spice_channel_flushed(channel, TRUE); > @@ -2813,6 +2819,25 @@ enum spice_channel_state spice_channel_get_state(SpiceChannel *channel) > return channel->priv->state; > } > > +#define QUEUE_MEMORY_LIMIT_MB 10 > + > +G_GNUC_INTERNAL > +gboolean spice_channel_has_full_queue (SpiceChannel *channel) > +{ > + SpiceChannelPrivate *c = channel->priv; > + gdouble ret_mb; > + guint mb_limit; > + const gchar *env = g_getenv("SPICE_CHANNEL_QUEUE_SIZE"); > + mb_limit = (env == NULL) ? QUEUE_MEMORY_LIMIT_MB : strtoul(env, NULL, 10); > + > + STATIC_MUTEX_LOCK(c->xmit_queue_lock); > + ret_mb = c->queue_size/1024.0/1024.0; > + STATIC_MUTEX_UNLOCK(c->xmit_queue_lock); > + CHANNEL_DEBUG(channel, "queue length: %u with total size: %4.4f MB (max = %u MB)", > + g_queue_get_length(&c->xmit_queue), ret_mb, mb_limit); > + return (ret_mb > mb_limit); > +} > + > G_GNUC_INTERNAL > void spice_channel_swap(SpiceChannel *channel, SpiceChannel *swap, gboolean swap_msgs) > { > -- > 2.5.0 > > _______________________________________________ > Spice-devel mailing list > Spice-devel@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/spice-devel _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel