From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefano Stabellini Subject: Re: [PATCH] unnecessary removal of guest console characters Date: Mon, 21 Apr 2008 17:28:27 +0100 Message-ID: <480CC0AB.20001@eu.citrix.com> References: Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------090308090703010100060408" Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: xen-devel@lists.xensource.com List-Id: xen-devel@lists.xenproject.org This is a multi-part message in MIME format. --------------090308090703010100060408 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 8bit Hi all, I am attaching a patch that adds a new command line option to xenconsoled: -o, --overflow-data=discard|keep this option changes the behaviour when dealing with data that overflow the max capacity of the buffer. If overflow-data is set to discard (the default), the current behaviour is used: we discard some data in the middle of the buffer. If overflow-data is set to keep, we stop listening to the ring until we free some space in the buffer. This can cause the ring to fill up and the guest kernel internal buffer to fill up as well. When this happens the guest kernel stops reading characters from the console device so the application generating data hangs. When xenconsoled resumes reading from the ring, the guest kernel will be able to resume reading from the console device as well. At that point the guest application will be allowed to continue. Regards, Stefano Stabellini Signed-off-by: Stefano Stabellini Keir Fraser wrote: > On 14/4/08 15:14, "Samuel Thibault" wrote: > >> Keir Fraser, le Fri 11 Apr 2008 16:46:48 +0100, a écrit : >>> The 'application' in this case is the kernel. >> Mmm, in my understanding of printk, it is not blocking for the kernel, >> and if a console is not able to catch up, the data simply gets >> discarded (see __call_console_drivers, which doesn't check the result >> of con->write(), which can hence be even 0 if the console is really >> completely stuck). > > Actually I was worried that we might depend on the ring being drained in a > timely fashion in our own console driver implementation in Linux. Actually I > think we don't (except that the handling of x_char looks suspect). Still, I > don't know whether other guest types depend on the current behaviour. We > could make the behaviour switchable I suppose. > > -- Keir > > --------------090308090703010100060408 Content-Type: text/x-diff; name="console-overflow2.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="console-overflow2.patch" diff -r 84b5dee690f5 tools/console/daemon/io.c --- a/tools/console/daemon/io.c Mon Apr 21 14:59:25 2008 +0100 +++ b/tools/console/daemon/io.c Mon Apr 21 16:51:32 2008 +0100 @@ -63,6 +63,7 @@ extern int log_time_hv; extern int log_time_hv; extern int log_time_guest; extern char *log_dir; +extern int discard_overflowed_data; static int log_time_hv_needts = 1; static int log_time_guest_needts = 1; @@ -201,7 +202,7 @@ static void buffer_append(struct domain dom->domid, errno, strerror(errno)); } - if (buffer->max_capacity && + if (discard_overflowed_data && buffer->max_capacity && buffer->size > buffer->max_capacity) { /* Discard the middle of the data. */ @@ -228,6 +229,11 @@ static void buffer_advance(struct buffer if (buffer->consumed == buffer->size) { buffer->consumed = 0; buffer->size = 0; + if (buffer->max_capacity && + buffer->capacity > buffer->max_capacity) { + buffer->data = realloc(buffer->data, buffer->max_capacity); + buffer->capacity = buffer->max_capacity; + } } } @@ -1005,9 +1011,13 @@ void handle_io(void) d->next_period < next_timeout) next_timeout = d->next_period; } else if (d->xce_handle != -1) { - int evtchn_fd = xc_evtchn_fd(d->xce_handle); - FD_SET(evtchn_fd, &readfds); - max_fd = MAX(evtchn_fd, max_fd); + if (discard_overflowed_data || + !d->buffer.max_capacity || + d->buffer.size < d->buffer.max_capacity) { + int evtchn_fd = xc_evtchn_fd(d->xce_handle); + FD_SET(evtchn_fd, &readfds); + max_fd = MAX(evtchn_fd, max_fd); + } } if (d->master_fd != -1) { diff -r 84b5dee690f5 tools/console/daemon/main.c --- a/tools/console/daemon/main.c Mon Apr 21 14:59:25 2008 +0100 +++ b/tools/console/daemon/main.c Mon Apr 21 16:51:54 2008 +0100 @@ -38,6 +38,7 @@ int log_time_hv = 0; int log_time_hv = 0; int log_time_guest = 0; char *log_dir = NULL; +int discard_overflowed_data = 1; static void handle_hup(int sig) { @@ -46,7 +47,7 @@ static void handle_hup(int sig) static void usage(char *name) { - printf("Usage: %s [-h] [-V] [-v] [-i] [--log=none|guest|hv|all] [--log-dir=DIR] [--pid-file=PATH] [-t, --timestamp=none|guest|hv|all]\n", name); + printf("Usage: %s [-h] [-V] [-v] [-i] [--log=none|guest|hv|all] [--log-dir=DIR] [--pid-file=PATH] [-t, --timestamp=none|guest|hv|all] [-o, --overflow-data=discard|keep]\n", name); } static void version(char *name) @@ -56,7 +57,7 @@ static void version(char *name) int main(int argc, char **argv) { - const char *sopts = "hVvit:"; + const char *sopts = "hVvit:o:"; struct option lopts[] = { { "help", 0, 0, 'h' }, { "version", 0, 0, 'V' }, @@ -66,6 +67,7 @@ int main(int argc, char **argv) { "log-dir", 1, 0, 'r' }, { "pid-file", 1, 0, 'p' }, { "timestamp", 1, 0, 't' }, + { "overflow-data", 1, 0, 'o'}, { 0 }, }; bool is_interactive = false; @@ -119,6 +121,13 @@ int main(int argc, char **argv) log_time_hv = 0; } break; + case 'o': + if (!strcmp(optarg, "keep")) { + discard_overflowed_data = 0; + } else if (!strcmp(optarg, "discard")) { + discard_overflowed_data = 1; + } + break; case '?': fprintf(stderr, "Try `%s --help' for more information\n", --------------090308090703010100060408 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel --------------090308090703010100060408--