[RFC] Patch for VNC SPU getting "stuck"
"Niraj Tolia" <[email protected]> Tue, 28 Feb 2006 12:23:38 -0500
| Newsgroups | gmane.comp.graphics.chromium.devel |
|---|---|
| Message-ID | <[email protected]> |
This report is against CVS HEAD. With the last round of changes, the Async
IO code path will block everytime there is no update to send. The relevant
stack trace is
#3 0x00a63d75 in crWaitSemaphore (s=0xca46b8) at threads.c:259
#4 0x00c8786c in vncspuWaitDirtyRects (region=0xa246884,
frame_num=0xa2469e8)
at vncspu.c:335
#5 0x00c8b13e in rf_client_updatereq () at client_io.c:516
#6 0x00c8989d in aio_process_input (slot=0xa246728) at async_io.c:562
#7 0x00c89486 in aio_mainloop () at async_io.c:390
We never ran into this issue earlier as the VNC SPU had a fixed frame update
rate and we would always have something to send out. However, now we can get
stuck because of the above. Things that the user runs into includes attempts
at new connections "hanging". This happens because we never return to the
aio_mainloop unless an update was received from the remote application
running under crappfaker.
As we already record that an update was requested on the current client
slot, one option is to return to the aio_mainloop and defer sending the
update till something is actually available. The deferred update can be sent
by leveraging the s_idle_func used in the AIO code. An example patch of how
this could be done is attached. One disadvantage is that we need to reduce
the timeout value of the select() call in aio_mainloop(). Testing did not
indicate any increased CPU usage though as the idle function is cheap.
I have a decent handle on the VNC SPU code and so, if you feel this should
be done differently, just let me know and I should be able to code up
another patch.
Thanks,
Niraj
--
http://www.cs.cmu.edu/~ntolia <http://www.cs.cmu.edu/%7Entolia>
vncspu_block.patch
(application/octet-stream, 3.6 KB)
Index: async_io.c
===================================================================
RCS file: /cvsroot/chromium/cr/spu/vnc/async_io.c,v
retrieving revision 1.2
diff -u -r1.2 async_io.c
--- async_io.c 24 Feb 2006 20:46:35 -0000 1.2
+++ async_io.c 28 Feb 2006 17:11:00 -0000
@@ -375,8 +375,8 @@
while (!s_close_f) {
memcpy(&fdset_r, &s_fdset_read, sizeof(fd_set));
memcpy(&fdset_w, &s_fdset_write, sizeof(fd_set));
- timeout.tv_sec = 1; /* One second timeout */
- timeout.tv_usec = 0;
+ timeout.tv_sec = 0; /* microsecond timeout */
+ timeout.tv_usec = 1000;
if (select(s_max_fd + 1, &fdset_r, &fdset_w, NULL, &timeout) > 0) {
slot = s_first_slot;
while (slot != NULL && !s_close_f) {
@@ -487,6 +487,10 @@
cur_slot->closefunc = closefunc;
}
+void aio_set_idle_func(AIO_FUNCPTR idlefunc)
+{
+ s_idle_func = idlefunc;
+}
/***************************
* Static functions follow
Index: async_io.h
===================================================================
RCS file: /cvsroot/chromium/cr/spu/vnc/async_io.h,v
retrieving revision 1.2
diff -u -r1.2 async_io.h
--- async_io.h 24 Feb 2006 20:46:35 -0000 1.2
+++ async_io.h 28 Feb 2006 17:11:00 -0000
@@ -98,6 +98,7 @@
void aio_write(AIO_FUNCPTR fn, void *outbuf, int bytes_to_write);
void aio_write_nocopy(AIO_FUNCPTR fn, AIO_BLOCK *block);
void aio_setclose(AIO_FUNCPTR closefunc);
+void aio_set_idle_func(AIO_FUNCPTR idlefunc);
int aio_any_output_pending(void);
AIO_SLOT *aio_first_slot(void);
Index: client_io.c
===================================================================
RCS file: /cvsroot/chromium/cr/spu/vnc/client_io.c,v
retrieving revision 1.10
diff -u -r1.10 client_io.c
--- client_io.c 24 Feb 2006 20:46:35 -0000 1.10
+++ client_io.c 28 Feb 2006 17:11:00 -0000
@@ -511,10 +511,13 @@
if (!cl->update_in_progress) {
k = (cl->newfbsize_pending ||
- REGION_NOTEMPTY(&cl->copy_region));
+ REGION_NOTEMPTY(&cl->copy_region) ||
+ vncspuGetDirtyRects(&cl->pending_region, &cl->frame_num));
+#ifdef DISABLED_AS_THIS_BLOCKS
if (!k) {
k = vncspuWaitDirtyRects(&cl->pending_region, &cl->frame_num);
}
+#endif
if (k) {
send_update();
}
@@ -680,6 +683,15 @@
}
}
+/**
+ * Used if an outstanding FBUR exists, ie an update was requested in
+ * the past.
+ */
+void fn_client_send_deferred_update(void)
+{
+ aio_walk_slots(fn_client_send_rects, TYPE_CL_SLOT);
+}
+
void fn_client_send_cuttext(AIO_SLOT *slot, CARD8 *text, size_t len)
{
CL_SLOT *cl = (CL_SLOT *)slot;
Index: client_io.h
===================================================================
RCS file: /cvsroot/chromium/cr/spu/vnc/client_io.h,v
retrieving revision 1.5
diff -u -r1.5 client_io.h
--- client_io.h 24 Feb 2006 20:46:35 -0000 1.5
+++ client_io.h 28 Feb 2006 17:11:00 -0000
@@ -72,6 +72,7 @@
void fn_client_add_rect(AIO_SLOT *slot, FB_RECT *rect);
void fn_client_send_rects(AIO_SLOT *slot);
void fn_client_send_cuttext(AIO_SLOT *slot, CARD8 *text, size_t len);
+void fn_client_send_deferred_update(void);
int num_clients(void);
Index: main.c
===================================================================
RCS file: /cvsroot/chromium/cr/spu/vnc/main.c,v
retrieving revision 1.5
diff -u -r1.5 main.c
--- main.c 1 Feb 2006 19:31:24 -0000 1.5
+++ main.c 28 Feb 2006 17:11:00 -0000
@@ -159,6 +159,7 @@
set_actions_file(opt_actions_filename);
aio_init();
+ aio_set_idle_func(fn_client_send_deferred_update);
if (opt_bind_ip != NULL) {
if (aio_set_bind_address(opt_bind_ip)) {
log_write(LL_INFO, "Would bind listening sockets to address %s",