From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([140.186.70.92]:34336)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <mtosatti@redhat.com>) id 1RXbSn-0005ZV-FO
	for qemu-devel@nongnu.org; Mon, 05 Dec 2011 11:32:28 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <mtosatti@redhat.com>) id 1RXbSm-0002ET-BY
	for qemu-devel@nongnu.org; Mon, 05 Dec 2011 11:32:21 -0500
Received: from mx1.redhat.com ([209.132.183.28]:12155)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <mtosatti@redhat.com>) id 1RXbSm-0002E6-4p
	for qemu-devel@nongnu.org; Mon, 05 Dec 2011 11:32:20 -0500
Date: Mon, 5 Dec 2011 14:31:51 -0200
From: Marcelo Tosatti <mtosatti@redhat.com>
Message-ID: <20111205163151.GA1998@amt.cnet>
References: <1322048878-26348-1-git-send-email-stefanha@linux.vnet.ibm.com>
	<1322048878-26348-4-git-send-email-stefanha@linux.vnet.ibm.com>
	<20111205160931.GA31162@amt.cnet>
	<CAJSP0QXKZhneObhWtYRQvV8Khw5fmVvsgQ5tNXqcE=Z6JEeQXQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
In-Reply-To: <CAJSP0QXKZhneObhWtYRQvV8Khw5fmVvsgQ5tNXqcE=Z6JEeQXQ@mail.gmail.com>
Content-Transfer-Encoding: quoted-printable
Subject: Re: [Qemu-devel] [PATCH v4 3/8] block: add request tracking
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Stefan Hajnoczi <stefanha@gmail.com>
Cc: Kevin Wolf <kwolf@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>, qemu-devel@nongnu.org

On Mon, Dec 05, 2011 at 04:20:55PM +0000, Stefan Hajnoczi wrote:
> On Mon, Dec 5, 2011 at 4:09 PM, Marcelo Tosatti <mtosatti@redhat.com> w=
rote:
> > On Wed, Nov 23, 2011 at 11:47:53AM +0000, Stefan Hajnoczi wrote:
> > On earlier discussion, you replied to me:
> >
> > "
> >>> =A0 =A0 =A0req =3D tracked_request_add(bs, sector_num, nb_sectors, =
false);
> >>
> >> The tracked request should include cluster round info?
> >
> > When checking A and B for overlap, only one of them needs to be
> > rounded in order for overlap detection to be correct. =A0Therefore we
> > can store the original request [start, length) in tracked_requests an=
d
> > only round the new request.
> > "
> >
> > The problem AFAICS is this:
> >
> > - Store a non-cluster-aligned request in the tracked request list.
> > - Wait on that non-cluster-aligned request
> > =A0(wait_for_overlapping_requests).
> > - Submit cluster-aligned request for COR request.
> >
> > So, the tracked request list does not properly reflect the in-flight
> > COR requests. Which can result in:
> >
> > 1) guest reads sector 10.
> > 2) <sector_num=3D10,nb_sectors=3D2> added to tracked request list.
> > 3) COR code submits read for <sector_num=3D10,nb_sectors=3D2+cluster_=
align>
>=20
> It will also round down to sector_num=3D0 when cluster size is 128
> sectors (e.g. qcow2 and qed).
>=20
> > 4) unrelated guest operation writes to sector 13, nb_sectors=3D1. Tha=
t is
> > allowed to proceed without waiting because tracked request list does =
not
> > reflect what COR in-flight requests.
>=20
> The tracked request list has <sector_num=3D10, nb_sectors=3D2> and the
> candidate write request is <sector_num=3D13, nb_sectors=3D1>.
>=20
> In wait_for_overlapping_requests() we round the candidate request to
> <sector_num=3D0, nb_sectors=3Dcluster_size>.  This rounded request does
> overlap <sector_num=3D10, sectors=3D2> so it will need to wait.
>=20
> Therefore CoR and write will not execute at the same time.
>=20
> Does this make more sense?
>=20
> Stefan

Ah yes same mistake on my part, again. Thanks.