From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33055) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aznKi-0004vh-S4 for qemu-devel@nongnu.org; Mon, 09 May 2016 11:42:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aznKh-00018p-Nr for qemu-devel@nongnu.org; Mon, 09 May 2016 11:42:56 -0400 Date: Mon, 9 May 2016 17:42:46 +0200 From: Kevin Wolf Message-ID: <20160509154246.GD5054@noname.redhat.com> References: <1461346962-4676-1-git-send-email-kwolf@redhat.com> <1461346962-4676-10-git-send-email-kwolf@redhat.com> <20160509131706.GD3372@stefanha-x1.localdomain> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="mP3DRpeJDSE+ciuQ" Content-Disposition: inline In-Reply-To: <20160509131706.GD3372@stefanha-x1.localdomain> Subject: Re: [Qemu-devel] [Qemu-block] [PATCH v2 09/13] block: Drain throttling queue with BdrvChild callback List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: qemu-block@nongnu.org, qemu-devel@nongnu.org, mreitz@redhat.com, stefanha@redhat.com, pbonzini@redhat.com --mP3DRpeJDSE+ciuQ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Am 09.05.2016 um 15:17 hat Stefan Hajnoczi geschrieben: > On Fri, Apr 22, 2016 at 07:42:38PM +0200, Kevin Wolf wrote: > > This removes the last part of I/O throttling from block/io.c and moves > > it to the BlockBackend. > >=20 > > Instead of having knowledge about throttling inside io.c, we can call a > > BdrvChild callback .drained_begin/end, which happens to drain the > > throttled requests for BlockBackend parents. > >=20 > > Signed-off-by: Kevin Wolf > > --- > > block/block-backend.c | 32 +++++++++++++++++++++++++++----- > > block/io.c | 39 ++++++++++++++++++--------------------- > > include/block/block_int.h | 8 +++----- > > 3 files changed, 48 insertions(+), 31 deletions(-) >=20 > I'm confused about the naming. BdrvChildRole.drained_begin/end and > bdrv_parent_drained_begin/end have nothing to do with > bdrv_drained_begin()/bdrv_drained_end()? Hm, you may have a point there. I think we need to add another pair of calls in bdrv_drained_begin()/bdrv_drained_end(). We just want to call the callbacks as well on a simple bdrv_drain() or bdrv_drain_all(), so the existing calls should be right. > If these were callbacks that happened at bdrv_drained_begin/end time > then I could understand, but that doesn't seem to be the case. >=20 > What are the semantics of these callbacks? Maybe we can find a clearer > name. I think the point is not to "drain" (in the sense of completing > requests) but simply to restart queued requests? This is just a different perspective on the same thing, as these callbacks are always called as part of a bdrv_drain(). Any request that is restarted is also completed before bdrv_drain() returns. The intended semantics is that a parent doesn't submit new requests after returning from .drained_begin() until .drained_end() is called. The easy implementation for throttling was to simply restart the requests like the old implementation did. Kevin --mP3DRpeJDSE+ciuQ Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJXMK/2AAoJEH8JsnLIjy/WtmoQAJB48XVxybDb1ZC8vlhntRJ4 UXo5q/d5d/610AfUMFHdQhQzNv7GE4V6UhNUl1QvnCex8VeKbDmaYpC9SQphKpR+ UgIpI4m3GC1tqefEYBL9ufcDdMqKeZzMol8DDbOHpOctsQIsZ+EBiQJtwzWxfkXv OzDdoMqRG5xiD6sYniVY1mHYeK0zY7P8Eyk23erbys8oj/lEf2qczMTNie2h12z1 /hSj7YUTonRlCKtlWG7J7rVDheXWOqwQ8VmQGCySrrlcwVw++IXwAzTeGSRTJZ45 ATdJzgz+oxCjR06UJunI2n89hE/yfBahD4Sij+K9wVt1ROCkwQobe6CZubVfXol/ XX7blByO6KBEI3xCUa7Gb2yf1oeCk19Z58ugYleg6Vk56NjFfsXjqiO8yLVpVW2O z2VGzslECIVT6iF8N2GgLiqjl7nzhjzuVX/XvMFWWBOLzhM25i2SVOGmSrlvhAPQ WFvHMepahdztRUBfPUQ/bWeo8vckTlKzPahoaKEmSsn4RS+w7cvDnKF6DiMgnzve ywsfBQ+hqseJidG2C1Iy3keAAZCobOo40lEI9bmeJoMEP6DDKXwaJZrVSArIFNU+ m8ncXIzyLIj13F/SLRCkoODC4LLHeGAoLCXKza7/jrTABJHyztcvczl2YjWjQRX7 bnJUIvwOhny49LXDZNtq =Xoh7 -----END PGP SIGNATURE----- --mP3DRpeJDSE+ciuQ--