From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bart Van Assche Subject: Re: [PATCH 1/2] vhost: Reduce vhost_work_flush() wakeup latency Date: Thu, 15 Aug 2013 08:13:14 +0200 Message-ID: <520C717A.7060007@acm.org> References: <520B2B47.9040002@acm.org> <520B2B88.6020307@acm.org> <20130814113739.GE5430@redhat.com> <520BA0BC.6080202@acm.org> <20130814175825.GC17580@redhat.com> <20130815013056.GA7304@hj.localdomain> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: "Michael S. Tsirkin" , kvm-devel To: Asias He Return-path: Received: from jacques.telenet-ops.be ([195.130.132.50]:34127 "EHLO jacques.telenet-ops.be" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751851Ab3HOGNQ (ORCPT ); Thu, 15 Aug 2013 02:13:16 -0400 In-Reply-To: <20130815013056.GA7304@hj.localdomain> Sender: kvm-owner@vger.kernel.org List-ID: On 08/15/13 03:30, Asias He wrote: > On Wed, Aug 14, 2013 at 08:58:25PM +0300, Michael S. Tsirkin wrote: >> On Wed, Aug 14, 2013 at 05:22:36PM +0200, Bart Van Assche wrote: >>> On 08/14/13 13:37, Michael S. Tsirkin wrote: >>>> On Wed, Aug 14, 2013 at 09:02:32AM +0200, Bart Van Assche wrote: >>>>> If the TIF_NEED_RESCHED task flag is set, wake up any vhost_work_flush() >>>>> waiters before rescheduling instead of after rescheduling. >>>>> >>>>> Signed-off-by: Bart Van Assche >>>>> Cc: Michael S. Tsirkin >>>>> Cc: Asias He >>>> >>>> Why exactly? It's not like flush needs to be extra fast ... >>> >>> I'm not worried about how fast a flush is processed either. But I >>> think this patch is a nice cleanup of the vhost_worker() function. >>> It eliminates the uninitialized_var() construct and moves the >>> assignment of "done_seq" below the read of "queue_seq". >>> >>> Bart. >> >> I'm not worried about uninitialized_var - it's just a compiler bug. >> done_seq below read is nice, but this is at the cost of sprinkling >> lock/unlock, lock/unlock all over the code. Maybe we can come up with >> something that doesn't have this property. > > The extra locking introduced here does not look good to me neither. Please note that although there are additional locking statements in the code, there are no additional locking calls at runtime. Bart.