From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51445) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W63yX-0008HO-RU for qemu-devel@nongnu.org; Wed, 22 Jan 2014 15:00:42 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1W63yS-0001ZF-PF for qemu-devel@nongnu.org; Wed, 22 Jan 2014 15:00:37 -0500 Received: from paradis.irqsave.net ([62.212.105.220]:48076) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W63yS-0001Y1-1T for qemu-devel@nongnu.org; Wed, 22 Jan 2014 15:00:32 -0500 Date: Wed, 22 Jan 2014 21:00:26 +0100 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20140122200026.GC3053@irqsave.net> References: <1389968119-24771-1-git-send-email-kwolf@redhat.com> <1389968119-24771-18-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-18-git-send-email-kwolf@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v3 17/29] block: Generalise and optimise COR serialisation 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:07 (+0100), Kevin Wolf a =E9crit : > Change the API so that specific requests can be marked serialising. Onl= y I find the spelling "serialising" instead of "serializing" really odd sin= ce QEMU code is full of z like the word virtualization. Reviewed-by: Benoit Canet > these requests are checked for overlaps then. >=20 > This means that during a Copy on Read operation, not all requests > overlapping other requests are serialised any more, but only those that > actually overlap with the specific COR request. >=20 > Also remove COR from function and variable names because this > functionality can be useful in other contexts. >=20 > Signed-off-by: Kevin Wolf > Reviewed-by: Max Reitz > --- > block.c | 48 ++++++++++++++++++++++++++++-----------= -------- > include/block/block_int.h | 5 +++-- > 2 files changed, 32 insertions(+), 21 deletions(-) >=20 > diff --git a/block.c b/block.c > index d7156ce..efa8979 100644 > --- a/block.c > +++ b/block.c > @@ -2028,6 +2028,10 @@ int bdrv_commit_all(void) > */ > static void tracked_request_end(BdrvTrackedRequest *req) > { > + if (req->serialising) { > + req->bs->serialising_in_flight--; > + } > + > QLIST_REMOVE(req, list); > qemu_co_queue_restart_all(&req->wait_queue); > } > @@ -2042,10 +2046,11 @@ static void tracked_request_begin(BdrvTrackedRe= quest *req, > { > *req =3D (BdrvTrackedRequest){ > .bs =3D bs, > - .offset =3D offset, > - .bytes =3D bytes, > - .is_write =3D is_write, > - .co =3D qemu_coroutine_self(), > + .offset =3D offset, > + .bytes =3D bytes, > + .is_write =3D is_write, > + .co =3D qemu_coroutine_self(), > + .serialising =3D false, > }; > =20 > qemu_co_queue_init(&req->wait_queue); > @@ -2053,6 +2058,14 @@ static void tracked_request_begin(BdrvTrackedReq= uest *req, > QLIST_INSERT_HEAD(&bs->tracked_requests, req, list); > } > =20 > +static void mark_request_serialising(BdrvTrackedRequest *req) > +{ > + if (!req->serialising) { > + req->bs->serialising_in_flight++; > + req->serialising =3D true; > + } > +} > + > /** > * Round a region to cluster boundaries > */ > @@ -2105,26 +2118,31 @@ static bool tracked_request_overlaps(BdrvTracke= dRequest *req, > return true; > } > =20 > -static void coroutine_fn wait_for_overlapping_requests(BlockDriverStat= e *bs, > - BdrvTrackedRequest *self, int64_t offset, unsigned int bytes) > +static void coroutine_fn wait_serialising_requests(BdrvTrackedRequest = *self) > { > + BlockDriverState *bs =3D self->bs; > BdrvTrackedRequest *req; > int64_t cluster_offset; > unsigned int cluster_bytes; > bool retry; > =20 > + if (!bs->serialising_in_flight) { > + return; > + } > + > /* If we touch the same cluster it counts as an overlap. This gua= rantees > * that allocating writes will be serialized and not race with eac= h other > * for the same cluster. For example, in copy-on-read it ensures = that the > * CoR read and write operations are atomic and guest writes canno= t > * interleave between them. > */ > - round_bytes_to_clusters(bs, offset, bytes, &cluster_offset, &clust= er_bytes); > + round_bytes_to_clusters(bs, self->offset, self->bytes, > + &cluster_offset, &cluster_bytes); > =20 > do { > retry =3D false; > QLIST_FOREACH(req, &bs->tracked_requests, list) { > - if (req =3D=3D self) { > + if (req =3D=3D self || (!req->serialising && !self->serial= ising)) { > continue; > } > if (tracked_request_overlaps(req, cluster_offset, cluster_= bytes)) { > @@ -2743,12 +2761,10 @@ static int coroutine_fn bdrv_aligned_preadv(Blo= ckDriverState *bs, > =20 > /* Handle Copy on Read and associated serialisation */ > if (flags & BDRV_REQ_COPY_ON_READ) { > - bs->copy_on_read_in_flight++; > + mark_request_serialising(req); > } > =20 > - if (bs->copy_on_read_in_flight) { > - wait_for_overlapping_requests(bs, req, offset, bytes); > - } > + wait_serialising_requests(req); > =20 > if (flags & BDRV_REQ_COPY_ON_READ) { > int pnum; > @@ -2797,10 +2813,6 @@ static int coroutine_fn bdrv_aligned_preadv(Bloc= kDriverState *bs, > } > =20 > out: > - if (flags & BDRV_REQ_COPY_ON_READ) { > - bs->copy_on_read_in_flight--; > - } > - > return ret; > } > =20 > @@ -2999,9 +3011,7 @@ static int coroutine_fn bdrv_aligned_pwritev(Bloc= kDriverState *bs, > assert((offset & (BDRV_SECTOR_SIZE - 1)) =3D=3D 0); > assert((bytes & (BDRV_SECTOR_SIZE - 1)) =3D=3D 0); > =20 > - if (bs->copy_on_read_in_flight) { > - wait_for_overlapping_requests(bs, req, offset, bytes); > - } > + wait_serialising_requests(req); > =20 > ret =3D notifier_with_return_list_notify(&bs->before_write_notifie= rs, req); > =20 > diff --git a/include/block/block_int.h b/include/block/block_int.h > index 97a4d23..d8443df 100644 > --- a/include/block/block_int.h > +++ b/include/block/block_int.h > @@ -60,6 +60,7 @@ typedef struct BdrvTrackedRequest { > int64_t offset; > unsigned int bytes; > bool is_write; > + bool serialising; > QLIST_ENTRY(BdrvTrackedRequest) list; > Coroutine *co; /* owner, used for deadlock detection */ > CoQueue wait_queue; /* coroutines blocked on this request */ > @@ -296,8 +297,8 @@ struct BlockDriverState { > /* Callback before write request is processed */ > NotifierWithReturnList before_write_notifiers; > =20 > - /* number of in-flight copy-on-read requests */ > - unsigned int copy_on_read_in_flight; > + /* number of in-flight serialising requests */ > + unsigned int serialising_in_flight; > =20 > /* I/O throttling */ > ThrottleState throttle_state; > --=20 > 1.8.1.4 >=20 >=20