From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001

From: Yonit Halperin <yhalperi@redhat.com>

Date: Wed, 11 Sep 2013 13:31:21 -0400

Subject: [PATCH] red_channel: cleanup of red_channel_client blocking methods

(1) receive timeout as a parameter.

(2) add a return value and pass the handling

    of failures to the calling routine.

https://bugzilla.redhat.com/show_bug.cgi?id=1016795

(cherry picked from commit bcf9e64f134a6073c1e404efc8892c1cb453bd8a)

---

 server/red_channel.c | 73 ++++++++++++++++++++++++++--------------------------

 server/red_channel.h | 22 ++++++++++------

 server/red_worker.c  | 55 ++++++++++++++++++++++++++++++---------

 3 files changed, 93 insertions(+), 57 deletions(-)

diff --git a/server/red_channel.c b/server/red_channel.c

index 6e43e0a..228669b 100644

--- a/server/red_channel.c

+++ b/server/red_channel.c

@@ -48,11 +48,7 @@ typedef struct EmptyMsgPipeItem {

 #define PING_TEST_TIMEOUT_MS 15000

 #define PING_TEST_IDLE_NET_TIMEOUT_MS 100

-#define DETACH_TIMEOUT 15000000000ULL //nano

-#define DETACH_SLEEP_DURATION 10000 //micro

-

-#define CHANNEL_PUSH_TIMEOUT 30000000000ULL //nano

-#define CHANNEL_PUSH_SLEEP_DURATION 10000 //micro

+#define CHANNEL_BLOCKED_SLEEP_DURATION 10000 //micro

 enum QosPingState {

     PING_STATE_NONE,

@@ -2191,43 +2187,49 @@ uint32_t red_channel_sum_pipes_size(RedChannel *channel)

     return sum;

 }

-void red_wait_outgoing_item(RedChannelClient *rcc)

+int red_channel_client_wait_outgoing_item(RedChannelClient *rcc,

+                                          int64_t timeout)

 {

     uint64_t end_time;

     int blocked;

     if (!red_channel_client_blocked(rcc)) {

-        return;

+        return TRUE;

+    }

+    if (timeout != -1) {

+        end_time = red_now() + timeout;

     }

-    end_time = red_now() + DETACH_TIMEOUT;

     spice_info("blocked");

     do {

-        usleep(DETACH_SLEEP_DURATION);

+        usleep(CHANNEL_BLOCKED_SLEEP_DURATION);

         red_channel_client_receive(rcc);

         red_channel_client_send(rcc);

-    } while ((blocked = red_channel_client_blocked(rcc)) && red_now() < end_time);

+    } while ((blocked = red_channel_client_blocked(rcc)) &&

+             (timeout == -1 || red_now() < end_time));

     if (blocked) {

         spice_warning("timeout");

-        // TODO - shutting down the socket but we still need to trigger

-        // disconnection. Right now we wait for main channel to error for that.

-        red_channel_client_shutdown(rcc);

+        return FALSE;

     } else {

         spice_assert(red_channel_client_no_item_being_sent(rcc));

+        return TRUE;

     }

 }

 /* TODO: more evil sync stuff. anything with the word wait in it's name. */

-void red_channel_client_wait_pipe_item_sent(RedChannelClient *rcc,

-                                            PipeItem *item)

+int red_channel_client_wait_pipe_item_sent(RedChannelClient *rcc,

+                                           PipeItem *item,

+                                           int64_t timeout)

 {

     uint64_t end_time;

     int item_in_pipe;

     spice_info(NULL);

-    end_time = red_now() + CHANNEL_PUSH_TIMEOUT;

+    if (timeout != -1) {

+        end_time = red_now() + timeout;

+    }

     rcc->channel->channel_cbs.hold_item(rcc, item);

@@ -2237,55 +2239,52 @@ void red_channel_client_wait_pipe_item_sent(RedChannelClient *rcc,

     }

     red_channel_client_push(rcc);

-    while((item_in_pipe = ring_item_is_linked(&item->link)) && (red_now() < end_time)) {

-        usleep(CHANNEL_PUSH_SLEEP_DURATION);

+    while((item_in_pipe = ring_item_is_linked(&item->link)) &&

+          (timeout == -1 || red_now() < end_time)) {

+        usleep(CHANNEL_BLOCKED_SLEEP_DURATION);

         red_channel_client_receive(rcc);

         red_channel_client_send(rcc);

         red_channel_client_push(rcc);

     }

+    red_channel_client_release_item(rcc, item, TRUE);

     if (item_in_pipe) {

         spice_warning("timeout");

-        red_channel_client_disconnect(rcc);

-    } else {

-        red_wait_outgoing_item(rcc);

-    }

-    red_channel_client_release_item(rcc, item, TRUE);

-}

-

-static void rcc_shutdown_if_pending_send(RedChannelClient *rcc)

-{

-    if (red_channel_client_blocked(rcc) || rcc->pipe_size > 0) {

-        red_channel_client_shutdown(rcc);

+        return FALSE;

     } else {

-        spice_assert(red_channel_client_no_item_being_sent(rcc));

+        return red_channel_client_wait_outgoing_item(rcc,

+                                                     timeout == -1 ? -1 : end_time - red_now());

     }

 }

-void red_channel_wait_all_sent(RedChannel *channel)

+int red_channel_wait_all_sent(RedChannel *channel,

+                              int64_t timeout)

 {

     uint64_t end_time;

     uint32_t max_pipe_size;

     int blocked = FALSE;

-    end_time = red_now() + DETACH_TIMEOUT;

+    if (timeout != -1) {

+        end_time = red_now() + timeout;

+    }

     red_channel_push(channel);

     while (((max_pipe_size = red_channel_max_pipe_size(channel)) ||

            (blocked = red_channel_any_blocked(channel))) &&

-           red_now() < end_time) {

+           (timeout == -1 || red_now() < end_time)) {

         spice_debug("pipe-size %u blocked %d", max_pipe_size, blocked);

-        usleep(DETACH_SLEEP_DURATION);

+        usleep(CHANNEL_BLOCKED_SLEEP_DURATION);

         red_channel_receive(channel);

         red_channel_send(channel);

         red_channel_push(channel);

     }

     if (max_pipe_size || blocked) {

-        spice_printerr("timeout: pending out messages exist (pipe-size %u, blocked %d)",

-                       max_pipe_size, blocked);

-        red_channel_apply_clients(channel, rcc_shutdown_if_pending_send);

+        spice_warning("timeout: pending out messages exist (pipe-size %u, blocked %d)",

+                      max_pipe_size, blocked);

+        return FALSE;

     } else {

         spice_assert(red_channel_no_item_being_sent(channel));

+        return TRUE;

     }

 }

diff --git a/server/red_channel.h b/server/red_channel.h

index 9021b3f..fa11505 100644

--- a/server/red_channel.h

+++ b/server/red_channel.h

@@ -596,14 +596,20 @@ int red_client_during_migrate_at_target(RedClient *client);

 void red_client_migrate(RedClient *client);

-/* blocking function */

-void red_channel_client_wait_pipe_item_sent(RedChannelClient *rcc,

-                                            PipeItem *item);

-

-/* blocking function */

-void red_wait_outgoing_item(RedChannelClient *rcc);

+/*

+ * blocking functions.

+ *

+ * timeout is in nano sec. -1 for no timeout.

+ *

+ * Return: TRUE if waiting succeeded. FALSE if timeout expired.

+ */

-/* blocking function */

-void red_channel_wait_all_sent(RedChannel *channel);

+int red_channel_client_wait_pipe_item_sent(RedChannelClient *rcc,

+                                           PipeItem *item,

+                                           int64_t timeout);

+int red_channel_client_wait_outgoing_item(RedChannelClient *rcc,

+                                          int64_t timeout);

+int red_channel_wait_all_sent(RedChannel *channel,

+                              int64_t timeout);

 #endif

diff --git a/server/red_worker.c b/server/red_worker.c

index f2c9220..31f3cbb 100644

--- a/server/red_worker.c

+++ b/server/red_worker.c

@@ -98,6 +98,7 @@

 #define CMD_RING_POLL_TIMEOUT 10 //milli

 #define CMD_RING_POLL_RETRIES 200

+#define DISPLAY_CLIENT_SHORT_TIMEOUT 15000000000ULL //nano

 #define DISPLAY_CLIENT_TIMEOUT 30000000000ULL //nano

 #define DISPLAY_CLIENT_MIGRATE_DATA_TIMEOUT 10000000000ULL //nano, 10 sec

 #define DISPLAY_CLIENT_RETRY_INTERVAL 10000 //micro

@@ -2031,8 +2032,12 @@ static void red_current_clear(RedWorker *worker, int surface_id)

     }

 }

-static void red_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc, int surface_id,

-                                                  int wait_if_used)

+/*

+ * Return: TRUE if wait_if_used == FALSE, or otherwise, if all of the pipe items that

+ * are related to the surface have been cleared (or sent) from the pipe.

+ */

+static int red_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc, int surface_id,

+                                                 int wait_if_used)

 {

     Ring *ring;

     PipeItem *item;

@@ -2040,7 +2045,7 @@ static void red_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc, int

     RedChannelClient *rcc;

     if (!dcc) {

-        return;

+        return TRUE;

     }

     /* removing the newest drawables that their destination is surface_id and

@@ -2085,24 +2090,27 @@ static void red_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc, int

             if (wait_if_used) {

                 break;

             } else {

-                return;

+                return TRUE;

             }

         }

     }

     if (!wait_if_used) {

-        return;

+        return TRUE;

     }

     if (item) {

-        red_channel_client_wait_pipe_item_sent(&dcc->common.base, item);

+        return red_channel_client_wait_pipe_item_sent(&dcc->common.base, item,

+                                                      DISPLAY_CLIENT_TIMEOUT);

     } else {

         /*

          * in case that the pipe didn't contain any item that is dependent on the surface, but

-         * there is one during sending.

+         * there is one during sending. Use a shorter timeout, since it is just one item

          */

-        red_wait_outgoing_item(&dcc->common.base);

+        return red_channel_client_wait_outgoing_item(&dcc->common.base,

+                                                     DISPLAY_CLIENT_SHORT_TIMEOUT);

     }

+    return TRUE;

 }

 static void red_clear_surface_drawables_from_pipes(RedWorker *worker,

@@ -2113,7 +2121,9 @@ static void red_clear_surface_drawables_from_pipes(RedWorker *worker,

     DisplayChannelClient *dcc;

     WORKER_FOREACH_DCC_SAFE(worker, item, next, dcc) {

-        red_clear_surface_drawables_from_pipe(dcc, surface_id, wait_if_used);

+        if (!red_clear_surface_drawables_from_pipe(dcc, surface_id, wait_if_used)) {

+            red_channel_client_disconnect(&dcc->common.base);

+        }

     }

 }

@@ -11127,6 +11137,15 @@ void handle_dev_destroy_surface_wait(void *opaque, void *payload)

     dev_destroy_surface_wait(worker, msg->surface_id);

 }

+static void rcc_shutdown_if_pending_send(RedChannelClient *rcc)

+{

+    if (red_channel_client_blocked(rcc) || rcc->pipe_size > 0) {

+        red_channel_client_shutdown(rcc);

+    } else {

+        spice_assert(red_channel_client_no_item_being_sent(rcc));

+    }

+}

+

 static inline void red_cursor_reset(RedWorker *worker)

 {

     if (worker->cursor) {

@@ -11144,7 +11163,11 @@ static inline void red_cursor_reset(RedWorker *worker)

         if (!worker->cursor_channel->common.during_target_migrate) {

             red_pipes_add_verb(&worker->cursor_channel->common.base, SPICE_MSG_CURSOR_RESET);

         }

-        red_channel_wait_all_sent(&worker->cursor_channel->common.base);

+        if (!red_channel_wait_all_sent(&worker->cursor_channel->common.base,

+                                       DISPLAY_CLIENT_TIMEOUT)) {

+            red_channel_apply_clients(&worker->cursor_channel->common.base,

+                                      rcc_shutdown_if_pending_send);

+        }

     }

 }

@@ -11427,8 +11450,16 @@ void handle_dev_stop(void *opaque, void *payload)

      * purge the pipe, send destroy_all_surfaces

      * to the client (there is no such message right now), and start

      * from scratch on the destination side */

-    red_channel_wait_all_sent(&worker->display_channel->common.base);

-    red_channel_wait_all_sent(&worker->cursor_channel->common.base);

+    if (!red_channel_wait_all_sent(&worker->display_channel->common.base,

+                                   DISPLAY_CLIENT_TIMEOUT)) {

+        red_channel_apply_clients(&worker->display_channel->common.base,

+                                  rcc_shutdown_if_pending_send);

+    }

+    if (!red_channel_wait_all_sent(&worker->cursor_channel->common.base,

+                                   DISPLAY_CLIENT_TIMEOUT)) {

+        red_channel_apply_clients(&worker->cursor_channel->common.base,

+                                  rcc_shutdown_if_pending_send);

+    }

 }

 static int display_channel_wait_for_migrate_data(DisplayChannel *display)