From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58394) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WkqY2-0000rl-Qx for qemu-devel@nongnu.org; Thu, 15 May 2014 03:57:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WkqXw-0006M0-Nc for qemu-devel@nongnu.org; Thu, 15 May 2014 03:57:50 -0400 Received: from mx1.redhat.com ([209.132.183.28]:59109) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WkqXw-0006Lo-EY for qemu-devel@nongnu.org; Thu, 15 May 2014 03:57:44 -0400 Date: Thu, 15 May 2014 09:46:38 +0200 From: Stefan Hajnoczi Message-ID: <20140515074638.GB1111@stefanha-thinkpad.redhat.com> References: <1400077367-23409-1-git-send-email-stefanha@redhat.com> <20140514174018.GF5955@irqsave.net> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <20140514174018.GF5955@irqsave.net> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 0/3] throttle: use AioContext for dataplane support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?iso-8859-1?Q?Beno=EEt?= Canet Cc: Kevin Wolf , qemu-devel@nongnu.org On Wed, May 14, 2014 at 07:40:18PM +0200, Beno=EEt Canet wrote: > The Wednesday 14 May 2014 =E0 16:22:44 (+0200), Stefan Hajnoczi wrote : > > This series applies on top of my "dataplane: use QEMU block layer" se= ries. > >=20 > > Now that the dataplane code path is using the QEMU block layer we sho= uld make > > I/O throttling limits safe to use. When the block_set_io_throttle mo= nitor > > command is executed, the BlockDriverState's AioContext must be acquir= ed in > > order to prevent race conditions with the IOThread that is processing= requests > > from the guest. > >=20 > > The new block layer AioContext detach/attach mechanism needs to be ex= tended to > > move the throttling timer to a new AioContext. This makes throttling= work > > across bdrv_set_aio_context() calls. > >=20 > > The result of this series is that I/O throttling works with dataplane= and > > limits may be changed at runtime using the monitor. > >=20 > > Stefan Hajnoczi (3): > > throttle: add throttle_detach/attach_aio_context() > > throttle: add detach/attach test case > > blockdev: acquire AioContext in block_set_io_throttle > >=20 > > block.c | 7 +++++++ > > blockdev.c | 6 ++++++ > > include/qemu/throttle.h | 10 ++++++++++ > > tests/test-throttle.c | 49 +++++++++++++++++++++++++++++++++++++++= +++++----- > > util/throttle.c | 27 +++++++++++++++++++++++---- > > 5 files changed, 90 insertions(+), 9 deletions(-) > >=20 > > --=20 > > 1.9.0 > >=20 >=20 > I find One thing chocking is this series. > I carefully decloupled the throttling code from the block layer so anyo= ne could reuse it. > I am under the impression that this series couples it back. The coupling is just due to the current header file layout. It is not a real coupling to the block layer. Throttling has a dependency on timers. Timers are part of an event loop (either AioContext or main loop). The AioContext prototypes happen to be in block/aio.h but they are not a dependency on block.h or BlockDriverState. This means we could extract them and move them to qemu/aiocontext.h in a separate series. Hope this explains the block/aio.h header and shows it's not true coupling. Stefan