From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33363) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xoso3-0007GN-P9 for qemu-devel@nongnu.org; Thu, 13 Nov 2014 06:43:24 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Xosny-00071n-4E for qemu-devel@nongnu.org; Thu, 13 Nov 2014 06:43:19 -0500 Received: from mx1.redhat.com ([209.132.183.28]:38985) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xosnx-00071j-TU for qemu-devel@nongnu.org; Thu, 13 Nov 2014 06:43:14 -0500 Date: Thu, 13 Nov 2014 12:42:57 +0100 From: Kevin Wolf Message-ID: <20141113114257.GB3933@noname.redhat.com> References: <1415807116-8375-1-git-send-email-roger.pau@citrix.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] xen_disk: fix unmapping of persistent grants List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefano Stabellini Cc: George Dunlap , xen-devel@lists.xenproject.org, qemu-devel@nongnu.org, Stefan Hajnoczi , Roger Pau Monne Am 12.11.2014 um 18:41 hat Stefano Stabellini geschrieben: > On Wed, 12 Nov 2014, Roger Pau Monne wrote: > > This patch fixes two issues with persistent grants and the disk PV ba= ckend > > (Qdisk): > >=20 > > - 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= ). >=20 > The real issue is that destroy_grant cannot work with batch_maps. > One could reimplement destroy_grant to build a single array with all th= e > grants to unmap and make a single xc_gnttab_munmap call. >=20 > Do you think that would be feasible? >=20 > Performance wise, it would certainly be better. >=20 >=20 > > - 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 > > --- > > hw/block/xen_disk.c | 35 ++++++++++++++++++++++++----------- > > 1 file changed, 24 insertions(+), 11 deletions(-) > >=20 > > 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 @@ > > =20 > > /* ------------------------------------------------------------- */ > > =20 > > -static int batch_maps =3D 0; > > - > > static int max_requests =3D 32; > > =20 > > /* ------------------------------------------------------------- */ > > @@ -105,6 +103,7 @@ struct XenBlkDev { > > blkif_back_rings_t rings; > > int more_work; > > int cnt_map; > > + bool batch_maps; > > =20 > > /* 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; > > } > > =20 > > - 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; > > } >=20 > true and false, lower capitals Or just blkdev->batch_maps =3D (xen_mode !=3D XEN_EMULATE); Kevin