From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57226) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W64JA-000222-OP for qemu-devel@nongnu.org; Wed, 22 Jan 2014 15:22:01 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1W64J4-0000b5-Ms for qemu-devel@nongnu.org; Wed, 22 Jan 2014 15:21:56 -0500 Received: from paradis.irqsave.net ([62.212.105.220]:48085) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W64J4-0000aw-Bq for qemu-devel@nongnu.org; Wed, 22 Jan 2014 15:21:50 -0500 Date: Wed, 22 Jan 2014 21:21:49 +0100 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20140122202149.GE3053@irqsave.net> References: <1389968119-24771-1-git-send-email-kwolf@redhat.com> <1389968119-24771-20-git-send-email-kwolf@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <1389968119-24771-20-git-send-email-kwolf@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v3 19/29] block: Allow wait_serialising_requests() at any point List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: pl@kamp.de, qemu-devel@nongnu.org, mreitz@redhat.com, stefanha@redhat.com, pbonzini@redhat.com, xiawenc@linux.vnet.ibm.com Le Friday 17 Jan 2014 =E0 15:15:09 (+0100), Kevin Wolf a =E9crit : > We can only have a single wait_serialising_requests() call per request > because otherwise we can run into deadlocks where requests are waiting > for each other. The same is true when wait_serialising_requests() is no= t > at the very beginning of a request, so that other requests can be issue= d > between the start of the tracking and wait_serialising_requests(). >=20 > Fix this by changing wait_serialising_requests() to ignore requests tha= t > are already (directly or indirectly) waiting for the calling request. >=20 > Signed-off-by: Kevin Wolf > Reviewed-by: Max Reitz > --- > block.c | 13 ++++++++++--- > include/block/block_int.h | 2 ++ > 2 files changed, 12 insertions(+), 3 deletions(-) >=20 > diff --git a/block.c b/block.c > index e72966a..55e8c69 100644 > --- a/block.c > +++ b/block.c > @@ -2148,9 +2148,16 @@ static void coroutine_fn wait_serialising_reques= ts(BdrvTrackedRequest *self) > */ > assert(qemu_coroutine_self() !=3D req->co); > =20 > - qemu_co_queue_wait(&req->wait_queue); > - retry =3D true; > - break; > + /* If the request is already (indirectly) waiting for = us, or > + * will wait for us as soon as it wakes up, then just = go on > + * (instead of producing a deadlock in the former case= ). */ > + if (!req->waiting_for) { > + self->waiting_for =3D req; > + qemu_co_queue_wait(&req->wait_queue); > + self->waiting_for =3D NULL; > + retry =3D true; > + break; > + } > } > } > } while (retry); > diff --git a/include/block/block_int.h b/include/block/block_int.h > index ccd2c68..fdf0e0b 100644 > --- a/include/block/block_int.h > +++ b/include/block/block_int.h > @@ -68,6 +68,8 @@ typedef struct BdrvTrackedRequest { > QLIST_ENTRY(BdrvTrackedRequest) list; > Coroutine *co; /* owner, used for deadlock detection */ > CoQueue wait_queue; /* coroutines blocked on this request */ > + > + struct BdrvTrackedRequest *waiting_for; > } BdrvTrackedRequest; > =20 > struct BlockDriver { > --=20 > 1.8.1.4 >=20 >=20 Reviewed-by: Benoit Canet