[Spice-devel] [PATCH v2 6/9] server: move three functions to red_channel
Yonit Halperin
yhalperi at redhat.com
Mon Aug 12 09:07:00 PDT 2013
Hi,
I think red_channel_client_wait_pipe_item_sent should ref and unref the
pipe item by itself (using the hold/release-item callbacks). Then, you
don't need dcc_wait_pipe_item_sent.
Also, I'm not sure why PUSH_TIMEOUT is bigger than DETACH timeout. It
would have been logical if the bigger timeout was used for
red_wait_all_sent, but it isn't. But we can clean it up in another patch
series :).
Can you also s/red_wait_all_sent/red_channel_wait_all_sent?
Cheers,
Yonit.
On 08/12/2013 11:32 AM, Alon Levy wrote:
> Three blocking functions, one was split to leave the display channel
> specific referencing of the DrawablePipeItem being sent inside
> red_worker, but the rest (most) of the timeout logic was moved to
> red_channel, including the associated constants.
>
> Moved functions:
> red_channel_client_wait_pipe_item_sent
> red_wait_outgoing_item
> red_wait_all_sent
> ---
> server/red_channel.c | 104 ++++++++++++++++++++++++++++++++++++++++
> server/red_channel.h | 10 ++++
> server/red_worker.c | 131 +++++----------------------------------------------
> 3 files changed, 126 insertions(+), 119 deletions(-)
>
> diff --git a/server/red_channel.c b/server/red_channel.c
> index d565634..29d64a9 100644
> --- a/server/red_channel.c
> +++ b/server/red_channel.c
> @@ -41,6 +41,7 @@
> #include "red_channel.h"
> #include "reds.h"
> #include "main_dispatcher.h"
> +#include "red_time.h"
>
> typedef struct EmptyMsgPipeItem {
> PipeItem base;
> @@ -50,6 +51,12 @@ 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
> +
> enum QosPingState {
> PING_STATE_NONE,
> PING_STATE_TIMER,
> @@ -2195,3 +2202,100 @@ uint32_t red_channel_sum_pipes_size(RedChannel *channel)
> }
> return sum;
> }
> +
> +void red_wait_outgoing_item(RedChannelClient *rcc)
> +{
> + uint64_t end_time;
> + int blocked;
> +
> + if (!red_channel_client_blocked(rcc)) {
> + return;
> + }
> + end_time = red_now() + DETACH_TIMEOUT;
> + spice_info("blocked");
> +
> + do {
> + usleep(DETACH_SLEEP_DURATION);
> + red_channel_client_receive(rcc);
> + red_channel_client_send(rcc);
> + } while ((blocked = red_channel_client_blocked(rcc)) && 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);
> + } else {
> + spice_assert(red_channel_client_no_item_being_sent(rcc));
> + }
> +}
> +
> +
> +/* 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)
> +{
> + uint64_t end_time;
> + int item_in_pipe;
> +
> + spice_info(NULL);
> +
> + end_time = red_now() + CHANNEL_PUSH_TIMEOUT;
> +
> + if (red_channel_client_blocked(rcc)) {
> + red_channel_client_receive(rcc);
> + red_channel_client_send(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);
> + red_channel_client_receive(rcc);
> + red_channel_client_send(rcc);
> + red_channel_client_push(rcc);
> + }
> +
> + if (item_in_pipe) {
> + spice_warning("timeout");
> + red_channel_client_disconnect(rcc);
> + } else {
> + red_wait_outgoing_item(rcc);
> + }
> +}
> +
> +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));
> + }
> +}
> +
> +void red_wait_all_sent(RedChannel *channel)
> +{
> + uint64_t end_time;
> + uint32_t max_pipe_size;
> + int blocked = FALSE;
> +
> + end_time = red_now() + DETACH_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) {
> + spice_debug("pipe-size %u blocked %d", max_pipe_size, blocked);
> + usleep(DETACH_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);
> + } else {
> + spice_assert(red_channel_no_item_being_sent(channel));
> + }
> +}
> diff --git a/server/red_channel.h b/server/red_channel.h
> index 0dd73ea..b2a3a6a 100644
> --- a/server/red_channel.h
> +++ b/server/red_channel.h
> @@ -596,4 +596,14 @@ 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 function */
> +void red_wait_all_sent(RedChannel *channel);
> +
> #endif
> diff --git a/server/red_worker.c b/server/red_worker.c
> index 3b9c5b0..334a709 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -83,6 +83,7 @@
> #include "spice_timer_queue.h"
> #include "main_dispatcher.h"
> #include "spice_server_utils.h"
> +#include "red_time.h"
>
> //#define COMPRESS_STAT
> //#define DUMP_BITMAP
> @@ -103,12 +104,6 @@
> #define CMD_RING_POLL_TIMEOUT 10 //milli
> #define CMD_RING_POLL_RETRIES 200
>
> -#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 DISPLAY_CLIENT_TIMEOUT 30000000000ULL //nano
> #define DISPLAY_CLIENT_MIGRATE_DATA_TIMEOUT 10000000000ULL //nano, 10 sec
> #define DISPLAY_CLIENT_RETRY_INTERVAL 10000 //micro
> @@ -1104,10 +1099,8 @@ static void cursor_channel_client_release_item_before_push(CursorChannelClient *
> PipeItem *item);
> static void cursor_channel_client_release_item_after_push(CursorChannelClient *ccc,
> PipeItem *item);
> -static void red_wait_pipe_item_sent(RedChannelClient *rcc, PipeItem *item);
>
> static void red_push_monitors_config(DisplayChannelClient *dcc);
> -static inline uint64_t red_now(void);
>
> /*
> * Macros to make iterating over stuff easier
> @@ -2035,6 +2028,16 @@ static void red_current_clear(RedWorker *worker, int surface_id)
> }
> }
>
> +static void dcc_wait_pipe_item_sent(RedChannelClient *rcc, PipeItem *item)
> +{
> + DrawablePipeItem *dpi;
> +
> + dpi = SPICE_CONTAINEROF(item, DrawablePipeItem, dpi_pipe_item);
> + ref_drawable_pipe_item(dpi);
> + red_channel_client_wait_pipe_item_sent(rcc, item);
> + put_drawable_pipe_item(dpi);
> +}
> +
> static void red_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc, int surface_id,
> int force)
> {
> @@ -2095,12 +2098,10 @@ static void red_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc, int
> }
>
> if (item) {
> - red_wait_pipe_item_sent(&dcc->common.base, item);
> + dcc_wait_pipe_item_sent(&dcc->common.base, item);
> }
> }
>
> -static void red_wait_outgoing_item(RedChannelClient *rcc);
> -
> static void red_clear_surface_drawables_from_pipes(RedWorker *worker, int surface_id,
> int force, int wait_for_outgoing_item)
> {
> @@ -5086,15 +5087,6 @@ static void qxl_process_cursor(RedWorker *worker, RedCursorCmd *cursor_cmd, uint
> red_release_cursor(worker, cursor_item);
> }
>
> -static inline uint64_t red_now(void)
> -{
> - struct timespec time;
> -
> - clock_gettime(CLOCK_MONOTONIC, &time);
> -
> - return ((uint64_t) time.tv_sec) * 1000000000 + time.tv_nsec;
> -}
> -
> static int red_process_cursor(RedWorker *worker, uint32_t max_pipe_size, int *ring_is_empty)
> {
> QXLCommandExt ext_cmd;
> @@ -10986,105 +10978,6 @@ typedef struct __attribute__ ((__packed__)) CursorData {
> SpiceCursor _cursor;
> } CursorData;
>
> -static void red_wait_outgoing_item(RedChannelClient *rcc)
> -{
> - uint64_t end_time;
> - int blocked;
> -
> - if (!red_channel_client_blocked(rcc)) {
> - return;
> - }
> - end_time = red_now() + DETACH_TIMEOUT;
> - spice_info("blocked");
> -
> - do {
> - usleep(DETACH_SLEEP_DURATION);
> - red_channel_client_receive(rcc);
> - red_channel_client_send(rcc);
> - } while ((blocked = red_channel_client_blocked(rcc)) && 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);
> - } else {
> - spice_assert(red_channel_client_no_item_being_sent(rcc));
> - }
> -}
> -
> -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 void red_wait_all_sent(RedChannel *channel)
> -{
> - uint64_t end_time;
> - uint32_t max_pipe_size;
> - int blocked = FALSE;
> -
> - end_time = red_now() + DETACH_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) {
> - spice_debug("pipe-size %u blocked %d", max_pipe_size, blocked);
> - usleep(DETACH_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);
> - } else {
> - spice_assert(red_channel_no_item_being_sent(channel));
> - }
> -}
> -
> -/* TODO: more evil sync stuff. anything with the word wait in it's name. */
> -static void red_wait_pipe_item_sent(RedChannelClient *rcc, PipeItem *item)
> -{
> - DrawablePipeItem *dpi;
> - uint64_t end_time;
> - int item_in_pipe;
> -
> - spice_info(NULL);
> - dpi = SPICE_CONTAINEROF(item, DrawablePipeItem, dpi_pipe_item);
> - ref_drawable_pipe_item(dpi);
> -
> - end_time = red_now() + CHANNEL_PUSH_TIMEOUT;
> -
> - if (red_channel_client_blocked(rcc)) {
> - red_channel_client_receive(rcc);
> - red_channel_client_send(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);
> - red_channel_client_receive(rcc);
> - red_channel_client_send(rcc);
> - red_channel_client_push(rcc);
> - }
> -
> - if (item_in_pipe) {
> - spice_warning("timeout");
> - red_channel_client_disconnect(rcc);
> - } else {
> - red_wait_outgoing_item(rcc);
> - }
> - put_drawable_pipe_item(dpi);
> -}
> -
> static void surface_dirty_region_to_rects(RedSurface *surface,
> QXLRect *qxl_dirty_rects,
> uint32_t num_dirty_rects,
>
More information about the Spice-devel
mailing list