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