From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54282) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d5Z7r-0007xg-Vy for qemu-devel@nongnu.org; Tue, 02 May 2017 10:50:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1d5Z7o-0004IJ-5U for qemu-devel@nongnu.org; Tue, 02 May 2017 10:50:04 -0400 Received: from mx1.redhat.com ([209.132.183.28]:55930) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1d5Z7n-0004HV-TZ for qemu-devel@nongnu.org; Tue, 02 May 2017 10:50:00 -0400 Date: Tue, 2 May 2017 15:49:52 +0100 From: "Dr. David Alan Gilbert" Message-ID: <20170502144951.GE2072@work-vm> References: <1493732857-10721-1-git-send-email-den@openvz.org> <20170502144426.GE16624@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170502144426.GE16624@redhat.com> Subject: Re: [Qemu-devel] [PATCH 1/1] monitor: increase amount of data for monitor to read List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" Cc: Eric Blake , "Denis V. Lunev" , qemu-devel@nongnu.org, Markus Armbruster * Daniel P. Berrange (berrange@redhat.com) wrote: > On Tue, May 02, 2017 at 09:34:55AM -0500, Eric Blake wrote: > > On 05/02/2017 08:47 AM, Denis V. Lunev wrote: > > > Right now QMP and HMP monitors read 1 byte at a time from the socket, which > > > is very inefficient. With 100+ VMs on the host this easily reasults in > > > > s/reasults/results/ > > > > > a lot of unnecessary system calls and CPU usage in the system. > > > > > > This patch changes the amount of data to read to 4096 bytes, which matches > > > buffer size on the channel level. Fortunately, monitor protocol is > > > synchronous right now thus we should not face side effects in reality. > > > > Do you have any easy benchmarks or measurements to prove what sort of > > efficiencies we get? (I believe they exist, but quantifying them never > > hurts) > > > > > > > > Signed-off-by: Denis V. Lunev > > > CC: Markus Armbruster > > > CC: "Dr. David Alan Gilbert" > > > CC: Eric Blake > > > --- > > > monitor.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/monitor.c b/monitor.c > > > index be282ec..00df5d0 100644 > > > --- a/monitor.c > > > +++ b/monitor.c > > > @@ -3698,7 +3698,7 @@ static int monitor_can_read(void *opaque) > > > { > > > Monitor *mon = opaque; > > > > > > - return (mon->suspend_cnt == 0) ? 1 : 0; > > > + return (mon->suspend_cnt == 0) ? 4096 : 0; > > > > Is a hard-coded number correct, or should we be asking the channel for > > an actual number? > > There's no need - this will cause the chardev code to just do a > gio_channel_read() with a 4096 byte buffer. The chardev backend > impl will then happily return fewer bytes than this - just whatever > happens to be pending. IOW this is just acting as an upper bound > on the amount of data we read at once. So 4k seems reasonable to > me, given the typical size of QMP/HMP command strings. So there's *no* situation in which that will block? I'm assuming the reason it read one byte was thats the only thing that poll() coming back to you guarantees. Dave > Regards, > Daniel > -- > |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o- https://fstop138.berrange.com :| > |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK