From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41616) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xobb0-0007jd-RY for qemu-devel@nongnu.org; Wed, 12 Nov 2014 12:20:50 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Xobat-0000hg-IQ for qemu-devel@nongnu.org; Wed, 12 Nov 2014 12:20:42 -0500 Received: from aserp1040.oracle.com ([141.146.126.69]:33579) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xobat-0000hZ-CE for qemu-devel@nongnu.org; Wed, 12 Nov 2014 12:20:35 -0500 Date: Wed, 12 Nov 2014 12:20:20 -0500 From: Konrad Rzeszutek Wilk Message-ID: <20141112172020.GC32634@laptop.dumpdata.com> References: <1415807116-8375-1-git-send-email-roger.pau@citrix.com> <54638307.1080500@eu.citrix.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <54638307.1080500@eu.citrix.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH for-xen-4.5] xen_disk: fix unmapping of persistent grants List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: George Dunlap Cc: Kevin Wolf , Stefano Stabellini , qemu-devel@nongnu.org, Stefan Hajnoczi , xen-devel@lists.xenproject.org, Roger Pau Monne On Wed, Nov 12, 2014 at 03:55:51PM +0000, George Dunlap wrote: > On 11/12/2014 03:45 PM, Roger Pau Monne wrote: > >This patch fixes two issues with persistent grants and the disk PV bac= kend > >(Qdisk): > > > > - Don't use batch mappings when using persistent grants, doing so pr= events > > unmapping single grants (the whole area has to be unmapped at once= ). > > - Unmap persistent grants before switching to the closed state, so t= he > > frontend can also free them. > > > >Signed-off-by: Roger Pau Monn=E9 > >Reported-and-Tested-by: George Dunlap > >Cc: Stefano Stabellini > >Cc: Kevin Wolf > >Cc: Stefan Hajnoczi > >Cc: George Dunlap >=20 > CC'ing Konrad and Stefano: This fixes a critical bug that should be a > blocker for the Xen 4.5 release. Without this, any backend using qdisk= for > a PV guest with pygrub (including qcow2 and vhd) will crash dom0. Changing the title to reflect that. Stefano? >=20 > -George >=20 > >--- > > hw/block/xen_disk.c | 35 ++++++++++++++++++++++++----------- > > 1 file changed, 24 insertions(+), 11 deletions(-) > > > >diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c > >index 231e9a7..1300c0a 100644 > >--- a/hw/block/xen_disk.c > >+++ b/hw/block/xen_disk.c > >@@ -43,8 +43,6 @@ > > /* ------------------------------------------------------------- */ > >-static int batch_maps =3D 0; > >- > > static int max_requests =3D 32; > > /* ------------------------------------------------------------- */ > >@@ -105,6 +103,7 @@ struct XenBlkDev { > > blkif_back_rings_t rings; > > int more_work; > > int cnt_map; > >+ bool batch_maps; > > /* request lists */ > > QLIST_HEAD(inflight_head, ioreq) inflight; > >@@ -309,7 +308,7 @@ static void ioreq_unmap(struct ioreq *ioreq) > > if (ioreq->num_unmap =3D=3D 0 || ioreq->mapped =3D=3D 0) { > > return; > > } > >- if (batch_maps) { > >+ if (ioreq->blkdev->batch_maps) { > > if (!ioreq->pages) { > > return; > > } > >@@ -386,7 +385,7 @@ static int ioreq_map(struct ioreq *ioreq) > > new_maps =3D ioreq->v.niov; > > } > >- if (batch_maps && new_maps) { > >+ if (ioreq->blkdev->batch_maps && new_maps) { > > ioreq->pages =3D xc_gnttab_map_grant_refs > > (gnt, new_maps, domids, refs, ioreq->prot); > > if (ioreq->pages =3D=3D NULL) { > >@@ -433,7 +432,7 @@ static int ioreq_map(struct ioreq *ioreq) > > */ > > grant =3D g_malloc0(sizeof(*grant)); > > new_maps--; > >- if (batch_maps) { > >+ if (ioreq->blkdev->batch_maps) { > > grant->page =3D ioreq->pages + (new_maps) * XC_PAGE_= SIZE; > > } else { > > grant->page =3D ioreq->page[new_maps]; > >@@ -718,7 +717,9 @@ static void blk_alloc(struct XenDevice *xendev) > > QLIST_INIT(&blkdev->freelist); > > blkdev->bh =3D qemu_bh_new(blk_bh, blkdev); > > if (xen_mode !=3D XEN_EMULATE) { > >- batch_maps =3D 1; > >+ blkdev->batch_maps =3D TRUE; > >+ } else { > >+ blkdev->batch_maps =3D FALSE; > > } > > if (xc_gnttab_set_max_grants(xendev->gnttabdev, > > MAX_GRANTS(max_requests, BLKIF_MAX_SEGMENTS_PER_REQUEST)= ) < 0) { > >@@ -923,6 +924,13 @@ static int blk_connect(struct XenDevice *xendev) > > } else { > > blkdev->feature_persistent =3D !!pers; > > } > >+ if (blkdev->feature_persistent) { > >+ /* > >+ * Disable batch maps, since that would prevent unmapping > >+ * single persistent grants. > >+ */ > >+ blkdev->batch_maps =3D FALSE; > >+ } > > blkdev->protocol =3D BLKIF_PROTOCOL_NATIVE; > > if (blkdev->xendev.protocol) { > >@@ -1000,6 +1008,16 @@ static void blk_disconnect(struct XenDevice *xe= ndev) > > blkdev->cnt_map--; > > blkdev->sring =3D NULL; > > } > >+ > >+ /* > >+ * Unmap persistent grants before switching to the closed state > >+ * so the frontend can free them. > >+ */ > >+ if (blkdev->feature_persistent) { > >+ g_tree_destroy(blkdev->persistent_gnts); > >+ assert(blkdev->persistent_gnt_count =3D=3D 0); > >+ blkdev->feature_persistent =3D FALSE; > >+ } > > } > > static int blk_free(struct XenDevice *xendev) > >@@ -1011,11 +1029,6 @@ static int blk_free(struct XenDevice *xendev) > > blk_disconnect(xendev); > > } > >- /* Free persistent grants */ > >- if (blkdev->feature_persistent) { > >- g_tree_destroy(blkdev->persistent_gnts); > >- } > >- > > while (!QLIST_EMPTY(&blkdev->freelist)) { > > ioreq =3D QLIST_FIRST(&blkdev->freelist); > > QLIST_REMOVE(ioreq, list); >=20