From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55269) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XOpiV-00021e-94 for qemu-devel@nongnu.org; Tue, 02 Sep 2014 11:10:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XOpiP-0001kC-3V for qemu-devel@nongnu.org; Tue, 02 Sep 2014 11:09:55 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38394) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XOpiO-0001ij-K9 for qemu-devel@nongnu.org; Tue, 02 Sep 2014 11:09:49 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s82F9mD4022923 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Tue, 2 Sep 2014 11:09:48 -0400 Date: Tue, 2 Sep 2014 16:09:46 +0100 From: Stefan Hajnoczi Message-ID: <20140902150946.GL29067@stefanha-thinkpad.redhat.com> References: <1409107756-5967-1-git-send-email-famz@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="Qgd2S+2VS1hsWwXW" Content-Disposition: inline In-Reply-To: <1409107756-5967-1-git-send-email-famz@redhat.com> Subject: Re: [Qemu-devel] [PATCH v3 0/8] block: Asynchronous request cancellation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng Cc: Kevin Wolf , Paolo Bonzini , qemu-devel@nongnu.org --Qgd2S+2VS1hsWwXW Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Aug 27, 2014 at 10:49:08AM +0800, Fam Zheng wrote: > v3: Drop "RFC". > Improvements according to Paolo's comments: > 05: Just use THREAD_DONE and ret =3D -ECANCELED in thread-pool.c > 06: Don't check dbs->cancelled for twice. > Don't set dbs->acb to NULL. >=20 > v2: Drop the unfinished scsi part, which was broken in v1. (Paolo) > Add refcnt in BlockDriverAIOCB to maintain invariant of acb availabil= ity in > bdrv_aio_cancel_async. (Paolo) > Drop blkdebug change. (Stefan) >=20 > This series adds a new block layer API: >=20 > void bdrv_aio_cancel_async(BlockDriverAIOCB *acb); >=20 > which is similar to existing bdrv_aio_cancel in that it cancels an AIO re= quest, > but different that it doesn't block until the request is completely cance= lled > or done. >=20 > More importantly, the completion callback, BlockDriverAIOCB.cb, is guaran= teed > to be called, so that the cb can take care of resource releasing and stat= us > reporting to guest, etc. >=20 > In the following work, scsi emulation code will be shifted to use the asy= nc > cancelling. >=20 > One major benefit would be that when guest tries to cancel a request, whe= re the > request cannot be cancelled easily, (due to throttled BlockDriverState, a= lost > connection, or a large request queue), we don't need to block the whole v= m with > a busy loop, which is how bdrv_aio_cancel is implemented now. >=20 > A test case that is easy to reproduce is, throttle a scsi-disk to a very = low > limit, for example 50 bps, then stress the guest block device with dd or = fio. >=20 > Currently, the vm will quickly hang when it loses patience and send a tmf > command to cancel the request, at which point we will busy wait in > bdrv_aio_cancel, until the request is slowly spit out from throttled_reqs. >=20 > Later, we will change scsi device code to make this asynchronous, on top = of > bdrv_aio_cancel_async. We need to get rid of .bdrv_aio_cancel().=A0 Keeping both =2Ebdrv_aio_cancel() and .bdrv_aio_cancel_async() around is problematic because they have slightly different semantics. This patch series makes block driver cancellation more complex because we support both approaches :(. Could we do something like: void bdrv_aio_cancel(BdrvAIOCB *acb) { bdrv_aiocb_ref(acb); bdrv_aio_cancel_async(acb); while (acb->ref > 1) { aio_poll(bdrv_get_aio_context(acb->bs), true); } bdrv_aiocb_release(acb); } (pseudo-code) Then .bdrv_aio_cancel() should be deleted. Stefan --Qgd2S+2VS1hsWwXW Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJUBd26AAoJEJykq7OBq3PI8V0H/RICXscCnOAMQGqGBzI10uKK EKYTUUF/766uukwo6tVgXgyWaVFZLoM5InTRwpOFq7VFroxXud9BGp/A1mB+Va3e v1+oJ6nOnbEvk+uChEknFVuxDHV7n7a+yZmcT0ec6XpkgD0x2ROmDBx3eD5UziBF jEemhqK+cBP4YtS50MkTa6cGJSEg+et/ujYJ7nxPej1nmISJ+W9SS32ZqayVoTYs zStKNJrr7ak1SmJfYJGcNF5QIts/O99g1T9X2G8AuDTRSCtE4SsaIwuPZP1AQwsR eUSpR5XsKv9BiqyWytOv7fGjS1puS6ZNWBCqRae9ZnoQCT9CRK4kV7yhkUnV/SE= =jnOD -----END PGP SIGNATURE----- --Qgd2S+2VS1hsWwXW--