Re: [Spice-devel] [PATCH] server/red_worker: fix used but uninitialized warning (gcc 4.6.0)
On 02/07/2011 03:11 PM, Alon Levy wrote:
---
server/red_worker.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/server/red_worker.c b/server/red_worker.c
index f899ff2..f163a71 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -6219,7 +6219,7 @@ static inline int drawable_depends_on_areas(Drawable
*drawable,
int i;
RedDrawable *red_drawable;
int drawable_has_shadow;
-SpiceRect shadow_rect;
+SpiceRect shadow_rect = {0, 0, 0, 0};
red_drawable = drawable-red_drawable;
drawable_has_shadow = has_shadow(red_drawable);
OK.
Just a note, this does not fix spice code, but is only needed due to gcc 4.6.0
imperfection.
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 00/18] red_channel introducing refactoring, part 2, v1
Hi Alon, Gerd, While reviewing Alon patches, I started to get annoyed by a crash I didn't see before, although it seems to be an old bug, and comes from qemu perhaps? I am investing that crash before continuing the review, as it gets worst when I applied the first 3/4 patches (double free..). The crash happens under valgrind, but not under gdb, which could mean it is some kind of race. Using either spice.v30 or spice.kvm.v28 branch, and spice 0.6.3-173-g8c17f83 (current master) valgrind --db-attach=yes --smc-check=all --undef-value-errors=no /usr/local/bin/qemu-system-x86_64 --enable-kvm -net nic -net user -m 512 -localtime -device AC97 -spice port=5930,disable-ticketing -vga qxl start a client, and hit C-c to interrupt it, you get this: reds_disconnect: red_dispatcher_shutdown_cursor_peer: red_dispatcher_shutdown_peer: handle_dev_input: cursor disconnect ==4797== Invalid read of size 4 ==4797==at 0x50ABA24: red_channel_event (red_channel.c:433) ==4797==by 0x4D41CC: watch_read (spice-core.c:93) ==4797==by 0x59532E: main_loop_wait (vl.c:1349) ==4797==by 0x595551: main_loop (vl.c:1406) ==4797==by 0x599620: main (vl.c:3120) ==4797== Address 0x37205f88 is 104 bytes inside a block of size 28,608 free'd ==4797==at 0x4A05187: free (vg_replace_malloc.c:325) ==4797==by 0x50AB858: red_channel_destroy (red_channel.c:377) ==4797==by 0x508C333: main_disconnect (main_channel.c:140) ==4797==by 0x508D98B: main_channel_shutdown (main_channel.c:835) ==4797==by 0x5086264: reds_disconnect (reds.c:644) ==4797==by 0x508D6E6: main_channel_on_error (main_channel.c:759) ==4797==by 0x50AB27D: red_channel_peer_on_incoming_error (red_channel.c:205) ==4797==by 0x50AACFB: red_peer_handle_incoming (red_channel.c:85) ==4797==by 0x50AAFF6: red_channel_receive (red_channel.c:152) ==4797==by 0x50ABA15: red_channel_event (red_channel.c:429) ==4797==by 0x4D41CC: watch_read (spice-core.c:93) ==4797==by 0x59532E: main_loop_wait (vl.c:1349) ==4797== ==4797== If you try to reproduce the bug with gdb, by breaking first on red_channel_destroy and then on red_channel_event, you can see that red_channel_event is not called. Help appreciated! regards On Mon, Feb 7, 2011 at 7:19 PM, Alon Levy al...@redhat.com wrote: This is a part of a much larger series so I don't have a good concise description. Ultimately this all leads to a reuse of RedChannel across most of the channels (sound remains an odd out because of not using a pipe abstraction, but a bitmap of outstanding packets, you can have 0 or one of each type waiting to be sent, this makes sense for audio. Probably for video too actually). Anyway this patchset touches almost only red_worker (and it's helper cache files, split out because they are used for a number of different caches via macros before inclusion, that usual trick). It is mostly about renaming, making a CommonChannel take the place of the internal RedChannel to allow future use of the external RedChannel defined in red_channel.h Alon Levy (18): server/red_worker: change hold_item sig, drop the void* server/red_worker: use ack_data struct server/red_worker: introduce CommonChannel server/red_worker: add free cb to EventHandler server/red_worker: shorten some lines using alias variables server/red_worker: use red_channel pipe add versions server/red_worker: extract common_release_pipe_item from red_pipe_clear server/red_worker: split display_channel_send_item server/red_worker: add red_channel_init_send_data server/red_worker: use red_channel begin_send_message server/red_worker: small cleanup with worker alias server/red_worker: split cursor_channel_send_item server/red_worker: s/disconnect_channel_proc/channel_disconnect_proc/ server/red_worker: s/hold_pipe_item_proc/channel_hold_pipe_item/ server/red_worker: s/handle_message_proc/handle_parsed_proc/ server/red_worker: introduce an outgoing struct around out_bytes_counter server/red_worker: s/release_item_proc/channel_release_pipe_item_proc/ server/red_worker: match channel_release_pipe_item_proc to red_channel server/red_client_cache.h | 2 +- server/red_client_shared_cache.h | 20 +- server/red_worker.c | 1018 -- 3 files changed, 553 insertions(+), 487 deletions(-) -- 1.7.4 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel -- Marc-André Lureau ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 00/18] red_channel introducing refactoring, part 2, v1
Hi On Tue, Feb 8, 2011 at 7:47 PM, Marc-André Lureau marcandre.lur...@gmail.com wrote: Hi Alon, Gerd, While reviewing Alon patches, I started to get annoyed by a crash I didn't see before, although it seems to be an old bug, and comes from qemu perhaps? I am investing that crash before continuing the review, as it gets worst when I applied the first 3/4 patches (double free..). Actually, I was confused by something that Alon fixed in patch 04, so I'll send the review anyway, as the patch series doesn't affect this race. cheers -- Marc-André Lureau ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] server/red_worker: fix used but uninitialized warning (gcc 4.6.0)
On Tue, Feb 08, 2011 at 08:02:51PM +0200, Uri Lublin wrote:
On 02/07/2011 03:11 PM, Alon Levy wrote:
---
server/red_worker.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/server/red_worker.c b/server/red_worker.c
index f899ff2..f163a71 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -6219,7 +6219,7 @@ static inline int drawable_depends_on_areas(Drawable
*drawable,
int i;
RedDrawable *red_drawable;
int drawable_has_shadow;
-SpiceRect shadow_rect;
+SpiceRect shadow_rect = {0, 0, 0, 0};
red_drawable = drawable-red_drawable;
drawable_has_shadow = has_shadow(red_drawable);
OK.
Just a note, this does not fix spice code, but is only needed due to
gcc 4.6.0 imperfection.
Right, should have mentioned this - absolutely right, that variable only
gets used if it is initialized, which gcc 4.6.0 fails to notice. I could
probably file a bug on gcc :)
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 18/18] server/red_worker: match channel_release_pipe_item_proc to red_channel
Ack. I don't see the need for item_pushed argument, is it used later
in your patch series?
On Mon, Feb 7, 2011 at 7:20 PM, Alon Levy al...@redhat.com wrote:
---
server/red_worker.c | 19 ++-
1 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/server/red_worker.c b/server/red_worker.c
index 8c39d09..50038bf 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -350,7 +350,7 @@ typedef struct LocalCursor {
typedef struct RedChannel RedChannel;
typedef void (*channel_disconnect_proc)(RedChannel *channel);
typedef void (*channel_hold_pipe_item_proc)(PipeItem *item);
-typedef void (*channel_release_pipe_item_proc)(RedChannel *channel, void
*item);
+typedef void (*channel_release_pipe_item_proc)(RedChannel *channel, PipeItem
*item, int item_pushed);
typedef int (*handle_parsed_proc)(RedChannel *channel, uint32_t size,
uint16_t type, void *message);
struct RedChannel {
@@ -1845,7 +1845,8 @@ static void
red_clear_surface_drawables_from_pipe(RedWorker *worker, int surface
PipeItem *tmp_item = item;
item = (PipeItem *)ring_prev(ring, (RingItem *)item);
ring_remove(tmp_item-link);
-
worker-display_channel-common.base.release_item(worker-display_channel-common.base,
tmp_item);
+ worker-display_channel-common.base.release_item(
+ worker-display_channel-common.base, tmp_item, FALSE);
worker-display_channel-common.base.pipe_size--;
if (!item) {
@@ -7331,7 +7332,7 @@ static void inline channel_release_res(RedChannel
*channel)
if (!channel-send_data.item) {
return;
}
- channel-release_item(channel, channel-send_data.item);
+ channel-release_item(channel, channel-send_data.item, FALSE);
channel-send_data.item = NULL;
}
@@ -7345,7 +7346,7 @@ static void red_send_data(RedChannel *channel)
if (!n) {
channel-send_data.blocked = FALSE;
if (channel-send_data.item) {
- channel-release_item(channel, channel-send_data.item);
+ channel-release_item(channel, channel-send_data.item,
FALSE);
channel-send_data.item = NULL;
}
break;
@@ -9458,12 +9459,12 @@ static void display_channel_hold_pipe_item(PipeItem
*item)
}
}
-static void display_channel_release_item(RedChannel *channel, void *item)
+static void display_channel_release_item(RedChannel *channel, PipeItem
*item, int item_pushed /* ignored */)
{
CommonChannel *common = SPICE_CONTAINEROF(channel, CommonChannel, base);
ASSERT(item);
- switch (((PipeItem *)item)-type) {
+ switch (item-type) {
case PIPE_ITEM_TYPE_DRAW:
case PIPE_ITEM_TYPE_STREAM_CREATE:
release_drawable(common-worker, SPICE_CONTAINEROF(item, Drawable,
pipe_item));
@@ -9601,12 +9602,12 @@ static void cursor_channel_hold_pipe_item(PipeItem
*item)
((CursorItem *)item)-refs++;
}
-static void cursor_channel_release_item(RedChannel *channel, void *item)
+static void cursor_channel_release_item(RedChannel *channel, PipeItem *item,
int item_pushed)
{
CommonChannel *common = SPICE_CONTAINEROF(channel, CommonChannel, base);
ASSERT(item);
- red_release_cursor(common-worker, item);
+ red_release_cursor(common-worker, SPICE_CONTAINEROF(item, CursorItem,
pipe_data));
}
static void red_connect_cursor(RedWorker *worker, RedsStreamContext *peer,
int migrate)
@@ -9755,7 +9756,7 @@ static void red_wait_pipe_item_sent(RedChannel
*channel, PipeItem *item)
}
}
- channel-release_item(channel, item);
+ channel-release_item(channel, item, FALSE);
red_unref_channel(channel);
}
--
1.7.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
--
Marc-André Lureau
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 14/18] server/red_worker: s/hold_pipe_item_proc/channel_hold_pipe_item/
please merge with patch 13, it makes bisection faster (reviewing is
easy for this kind of renaming change, no need to split it).
On Mon, Feb 7, 2011 at 7:20 PM, Alon Levy al...@redhat.com wrote:
---
server/red_worker.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/server/red_worker.c b/server/red_worker.c
index 355e073..5c02518 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -349,7 +349,7 @@ typedef struct LocalCursor {
typedef struct RedChannel RedChannel;
typedef void (*channel_disconnect_proc)(RedChannel *channel);
-typedef void (*hold_pipe_item_proc)(PipeItem *item);
+typedef void (*channel_hold_pipe_item_proc)(PipeItem *item);
typedef void (*release_item_proc)(RedChannel *channel, void *item);
typedef int (*handle_message_proc)(RedChannel *channel, size_t size,
uint32_t type, void *message);
@@ -386,7 +386,7 @@ struct RedChannel {
} recive_data;
channel_disconnect_proc disconnect;
- hold_pipe_item_proc hold_item;
+ channel_hold_pipe_item_proc hold_item;
release_item_proc release_item;
handle_message_proc handle_message;
#ifdef RED_STATISTICS
@@ -9349,7 +9349,7 @@ static RedChannel *__new_channel(RedWorker *worker, int
size, uint32_t channel_i
RedsStreamContext *peer, int migrate,
event_listener_action_proc handler,
channel_disconnect_proc disconnect,
- hold_pipe_item_proc hold_item,
+ channel_hold_pipe_item_proc hold_item,
release_item_proc release_item,
handle_message_proc handle_message)
{
--
1.7.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
--
Marc-André Lureau
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 17/18] server/red_worker: s/release_item_proc/channel_release_pipe_item_proc/
to merge with patch 13
On Mon, Feb 7, 2011 at 7:20 PM, Alon Levy al...@redhat.com wrote:
---
server/red_worker.c | 7 ---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/server/red_worker.c b/server/red_worker.c
index 0393d77..8c39d09 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -350,7 +350,7 @@ typedef struct LocalCursor {
typedef struct RedChannel RedChannel;
typedef void (*channel_disconnect_proc)(RedChannel *channel);
typedef void (*channel_hold_pipe_item_proc)(PipeItem *item);
-typedef void (*release_item_proc)(RedChannel *channel, void *item);
+typedef void (*channel_release_pipe_item_proc)(RedChannel *channel, void
*item);
typedef int (*handle_parsed_proc)(RedChannel *channel, uint32_t size,
uint16_t type, void *message);
struct RedChannel {
@@ -387,8 +387,9 @@ struct RedChannel {
channel_disconnect_proc disconnect;
channel_hold_pipe_item_proc hold_item;
- release_item_proc release_item;
+ channel_release_pipe_item_proc release_item;
handle_parsed_proc handle_message;
+
#ifdef RED_STATISTICS
struct {
uint64_t *out_bytes_counter;
@@ -9352,7 +9353,7 @@ static RedChannel *__new_channel(RedWorker *worker, int
size, uint32_t channel_i
event_listener_action_proc handler,
channel_disconnect_proc disconnect,
channel_hold_pipe_item_proc hold_item,
- release_item_proc release_item,
+ channel_release_pipe_item_proc release_item,
handle_parsed_proc handle_message)
{
struct epoll_event event;
--
1.7.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
--
Marc-André Lureau
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 13/18] server/red_worker: s/disconnect_channel_proc/channel_disconnect_proc/
hmm, again, it's obvious that there is a problem of naming and
consistency in the code. Just modifying one makes perhaps things
worst.. what about? :
hold_pipe_item_proc - pipe_item_hold_proc
release_item_proc - channel_release_item_proc
handle_message_proc - channel_handle_message_proc
So, I see you do rename some of them in the following patch. I would
squash all those rename in this patch.
On Mon, Feb 7, 2011 at 7:20 PM, Alon Levy al...@redhat.com wrote:
---
server/red_worker.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/server/red_worker.c b/server/red_worker.c
index 1574b99..355e073 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -348,7 +348,7 @@ typedef struct LocalCursor {
#define PALETTE_CACHE_HASH_KEY(id) ((id) PALETTE_CACHE_HASH_MASK)
typedef struct RedChannel RedChannel;
-typedef void (*disconnect_channel_proc)(RedChannel *channel);
+typedef void (*channel_disconnect_proc)(RedChannel *channel);
typedef void (*hold_pipe_item_proc)(PipeItem *item);
typedef void (*release_item_proc)(RedChannel *channel, void *item);
typedef int (*handle_message_proc)(RedChannel *channel, size_t size,
uint32_t type, void *message);
@@ -385,7 +385,7 @@ struct RedChannel {
uint8_t *end;
} recive_data;
- disconnect_channel_proc disconnect;
+ channel_disconnect_proc disconnect;
hold_pipe_item_proc hold_item;
release_item_proc release_item;
handle_message_proc handle_message;
@@ -9348,7 +9348,7 @@ static void
free_common_channel_from_listener(EventListener *ctx)
static RedChannel *__new_channel(RedWorker *worker, int size, uint32_t
channel_id,
RedsStreamContext *peer, int migrate,
event_listener_action_proc handler,
- disconnect_channel_proc disconnect,
+ channel_disconnect_proc disconnect,
hold_pipe_item_proc hold_item,
release_item_proc release_item,
handle_message_proc handle_message)
--
1.7.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
--
Marc-André Lureau
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 16/18] server/red_worker: introduce an outgoing struct around out_bytes_counter
Ok, I would get rid of out_ prefix in out_bytes_counter.
What's the motivation for this change?
On Mon, Feb 7, 2011 at 7:20 PM, Alon Levy al...@redhat.com wrote:
---
server/red_worker.c | 10 ++
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/server/red_worker.c b/server/red_worker.c
index 70ec871..0393d77 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -390,7 +390,9 @@ struct RedChannel {
release_item_proc release_item;
handle_parsed_proc handle_message;
#ifdef RED_STATISTICS
- uint64_t *out_bytes_counter;
+ struct {
+ uint64_t *out_bytes_counter;
+ } outgoing;
#endif
};
@@ -7367,7 +7369,7 @@ static void red_send_data(RedChannel *channel)
}
} else {
channel-send_data.pos += n;
- stat_inc_counter(channel-out_bytes_counter, n);
+ stat_inc_counter(channel-outgoing.out_bytes_counter, n);
}
}
}
@@ -9497,7 +9499,7 @@ static void handle_new_display_channel(RedWorker
*worker, RedsStreamContext *pee
}
#ifdef RED_STATISTICS
display_channel-stat = stat_add_node(worker-stat, display_channel,
TRUE);
- display_channel-common.base.out_bytes_counter =
stat_add_counter(display_channel-stat,
+ display_channel-common.base.outgoing.out_bytes_counter =
stat_add_counter(display_channel-stat,
out_bytes,
TRUE);
display_channel-cache_hits_counter =
stat_add_counter(display_channel-stat,
cache_hits,
TRUE);
@@ -9623,7 +9625,7 @@ static void red_connect_cursor(RedWorker *worker,
RedsStreamContext *peer, int m
}
#ifdef RED_STATISTICS
channel-stat = stat_add_node(worker-stat, cursor_channel, TRUE);
- channel-common.base.out_bytes_counter = stat_add_counter(channel-stat,
out_bytes, TRUE);
+ channel-common.base.outgoing.out_bytes_counter =
stat_add_counter(channel-stat, out_bytes, TRUE);
#endif
ring_init(channel-cursor_cache_lru);
channel-cursor_cache_available = CLIENT_CURSOR_CACHE_SIZE;
--
1.7.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
--
Marc-André Lureau
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 09/18] server/red_worker: add red_channel_init_send_data
ack
On Mon, Feb 7, 2011 at 7:19 PM, Alon Levy al...@redhat.com wrote:
Changes semantics of send to always hold/release regardless of block, like
red_channel. A hold is just a reference count increment or nop.
---
server/red_worker.c | 171
+--
1 files changed, 84 insertions(+), 87 deletions(-)
diff --git a/server/red_worker.c b/server/red_worker.c
index 2e3dc16..393543a 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -976,8 +976,8 @@ static void red_display_release_stream(DisplayChannel
*display, StreamAgent *age
static inline void red_detach_stream(RedWorker *worker, Stream *stream);
static void red_stop_stream(RedWorker *worker, Stream *stream);
static inline void red_stream_maintenance(RedWorker *worker, Drawable
*candidate, Drawable *sect);
-static inline void red_begin_send_message(RedChannel *channel, void *item);
-static inline void display_begin_send_message(DisplayChannel *channel, void
*item);
+static inline void red_begin_send_message(RedChannel *channel);
+static inline void display_begin_send_message(DisplayChannel *channel);
static void red_receive(RedChannel *channel);
static void red_release_pixmap_cache(DisplayChannel *channel);
static void red_release_glz(DisplayChannel *channel);
@@ -1176,6 +1176,16 @@ static void show_draw_item(RedWorker *worker, DrawItem
*draw_item, const char *p
draw_item-base.rgn.extents.y2);
}
+static void red_channel_init_send_data(RedChannel *channel, uint16_t type,
PipeItem *item)
+{
+ if (item) {
+ channel-hold_item(item);
+ ASSERT(channel-send_data.item == NULL);
+ channel-send_data.item = item;
+ }
+ channel-send_data.header-type = type;
+}
+
static inline void red_pipe_item_init(PipeItem *item, int type)
{
ring_item_init(item-link);
@@ -6449,9 +6459,8 @@ static void red_send_qxl_draw_fill(RedWorker *worker,
SpiceMarshaller *mask_bitmap_out;
SpiceFill fill;
+ red_channel_init_send_data(channel, SPICE_MSG_DISPLAY_DRAW_FILL,
item-pipe_item);
fill_base(display_channel, item);
-
- channel-send_data.header-type = SPICE_MSG_DISPLAY_DRAW_FILL;
fill = drawable-u.fill;
spice_marshall_Fill(channel-send_data.marshaller,
fill,
@@ -6498,7 +6507,7 @@ static void red_lossy_send_qxl_draw_fill(RedWorker
*worker,
red_send_qxl_draw_fill(worker, display_channel, item);
- // eitehr the brush operation is opaque, or the dest is not lossy
+ // either the brush operation is opaque, or the dest is not lossy
surface_lossy_region_update(worker, display_channel, item, has_mask,
FALSE);
} else {
int resend_surface_ids[2];
@@ -6534,9 +6543,8 @@ static FillBitsType red_send_qxl_draw_opaque(RedWorker
*worker,
SpiceOpaque opaque;
FillBitsType src_send_type;
+ red_channel_init_send_data(channel, SPICE_MSG_DISPLAY_DRAW_OPAQUE,
item-pipe_item);
fill_base(display_channel, item);
-
- channel-send_data.header-type = SPICE_MSG_DISPLAY_DRAW_OPAQUE;
opaque = drawable-u.opaque;
spice_marshall_Opaque(channel-send_data.marshaller,
opaque,
@@ -6630,10 +6638,8 @@ static FillBitsType red_send_qxl_draw_copy(RedWorker
*worker,
SpiceCopy copy;
FillBitsType src_send_type;
+ red_channel_init_send_data(channel, SPICE_MSG_DISPLAY_DRAW_COPY,
item-pipe_item);
fill_base(display_channel, item);
-
- channel-send_data.header-type = SPICE_MSG_DISPLAY_DRAW_COPY;
-
copy = drawable-u.copy;
spice_marshall_Copy(channel-send_data.marshaller,
copy,
@@ -6680,8 +6686,8 @@ static void red_send_qxl_draw_transparent(RedWorker
*worker,
SpiceMarshaller *src_bitmap_out;
SpiceTransparent transparent;
+ red_channel_init_send_data(channel, SPICE_MSG_DISPLAY_DRAW_TRANSPARENT,
item-pipe_item);
fill_base(display_channel, item);
- channel-send_data.header-type = SPICE_MSG_DISPLAY_DRAW_TRANSPARENT;
transparent = drawable-u.transparent;
spice_marshall_Transparent(channel-send_data.marshaller,
transparent,
@@ -6727,8 +6733,8 @@ static FillBitsType
red_send_qxl_draw_alpha_blend(RedWorker *worker,
SpiceAlphaBlend alpha_blend;
FillBitsType src_send_type;
+ red_channel_init_send_data(channel, SPICE_MSG_DISPLAY_DRAW_ALPHA_BLEND,
item-pipe_item);
fill_base(display_channel, item);
- channel-send_data.header-type = SPICE_MSG_DISPLAY_DRAW_ALPHA_BLEND;
alpha_blend = drawable-u.alpha_blend;
spice_marshall_AlphaBlend(channel-send_data.marshaller,
alpha_blend,
@@ -6771,8 +6777,8 @@ static void red_send_qxl_copy_bits(RedWorker *worker,
RedDrawable *drawable = item-red_drawable;
SpicePoint copy_bits;
+ red_channel_init_send_data(channel, SPICE_MSG_DISPLAY_COPY_BITS,
item-pipe_item);
Re: [Spice-devel] [PATCH 07/18] server/red_worker: extract common_release_pipe_item from red_pipe_clear
Hopefully we can find a more elegant solution to the downcasting
introduced in patch 03: CommonChannel *common =
SPICE_CONTAINEROF(channel, CommonChannel, base);
I would rather modify the function common_release_pipe_item(RedChannel
*channel, PipeItem *item) to be:
common_release_pipe_item(CommonChannel *common, PipeItem *item).
On Mon, Feb 7, 2011 at 7:19 PM, Alon Levy al...@redhat.com wrote:
---
server/red_worker.c | 106
+++
1 files changed, 56 insertions(+), 50 deletions(-)
diff --git a/server/red_worker.c b/server/red_worker.c
index 76d0ef6..08aeef2 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -1354,63 +1354,69 @@ static void release_upgrade_item(RedWorker* worker,
UpgradeItem *item)
}
}
+static void common_release_pipe_item(RedChannel *channel, PipeItem *item)
+{
+ CommonChannel *common = SPICE_CONTAINEROF(channel, CommonChannel, base);
+
+ switch (item-type) {
+ case PIPE_ITEM_TYPE_DRAW:
+ release_drawable(common-worker, SPICE_CONTAINEROF(item, Drawable,
pipe_item));
+ break;
+ case PIPE_ITEM_TYPE_CURSOR:
+ red_release_cursor(common-worker, (CursorItem *)item);
+ break;
+ case PIPE_ITEM_TYPE_UPGRADE:
+ release_upgrade_item(common-worker, (UpgradeItem *)item);
+ break;
+ case PIPE_ITEM_TYPE_PIXMAP_RESET:
+ case PIPE_ITEM_TYPE_PIXMAP_SYNC:
+ case PIPE_ITEM_TYPE_INVAL_ONE:
+ case PIPE_ITEM_TYPE_MIGRATE:
+ case PIPE_ITEM_TYPE_SET_ACK:
+ case PIPE_ITEM_TYPE_CURSOR_INIT:
+ case PIPE_ITEM_TYPE_VERB:
+ case PIPE_ITEM_TYPE_MIGRATE_DATA:
+ case PIPE_ITEM_TYPE_INVAL_PALLET_CACHE:
+ case PIPE_ITEM_TYPE_INVAL_CURSOR_CACHE:
+ free(item);
+ break;
+ case PIPE_ITEM_TYPE_IMAGE:
+ release_image_item((ImageItem *)item);
+ break;
+ case PIPE_ITEM_TYPE_STREAM_CREATE:
+ red_display_release_stream((DisplayChannel *)channel,
+ SPICE_CONTAINEROF(item, StreamAgent,
create_item));
+ break;
+ case PIPE_ITEM_TYPE_STREAM_CLIP:
+ red_display_release_stream_clip((DisplayChannel *)channel,
(StreamClipItem*)item);
+ break;
+ case PIPE_ITEM_TYPE_STREAM_DESTROY:
+ red_display_release_stream((DisplayChannel *)channel,
+ SPICE_CONTAINEROF(item, StreamAgent,
destroy_item));
+ break;
+ case PIPE_ITEM_TYPE_CREATE_SURFACE: {
+ SurfaceCreateItem *surface_create = SPICE_CONTAINEROF(item,
SurfaceCreateItem,
+ pipe_item);
+ free(surface_create);
+ break;
+ }
+ case PIPE_ITEM_TYPE_DESTROY_SURFACE: {
+ SurfaceDestroyItem *surface_destroy = SPICE_CONTAINEROF(item,
SurfaceDestroyItem,
+ pipe_item);
+ free(surface_destroy);
+ break;
+ }
+ }
+}
+
static void red_pipe_clear(RedChannel *channel)
{
PipeItem *item;
- CommonChannel *common = SPICE_CONTAINEROF(channel, CommonChannel, base);
ASSERT(channel);
while ((item = (PipeItem *)ring_get_head(channel-pipe))) {
ring_remove(item-link);
- switch (item-type) {
- case PIPE_ITEM_TYPE_DRAW:
- release_drawable(common-worker, SPICE_CONTAINEROF(item,
Drawable, pipe_item));
- break;
- case PIPE_ITEM_TYPE_CURSOR:
- red_release_cursor(common-worker, (CursorItem *)item);
- break;
- case PIPE_ITEM_TYPE_UPGRADE:
- release_upgrade_item(common-worker, (UpgradeItem *)item);
- break;
- case PIPE_ITEM_TYPE_PIXMAP_RESET:
- case PIPE_ITEM_TYPE_PIXMAP_SYNC:
- case PIPE_ITEM_TYPE_INVAL_ONE:
- case PIPE_ITEM_TYPE_MIGRATE:
- case PIPE_ITEM_TYPE_SET_ACK:
- case PIPE_ITEM_TYPE_CURSOR_INIT:
- case PIPE_ITEM_TYPE_VERB:
- case PIPE_ITEM_TYPE_MIGRATE_DATA:
- case PIPE_ITEM_TYPE_INVAL_PALLET_CACHE:
- case PIPE_ITEM_TYPE_INVAL_CURSOR_CACHE:
- free(item);
- break;
- case PIPE_ITEM_TYPE_IMAGE:
- release_image_item((ImageItem *)item);
- break;
- case PIPE_ITEM_TYPE_STREAM_CREATE:
- red_display_release_stream((DisplayChannel *)channel,
- SPICE_CONTAINEROF(item, StreamAgent,
create_item));
- break;
- case PIPE_ITEM_TYPE_STREAM_CLIP:
- red_display_release_stream_clip((DisplayChannel *)channel,
(StreamClipItem*)item);
- break;
- case PIPE_ITEM_TYPE_STREAM_DESTROY:
- red_display_release_stream((DisplayChannel *)channel,
- SPICE_CONTAINEROF(item, StreamAgent,
destroy_item));
- break;
- case PIPE_ITEM_TYPE_CREATE_SURFACE: {
-
Re: [Spice-devel] [PATCH 01/18] server/red_worker: change hold_item sig, drop the void*
ack
Btw, I realize that in the previous patch series, we had
index e8ebb05..a778ffb 100644
--- a/server/red_channel.h
+++ b/server/red_channel.h
@@ -107,6 +107,7 @@ typedef void
(*channel_release_msg_recv_buf_proc)(RedChannel *channel,
typedef void (*channel_disconnect_proc)(RedChannel *channel);
typedef int (*channel_configure_socket_proc)(RedChannel *channel);
typedef void (*channel_send_pipe_item_proc)(RedChannel *channel,
PipeItem *item);
+typedef void (*channel_hold_pipe_item_proc)(PipeItem *item);
It would be more consistent to have:
+typedef void (*channel_hold_pipe_item_proc)(RedChannel *channel,
PipeItem *item);
On Mon, Feb 7, 2011 at 7:19 PM, Alon Levy al...@redhat.com wrote:
changed to PipeItem *
---
server/red_worker.c | 8
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/server/red_worker.c b/server/red_worker.c
index 2acef40..df51841 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -347,7 +347,7 @@ typedef struct LocalCursor {
typedef struct RedChannel RedChannel;
typedef void (*disconnect_channel_proc)(RedChannel *channel);
-typedef void (*hold_pipe_item_proc)(void *item);
+typedef void (*hold_pipe_item_proc)(PipeItem *item);
typedef void (*release_item_proc)(RedChannel *channel, void *item);
typedef int (*handle_message_proc)(RedChannel *channel, size_t size,
uint32_t type, void *message);
@@ -9389,10 +9389,10 @@ static void handle_channel_events(EventListener
*in_listener, uint32_t events)
}
}
-static void display_channel_hold_pipe_item(void *item)
+static void display_channel_hold_pipe_item(PipeItem *item)
{
ASSERT(item);
- switch (((PipeItem *)item)-type) {
+ switch (item-type) {
case PIPE_ITEM_TYPE_DRAW:
case PIPE_ITEM_TYPE_STREAM_CREATE:
SPICE_CONTAINEROF(item, Drawable, pipe_item)-refs++;
@@ -9544,7 +9544,7 @@ static void on_new_cursor_channel(RedWorker *worker)
}
}
-static void cursor_channel_hold_pipe_item(void *item)
+static void cursor_channel_hold_pipe_item(PipeItem *item)
{
ASSERT(item);
((CursorItem *)item)-refs++;
--
1.7.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
--
Marc-André Lureau
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 02/18] server/red_worker: use ack_data struct
ack
On Mon, Feb 7, 2011 at 7:19 PM, Alon Levy al...@redhat.com wrote:
start of move to red_channel based channels
---
server/red_worker.c | 38 --
1 files changed, 20 insertions(+), 18 deletions(-)
diff --git a/server/red_worker.c b/server/red_worker.c
index df51841..97f5e70 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -362,10 +362,12 @@ struct RedChannel {
Ring pipe;
uint32_t pipe_size;
- uint32_t client_ack_window;
- uint32_t ack_generation;
- uint32_t client_ack_generation;
- uint32_t messages_window;
+ struct {
+ uint32_t client_window;
+ uint32_t generation;
+ uint32_t client_generation;
+ uint32_t messages_window;
+ } ack_data;
struct {
int blocked;
@@ -7386,7 +7388,7 @@ static inline void red_begin_send_message(RedChannel
*channel, void *item)
spice_marshaller_flush(channel-send_data.marshaller);
channel-send_data.size =
spice_marshaller_get_total_size(channel-send_data.marshaller);
channel-send_data.header-size = channel-send_data.size -
sizeof(SpiceDataHeader);
- channel-messages_window++;
+ channel-ack_data.messages_window++;
channel-send_data.header = NULL; /* avoid writing to this until we have
a new message */
red_send_data(channel, item);
}
@@ -7742,9 +7744,9 @@ static void red_send_set_ack(RedChannel *channel)
ASSERT(channel);
channel-send_data.header-type = SPICE_MSG_SET_ACK;
- ack.generation = ++channel-ack_generation;
- ack.window = channel-client_ack_window;
- channel-messages_window = 0;
+ ack.generation = ++channel-ack_data.generation;
+ ack.window = channel-ack_data.client_window;
+ channel-ack_data.messages_window = 0;
spice_marshall_msg_set_ack(channel-send_data.marshaller, ack);
@@ -8274,7 +8276,7 @@ static inline PipeItem *red_pipe_get(RedChannel
*channel)
return NULL;
}
- if (channel-messages_window channel-client_ack_window * 2) {
+ if (channel-ack_data.messages_window channel-ack_data.client_window
* 2) {
channel-send_data.blocked = TRUE;
return NULL;
}
@@ -8896,7 +8898,7 @@ static void on_new_display_channel(RedWorker *worker)
if (!display_channel_wait_for_init(display_channel)) {
return;
}
- display_channel-base.messages_window = 0;
+ display_channel-base.ack_data.messages_window = 0;
if (worker-surfaces[0].context.canvas) {
red_current_flush(worker, 0);
push_new_primary_surface(worker);
@@ -8912,11 +8914,11 @@ static int channel_handle_message(RedChannel
*channel, size_t size, uint32_t typ
{
switch (type) {
case SPICE_MSGC_ACK_SYNC:
- channel-client_ack_generation = *(uint32_t *)message;
+ channel-ack_data.client_generation = *(uint32_t *)message;
break;
case SPICE_MSGC_ACK:
- if (channel-client_ack_generation == channel-ack_generation) {
- channel-messages_window -= channel-client_ack_window;
+ if (channel-ack_data.client_generation ==
channel-ack_data.generation) {
+ channel-ack_data.messages_window -=
channel-ack_data.client_window;
}
break;
case SPICE_MSGC_DISCONNECTING:
@@ -9211,7 +9213,7 @@ static int
display_channel_handle_migrate_data(DisplayChannel *channel, size_t s
red_pipe_add_type((RedChannel *)channel,
PIPE_ITEM_TYPE_INVAL_PALLET_CACHE);
- channel-base.messages_window = 0;
+ channel-base.ack_data.messages_window = 0;
return TRUE;
}
@@ -9346,11 +9348,11 @@ static RedChannel *__new_channel(RedWorker *worker,
int size, uint32_t channel_i
channel-handle_message = handle_message;
channel-peer = peer;
channel-worker = worker;
- channel-messages_window = ~0; // blocks send message (maybe use
send_data.blocked +
+ channel-ack_data.messages_window = ~0; // blocks send message (maybe
use send_data.blocked +
// block flags)
- channel-client_ack_window = IS_LOW_BANDWIDTH() ? WIDE_CLIENT_ACK_WINDOW
:
+ channel-ack_data.client_window = IS_LOW_BANDWIDTH() ?
WIDE_CLIENT_ACK_WINDOW :
NARROW_CLIENT_ACK_WINDOW;
- channel-client_ack_generation = ~0;
+ channel-ack_data.client_generation = ~0;
channel-recive_data.message = (SpiceDataHeader
*)channel-recive_data.buf;
channel-recive_data.now = channel-recive_data.buf;
channel-recive_data.end = channel-recive_data.buf +
sizeof(channel-recive_data.buf);
@@ -9537,7 +9539,7 @@ static void on_new_cursor_channel(RedWorker *worker)
ASSERT(channel);
- channel-base.messages_window = 0;
+ channel-base.ack_data.messages_window = 0;
red_pipe_add_type(channel-base, PIPE_ITEM_TYPE_SET_ACK);
if (worker-surfaces[0].context.canvas !channel-base.migrate) {
Re: [Spice-devel] [PATCH 04/18] server/red_worker: add free cb to EventHandler
Please merge with patch 03 (keep the comment though)
On Mon, Feb 7, 2011 at 7:19 PM, Alon Levy al...@redhat.com wrote:
Added cb takes care of non zero offset embedded EventHandler, which happens
now with the introduced CommonChannel.
---
server/red_worker.c | 19 ++-
1 files changed, 18 insertions(+), 1 deletions(-)
diff --git a/server/red_worker.c b/server/red_worker.c
index ba04a72..0ed46e9 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -218,9 +218,11 @@ double inline stat_byte_to_mega(uint64_t size)
typedef struct EventListener EventListener;
typedef void (*event_listener_action_proc)(EventListener *ctx, uint32_t
events);
+typedef void (*event_listener_free_proc)(EventListener *ctx);
struct EventListener {
uint32_t refs;
event_listener_action_proc action;
+ event_listener_free_proc free;
};
enum {
@@ -9319,6 +9321,13 @@ static void red_receive(RedChannel *channel)
}
}
+static void free_common_channel_from_listener(EventListener *ctx)
+{
+ CommonChannel* common = SPICE_CONTAINEROF(ctx, CommonChannel, listener);
+
+ free(common);
+}
+
static RedChannel *__new_channel(RedWorker *worker, int size, uint32_t
channel_id,
RedsStreamContext *peer, int migrate,
event_listener_action_proc handler,
@@ -9356,6 +9365,7 @@ static RedChannel *__new_channel(RedWorker *worker, int
size, uint32_t channel_i
channel-parser = spice_get_client_channel_parser(channel_id, NULL);
common-listener.refs = 1;
common-listener.action = handler;
+ common-listener.free = free_common_channel_from_listener;
channel-disconnect = disconnect;
channel-hold_item = hold_item;
channel-release_item = release_item;
@@ -10187,6 +10197,11 @@ static void handle_dev_input(EventListener
*listener, uint32_t events)
}
}
+static void handle_dev_free(EventListener *ctx)
+{
+ free(ctx);
+}
+
static void red_init(RedWorker *worker, WorkerInitData *init_data)
{
struct epoll_event event;
@@ -10202,6 +10217,7 @@ static void red_init(RedWorker *worker,
WorkerInitData *init_data)
worker-pending = init_data-pending;
worker-dev_listener.refs = 1;
worker-dev_listener.action = handle_dev_input;
+ worker-dev_listener.free = handle_dev_free;
worker-cursor_visible = TRUE;
ASSERT(init_data-num_renderers 0);
worker-num_renderers = init_data-num_renderers;
@@ -10313,7 +10329,8 @@ void *red_worker_main(void *arg)
continue;
}
}
- free(evt_listener);
+ red_printf(freeing event listener);
+ evt_listener-free(evt_listener);
}
if (worker.running) {
--
1.7.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
--
Marc-André Lureau
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 05/18] server/red_worker: shorten some lines using alias variables
ack
On Mon, Feb 7, 2011 at 7:19 PM, Alon Levy al...@redhat.com wrote:
---
server/red_worker.c | 34 +-
1 files changed, 21 insertions(+), 13 deletions(-)
diff --git a/server/red_worker.c b/server/red_worker.c
index 0ed46e9..6bd8bc2 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -8163,13 +8163,14 @@ static void red_send_local_cursor(CursorChannel
*cursor_channel, LocalCursor *cu
RedChannel *channel;
SpiceMsgCursorSet cursor_set;
AddBufInfo info;
+ RedWorker *worker = cursor_channel-common.worker;
ASSERT(cursor_channel);
channel = cursor_channel-common.base;
channel-send_data.header-type = SPICE_MSG_CURSOR_SET;
cursor_set.position = cursor-position;
- cursor_set.visible = cursor_channel-common.worker-cursor_visible;
+ cursor_set.visible = worker-cursor_visible;
fill_cursor(cursor_channel, cursor_set.cursor, cursor-base, info);
spice_marshall_msg_cursor_set(channel-send_data.marshaller, cursor_set);
@@ -8196,10 +8197,12 @@ static void red_send_cursor(CursorChannel
*cursor_channel, CursorItem *cursor)
RedChannel *channel;
RedCursorCmd *cmd;
SpiceMarshaller *m;
+ RedWorker *worker;
ASSERT(cursor_channel);
channel = cursor_channel-common.base;
+ worker = cursor_channel-common.worker;
m = channel-send_data.marshaller;
cmd = cursor-red_cursor;
@@ -8219,7 +8222,7 @@ static void red_send_cursor(CursorChannel
*cursor_channel, CursorItem *cursor)
channel-send_data.header-type = SPICE_MSG_CURSOR_SET;
cursor_set.position = cmd-u.set.position;
- cursor_set.visible =
cursor_channel-common.worker-cursor_visible;
+ cursor_set.visible = worker-cursor_visible;
fill_cursor(cursor_channel, cursor_set.cursor, cursor, info);
spice_marshall_msg_cursor_set(m, cursor_set);
@@ -8540,17 +8543,19 @@ static void red_disconnect_channel(RedChannel
*channel)
static void red_disconnect_display(RedChannel *channel)
{
DisplayChannel *display_channel;
+ CommonChannel *common = SPICE_CONTAINEROF(channel, CommonChannel, base);
+ RedWorker *worker;
if (!channel || !channel-peer) {
return;
}
-
+ worker = common-worker;
display_channel = (DisplayChannel *)channel;
- ASSERT(display_channel ==
display_channel-common.worker-display_channel);
+ ASSERT(display_channel == worker-display_channel);
#ifdef COMPRESS_STAT
print_compress_stats(display_channel);
#endif
- display_channel-common.worker-display_channel = NULL;
+ worker-display_channel = NULL;
red_display_unshare_stream_buf(display_channel);
red_release_pixmap_cache(display_channel);
red_release_glz(display_channel);
@@ -9962,6 +9967,8 @@ static void handle_dev_input(EventListener *listener,
uint32_t events)
{
RedWorker *worker = SPICE_CONTAINEROF(listener, RedWorker, dev_listener);
RedWorkerMessage message;
+ RedChannel *cursor_red_channel = worker-cursor_channel-common.base;
+ RedChannel *display_red_channel = worker-display_channel-common.base;
int ring_is_empty;
read_message(worker-channel, message);
@@ -9980,8 +9987,9 @@ static void handle_dev_input(EventListener *listener,
uint32_t events)
display_channel_push(worker);
}
if (worker-qxl-st-qif-flush_resources(worker-qxl) == 0) {
- red_printf(oom current %u pipe %u, worker-current_size,
worker-display_channel ?
- worker-display_channel-common.base.pipe_size : 0);
+ red_printf(oom current %u pipe %u, worker-current_size,
+ worker-display_channel ?
+ display_red_channel-pipe_size : 0);
red_free_some(worker);
worker-qxl-st-qif-flush_resources(worker-qxl);
}
@@ -9995,11 +10003,11 @@ static void handle_dev_input(EventListener
*listener, uint32_t events)
red_wait_outgoing_item((RedChannel *)worker-cursor_channel);
if (worker-cursor_channel) {
- red_pipe_add_type(worker-cursor_channel-common.base,
PIPE_ITEM_TYPE_INVAL_CURSOR_CACHE);
- if (!worker-cursor_channel-common.base.migrate) {
- red_pipe_add_verb(worker-cursor_channel-common.base,
SPICE_MSG_CURSOR_RESET);
+ red_pipe_add_type(cursor_red_channel,
PIPE_ITEM_TYPE_INVAL_CURSOR_CACHE);
+ if (!cursor_red_channel-migrate) {
+ red_pipe_add_verb(cursor_red_channel,
SPICE_MSG_CURSOR_RESET);
}
- ASSERT(!worker-cursor_channel-common.base.send_data.item);
+ ASSERT(!cursor_red_channel-send_data.item);
worker-cursor_visible = TRUE;
worker-cursor_position.x = worker-cursor_position.y = 0;
@@ -10063,10 +10071,10 @@ static void handle_dev_input(EventListener
*listener, uint32_t events)
Re: [Spice-devel] [PATCH 06/18] server/red_worker: use red_channel pipe add versions
ack
On Mon, Feb 7, 2011 at 7:19 PM, Alon Levy al...@redhat.com wrote:
s/red_pipe_add/red_channel_pipe_add/
s/red_pipe_add_after/red_channel_pipe_add_after/
---
server/red_worker.c | 32
1 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/server/red_worker.c b/server/red_worker.c
index 6bd8bc2..76d0ef6 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -1182,14 +1182,14 @@ static inline void red_pipe_item_init(PipeItem *item,
int type)
item-type = type;
}
-static inline void red_pipe_add(RedChannel *channel, PipeItem *item)
+static inline void red_channel_pipe_add(RedChannel *channel, PipeItem *item)
{
ASSERT(channel);
channel-pipe_size++;
ring_add(channel-pipe, item-link);
}
-static inline void red_pipe_add_after(RedChannel *channel, PipeItem *item,
PipeItem *pos)
+static inline void red_channel_pipe_add_after(RedChannel *channel, PipeItem
*item, PipeItem *pos)
{
ASSERT(channel pos);
channel-pipe_size++;
@@ -1218,14 +1218,14 @@ static void red_pipe_add_verb(RedChannel* channel,
uint16_t verb)
VerbItem *item = spice_new(VerbItem, 1);
red_pipe_item_init(item-base, PIPE_ITEM_TYPE_VERB);
item-verb = verb;
- red_pipe_add(channel, item-base);
+ red_channel_pipe_add(channel, item-base);
}
static void red_pipe_add_type(RedChannel* channel, int pipe_item_type)
{
PipeItem *item = spice_new(PipeItem, 1);
red_pipe_item_init(item, pipe_item_type);
- red_pipe_add(channel, item);
+ red_channel_pipe_add(channel, item);
}
static inline void red_create_surface_item(RedWorker *worker, int
surface_id);
@@ -1267,7 +1267,7 @@ static inline void red_pipe_add_drawable(RedWorker
*worker, Drawable *drawable)
red_handle_drawable_surfaces_client_synced(worker, drawable);
drawable-refs++;
- red_pipe_add(worker-display_channel-common.base,
drawable-pipe_item);
+ red_channel_pipe_add(worker-display_channel-common.base,
drawable-pipe_item);
}
static inline void red_pipe_add_drawable_to_tail(RedWorker *worker, Drawable
*drawable)
@@ -1291,7 +1291,7 @@ static inline void
red_pipe_add_drawable_after(RedWorker *worker, Drawable *draw
return;
}
drawable-refs++;
- red_pipe_add_after(worker-display_channel-common.base,
drawable-pipe_item, pos_after-pipe_item);
+ red_channel_pipe_add_after(worker-display_channel-common.base,
drawable-pipe_item, pos_after-pipe_item);
}
static inline PipeItem *red_pipe_get_tail(RedWorker *worker)
@@ -1320,7 +1320,7 @@ static inline void red_pipe_add_image_item(RedWorker
*worker, ImageItem *item)
return;
}
item-refs++;
- red_pipe_add(worker-display_channel-common.base, item-link);
+ red_channel_pipe_add(worker-display_channel-common.base, item-link);
}
static inline void red_pipe_add_image_item_after(RedWorker *worker,
ImageItem *item,
@@ -1330,7 +1330,7 @@ static inline void
red_pipe_add_image_item_after(RedWorker *worker, ImageItem *i
return;
}
item-refs++;
- red_pipe_add_after(worker-display_channel-common.base, item-link,
pos);
+ red_channel_pipe_add_after(worker-display_channel-common.base,
item-link, pos);
}
static inline uint64_t channel_message_serial(RedChannel *channel)
@@ -1493,7 +1493,7 @@ static inline void red_destroy_surface_item(RedWorker
*worker, uint32_t surface_
destroy = get_surface_destroy_item(surface_id);
- red_pipe_add(worker-display_channel-common.base, destroy-pipe_item);
+ red_channel_pipe_add(worker-display_channel-common.base,
destroy-pipe_item);
}
static inline void red_destroy_surface(RedWorker *worker, uint32_t
surface_id)
@@ -2286,7 +2286,7 @@ static void
push_stream_clip_by_drawable(DisplayChannel* channel, StreamAgent *a
item-rects-num_rects = n_rects;
region_ret_rects(drawable-tree_item.base.rgn, item-rects-rects,
n_rects);
}
- red_pipe_add((RedChannel*)channel, (PipeItem *)item);
+ red_channel_pipe_add((RedChannel*)channel, (PipeItem *)item);
}
static void push_stream_clip(DisplayChannel* channel, StreamAgent *agent)
@@ -2304,7 +2304,7 @@ static void push_stream_clip(DisplayChannel* channel,
StreamAgent *agent)
item-rects = spice_malloc_n_m(n_rects, sizeof(SpiceRect),
sizeof(SpiceClipRects));
item-rects-num_rects = n_rects;
region_ret_rects(agent-vis_region, item-rects-rects, n_rects);
- red_pipe_add((RedChannel*)channel, (PipeItem *)item);
+ red_channel_pipe_add((RedChannel*)channel, (PipeItem *)item);
}
static void red_display_release_stream_clip(DisplayChannel* channel,
StreamClipItem *item)
@@ -2349,7 +2349,7 @@ static void red_stop_stream(RedWorker *worker, Stream
*stream)
region_clear(stream_agent-vis_region);
ASSERT(!pipe_item_is_linked(stream_agent-destroy_item));
stream-refs++;
-
Re: [Spice-devel] [PATCH 08/18] server/red_worker: split display_channel_send_item
ack
On Mon, Feb 7, 2011 at 7:19 PM, Alon Levy al...@redhat.com wrote:
Split it out of display_channel_push.
---
server/red_worker.c | 182
++-
1 files changed, 94 insertions(+), 88 deletions(-)
diff --git a/server/red_worker.c b/server/red_worker.c
index 08aeef2..2e3dc16 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -8308,99 +8308,105 @@ static inline PipeItem *red_pipe_get(RedChannel
*channel)
return item;
}
+static void display_channel_send_item(RedChannel *base, PipeItem *pipe_item)
+{
+ DisplayChannel *display_channel = (DisplayChannel
*)red_ref_channel(base);
+ RedWorker *worker = display_channel-common.worker;
+
+ red_display_reset_send_data(display_channel);
+ switch (pipe_item-type) {
+ case PIPE_ITEM_TYPE_DRAW: {
+ Drawable *drawable = SPICE_CONTAINEROF(pipe_item, Drawable,
pipe_item);
+ send_qxl_drawable(display_channel, drawable);
+ release_drawable(worker, drawable);
+ break;
+ }
+ case PIPE_ITEM_TYPE_INVAL_ONE:
+ red_display_send_inval(display_channel, (CacheItem *)pipe_item);
+ free(pipe_item);
+ break;
+ case PIPE_ITEM_TYPE_STREAM_CREATE: {
+ StreamAgent *agent = SPICE_CONTAINEROF(pipe_item, StreamAgent,
create_item);
+ red_display_send_stream_start(display_channel, agent);
+ red_display_release_stream(display_channel, agent);
+ break;
+ }
+ case PIPE_ITEM_TYPE_STREAM_CLIP: {
+ StreamClipItem* clip_item = (StreamClipItem *)pipe_item;
+ red_display_send_stream_clip(display_channel, clip_item);
+ red_display_release_stream_clip(display_channel, clip_item);
+ break;
+ }
+ case PIPE_ITEM_TYPE_STREAM_DESTROY: {
+ StreamAgent *agent = SPICE_CONTAINEROF(pipe_item, StreamAgent,
destroy_item);
+ red_display_send_stream_end(display_channel, agent);
+ red_display_release_stream(display_channel, agent);
+ break;
+ }
+ case PIPE_ITEM_TYPE_UPGRADE:
+ red_display_send_upgrade(display_channel, (UpgradeItem *)pipe_item);
+ release_upgrade_item(worker, (UpgradeItem *)pipe_item);
+ break;
+ case PIPE_ITEM_TYPE_VERB:
+ display_send_verb(display_channel, ((VerbItem*)pipe_item)-verb);
+ free(pipe_item);
+ break;
+ case PIPE_ITEM_TYPE_MIGRATE:
+ red_printf(PIPE_ITEM_TYPE_MIGRATE);
+ display_channel_send_migrate(display_channel);
+ free(pipe_item);
+ break;
+ case PIPE_ITEM_TYPE_MIGRATE_DATA:
+ display_channel_send_migrate_data(display_channel);
+ free(pipe_item);
+ break;
+ case PIPE_ITEM_TYPE_SET_ACK:
+ red_send_set_ack((RedChannel *)display_channel);
+ free(pipe_item);
+ break;
+ case PIPE_ITEM_TYPE_IMAGE:
+ red_send_image(display_channel, (ImageItem *)pipe_item);
+ release_image_item((ImageItem *)pipe_item);
+ break;
+ case PIPE_ITEM_TYPE_PIXMAP_SYNC:
+ display_channel_pixmap_sync(display_channel);
+ free(pipe_item);
+ break;
+ case PIPE_ITEM_TYPE_PIXMAP_RESET:
+ display_channel_reset_cache(display_channel);
+ free(pipe_item);
+ break;
+ case PIPE_ITEM_TYPE_INVAL_PALLET_CACHE:
+ red_reset_palette_cache(display_channel);
+ red_send_verb((RedChannel *)display_channel,
SPICE_MSG_DISPLAY_INVAL_ALL_PALETTES);
+ free(pipe_item);
+ break;
+ case PIPE_ITEM_TYPE_CREATE_SURFACE: {
+ SurfaceCreateItem *surface_create = SPICE_CONTAINEROF(pipe_item,
SurfaceCreateItem,
+ pipe_item);
+ red_send_surface_create(display_channel,
surface_create-surface_create);
+ free(surface_create);
+ break;
+ }
+ case PIPE_ITEM_TYPE_DESTROY_SURFACE: {
+ SurfaceDestroyItem *surface_destroy = SPICE_CONTAINEROF(pipe_item,
SurfaceDestroyItem,
+ pipe_item);
+ red_send_surface_destroy(display_channel,
surface_destroy-surface_destroy.surface_id);
+ free(surface_destroy);
+ break;
+ }
+ default:
+ red_error(invalid pipe item type);
+ }
+ red_unref_channel((RedChannel *)display_channel);
+}
+
static void display_channel_push(RedWorker *worker)
{
PipeItem *pipe_item;
while ((pipe_item = red_pipe_get((RedChannel *)worker-display_channel)))
{
- DisplayChannel *display_channel;
- display_channel = (DisplayChannel *)red_ref_channel((RedChannel
*)worker-display_channel);
- red_display_reset_send_data(display_channel);
- switch (pipe_item-type) {
- case PIPE_ITEM_TYPE_DRAW: {
- Drawable *drawable = SPICE_CONTAINEROF(pipe_item, Drawable,
pipe_item);
-
Re: [Spice-devel] [PATCH 00/18] red_channel introducing refactoring, part 2, v1
On Tue, Feb 08, 2011 at 07:47:23PM +0100, Marc-André Lureau wrote: Hi Alon, Gerd, While reviewing Alon patches, I started to get annoyed by a crash I didn't see before, although it seems to be an old bug, and comes from qemu perhaps? I am investing that crash before continuing the review, as it gets worst when I applied the first 3/4 patches (double free..). The crash happens under valgrind, but not under gdb, which could mean it is some kind of race. Using either spice.v30 or spice.kvm.v28 branch, and spice 0.6.3-173-g8c17f83 (current master) valgrind --db-attach=yes --smc-check=all --undef-value-errors=no /usr/local/bin/qemu-system-x86_64 --enable-kvm -net nic -net user -m 512 -localtime -device AC97 -spice port=5930,disable-ticketing -vga qxl start a client, and hit C-c to interrupt it, you get this: reds_disconnect: red_dispatcher_shutdown_cursor_peer: red_dispatcher_shutdown_peer: handle_dev_input: cursor disconnect ==4797== Invalid read of size 4 ==4797==at 0x50ABA24: red_channel_event (red_channel.c:433) ==4797==by 0x4D41CC: watch_read (spice-core.c:93) ==4797==by 0x59532E: main_loop_wait (vl.c:1349) ==4797==by 0x595551: main_loop (vl.c:1406) ==4797==by 0x599620: main (vl.c:3120) ==4797== Address 0x37205f88 is 104 bytes inside a block of size 28,608 free'd ==4797==at 0x4A05187: free (vg_replace_malloc.c:325) ==4797==by 0x50AB858: red_channel_destroy (red_channel.c:377) ==4797==by 0x508C333: main_disconnect (main_channel.c:140) ==4797==by 0x508D98B: main_channel_shutdown (main_channel.c:835) ==4797==by 0x5086264: reds_disconnect (reds.c:644) ==4797==by 0x508D6E6: main_channel_on_error (main_channel.c:759) ==4797==by 0x50AB27D: red_channel_peer_on_incoming_error (red_channel.c:205) ==4797==by 0x50AACFB: red_peer_handle_incoming (red_channel.c:85) ==4797==by 0x50AAFF6: red_channel_receive (red_channel.c:152) ==4797==by 0x50ABA15: red_channel_event (red_channel.c:429) ==4797==by 0x4D41CC: watch_read (spice-core.c:93) ==4797==by 0x59532E: main_loop_wait (vl.c:1349) ==4797== ==4797== If you try to reproduce the bug with gdb, by breaking first on red_channel_destroy and then on red_channel_event, you can see that red_channel_event is not called. Do you mean you couldn't get a stacktrace from gdb showing the same? Anyway, (sidetracking just to make sure you know the architecture), we actually have two main loops in spice-server, or rather use two main loops: * all channels except cursor and display piggyback on the qemu main loop, consequently they run in the main thread (not the kvm cpu thread, nor the worker thread). * cursor and display channels both have their own main loop (epoll based just to make it more interesting), where they handle events from their channel sockets and from a pipe beeing fed by the red_dispatcher, which is called from the vcpu thread on io exits (actually, right now it can also be fed from the main thread, which might be related to this bug). It looks like the main channel disconnect, so it should be related to all this exposition material. So just looks like we are not removing the watch event fast enough? or maybe like you say qemu is calling an event we already requested removal of (we use core-watch_remove I think, core being a SpiceCoreInterface). Alon Help appreciated! regards On Mon, Feb 7, 2011 at 7:19 PM, Alon Levy al...@redhat.com wrote: This is a part of a much larger series so I don't have a good concise description. Ultimately this all leads to a reuse of RedChannel across most of the channels (sound remains an odd out because of not using a pipe abstraction, but a bitmap of outstanding packets, you can have 0 or one of each type waiting to be sent, this makes sense for audio. Probably for video too actually). Anyway this patchset touches almost only red_worker (and it's helper cache files, split out because they are used for a number of different caches via macros before inclusion, that usual trick). It is mostly about renaming, making a CommonChannel take the place of the internal RedChannel to allow future use of the external RedChannel defined in red_channel.h Alon Levy (18): server/red_worker: change hold_item sig, drop the void* server/red_worker: use ack_data struct server/red_worker: introduce CommonChannel server/red_worker: add free cb to EventHandler server/red_worker: shorten some lines using alias variables server/red_worker: use red_channel pipe add versions server/red_worker: extract common_release_pipe_item from red_pipe_clear server/red_worker: split display_channel_send_item server/red_worker: add red_channel_init_send_data server/red_worker: use red_channel begin_send_message server/red_worker: small cleanup with worker alias server/red_worker: split cursor_channel_send_item server/red_worker:
