From patchwork Fri Nov 14 11:30:10 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Stefano Stabellini X-Patchwork-Id: 40813 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-lb0-f200.google.com (mail-lb0-f200.google.com [209.85.217.200]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 55931244FC for ; Fri, 14 Nov 2014 11:31:50 +0000 (UTC) Received: by mail-lb0-f200.google.com with SMTP id f15sf9197473lbj.7 for ; Fri, 14 Nov 2014 03:31:49 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:from:to:date:message-id:in-reply-to :references:mime-version:cc:subject:precedence:list-id :list-unsubscribe:list-archive:list-post:list-help:list-subscribe :errors-to:sender:x-original-sender :x-original-authentication-results:mailing-list:content-type :content-transfer-encoding; bh=MseCp6ZbTLIpSIgWwo3EwMXqVyjtBhcc2joN9/zDEC0=; b=Vyg1hgyARzBMhvhEqaq0BHA22BR/A83znu0Dy0GC8OqUDH174NomvvF6Bjk9Yx5YjG BZfPwfX2u6fQODCgBrcSEtxCgztJSRLhBP4vdAH4wS39ZUBTYTDoyhwNzHuJ3AZPEbgl gSuiLJ3FACTzclkq7355UHkEu9j0t90I3nSQ/a5lhBNL3rDgeq4jKdDPz64GUjNax9HZ OhHF59JF2tZxBt32qnkvTR48swEeVX9EjB3SFDu7OhmszCN5EyoY/lf7nGnqd1+D/y90 oUotC+YtJ3ZSo2cYnHnI5ILqf88VWpK7+uOOLR87qldR+FontNuXsUrHo2cCBMsB/Haa /ATQ== X-Gm-Message-State: ALoCoQk6qgWnT2xxJi/NfjQvAWiIMZnz3u5qIM/ZhVZFvBW4mvecAIk/IE8w3CsFgrakI2yoLIWE X-Received: by 10.112.162.6 with SMTP id xw6mr10339154lbb.4.1415964709314; Fri, 14 Nov 2014 03:31:49 -0800 (PST) X-BeenThere: patchwork-forward@linaro.org Received: by 10.152.42.137 with SMTP id o9ls755217lal.17.gmail; Fri, 14 Nov 2014 03:31:49 -0800 (PST) X-Received: by 10.112.136.37 with SMTP id px5mr7985669lbb.36.1415964709037; Fri, 14 Nov 2014 03:31:49 -0800 (PST) Received: from mail-la0-f42.google.com (mail-la0-f42.google.com. [209.85.215.42]) by mx.google.com with ESMTPS id xs11si34460793lac.7.2014.11.14.03.31.49 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Fri, 14 Nov 2014 03:31:49 -0800 (PST) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.215.42 as permitted sender) client-ip=209.85.215.42; Received: by mail-la0-f42.google.com with SMTP id s18so992740lam.29 for ; Fri, 14 Nov 2014 03:31:48 -0800 (PST) X-Received: by 10.152.29.8 with SMTP id f8mr7764544lah.56.1415964708798; Fri, 14 Nov 2014 03:31:48 -0800 (PST) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.112.184.201 with SMTP id ew9csp748522lbc; Fri, 14 Nov 2014 03:31:48 -0800 (PST) X-Received: by 10.140.46.75 with SMTP id j69mr2946056qga.106.1415964707244; Fri, 14 Nov 2014 03:31:47 -0800 (PST) Received: from lists.gnu.org (lists.gnu.org. [2001:4830:134:3::11]) by mx.google.com with ESMTPS id s38si50587121qge.85.2014.11.14.03.31.46 for (version=TLSv1 cipher=RC4-SHA bits=128/128); Fri, 14 Nov 2014 03:31:47 -0800 (PST) Received-SPF: pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 2001:4830:134:3::11 as permitted sender) client-ip=2001:4830:134:3::11; Received: from localhost ([::1]:35956 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XpF6Q-0000ai-4q for patch@linaro.org; Fri, 14 Nov 2014 06:31:46 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58336) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XpF5M-0007lU-UO for qemu-devel@nongnu.org; Fri, 14 Nov 2014 06:30:45 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XpF5I-0008Gx-GE for qemu-devel@nongnu.org; Fri, 14 Nov 2014 06:30:40 -0500 Received: from smtp.citrix.com ([66.165.176.89]:9123) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XpF5H-0008Gs-Pg for qemu-devel@nongnu.org; Fri, 14 Nov 2014 06:30:36 -0500 X-IronPort-AV: E=Sophos;i="5.07,385,1413244800"; d="scan'208";a="191345667" Received: from ukmail1.uk.xensource.com (10.80.16.128) by smtprelay.citrix.com (10.13.107.79) with Microsoft SMTP Server id 14.3.181.6; Fri, 14 Nov 2014 06:30:33 -0500 Received: from kaball.uk.xensource.com ([10.80.2.59]) by ukmail1.uk.xensource.com with esmtp (Exim 4.69) (envelope-from ) id 1XpF59-0006kB-LY; Fri, 14 Nov 2014 11:30:27 +0000 From: Stefano Stabellini To: Date: Fri, 14 Nov 2014 11:30:10 +0000 Message-ID: <1415964611-20838-2-git-send-email-stefano.stabellini@eu.citrix.com> X-Mailer: git-send-email 1.7.9.5 In-Reply-To: References: MIME-Version: 1.0 X-DLP: MIA2 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 66.165.176.89 Cc: Kevin Wolf , peter.maydell@linaro.org, xen-devel@lists.xensource.com, Stefano Stabellini , George Dunlap , Konrad Rzeszutek Wilk , Stefan Hajnoczi , Roger Pau Monne Subject: [Qemu-devel] [PULL for-2.2 2/2] xen_disk: fix unmapping of persistent grants X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: , List-Help: , List-Subscribe: , Errors-To: qemu-devel-bounces+patch=linaro.org@nongnu.org Sender: qemu-devel-bounces+patch=linaro.org@nongnu.org X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: stefano.stabellini@eu.citrix.com X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.215.42 as permitted sender) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org X-Google-Group-Id: 836684582541 From: Roger Pau Monne This patch fixes two issues with persistent grants and the disk PV backend (Qdisk): - Keep track of memory regions where persistent grants have been mapped since we need to unmap them as a whole. It is not possible to unmap a single grant if it has been batch-mapped. A new check has also been added to make sure persistent grants are only used if the whole mapped region can be persistently mapped in the batch_maps case. - Unmap persistent grants before switching to the closed state, so the frontend can also free them. Signed-off-by: Roger Pau Monné Reported-by: George Dunlap Cc: Stefano Stabellini Cc: Kevin Wolf Cc: Stefan Hajnoczi Cc: George Dunlap Cc: Konrad Rzeszutek Wilk --- hw/block/xen_disk.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 66 insertions(+), 6 deletions(-) diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c index 231e9a7..21842a0 100644 --- a/hw/block/xen_disk.c +++ b/hw/block/xen_disk.c @@ -59,6 +59,13 @@ struct PersistentGrant { typedef struct PersistentGrant PersistentGrant; +struct PersistentRegion { + void *addr; + int num; +}; + +typedef struct PersistentRegion PersistentRegion; + struct ioreq { blkif_request_t req; int16_t status; @@ -118,6 +125,7 @@ struct XenBlkDev { gboolean feature_discard; gboolean feature_persistent; GTree *persistent_gnts; + GSList *persistent_regions; unsigned int persistent_gnt_count; unsigned int max_grants; @@ -177,6 +185,23 @@ static void destroy_grant(gpointer pgnt) g_free(grant); } +static void remove_persistent_region(gpointer data, gpointer dev) +{ + PersistentRegion *region = data; + struct XenBlkDev *blkdev = dev; + XenGnttab gnt = blkdev->xendev.gnttabdev; + + if (xc_gnttab_munmap(gnt, region->addr, region->num) != 0) { + xen_be_printf(&blkdev->xendev, 0, + "xc_gnttab_munmap region %p failed: %s\n", + region->addr, strerror(errno)); + } + xen_be_printf(&blkdev->xendev, 3, + "unmapped grant region %p with %d pages\n", + region->addr, region->num); + g_free(region); +} + static struct ioreq *ioreq_start(struct XenBlkDev *blkdev) { struct ioreq *ioreq = NULL; @@ -343,6 +368,7 @@ static int ioreq_map(struct ioreq *ioreq) void *page[BLKIF_MAX_SEGMENTS_PER_REQUEST]; int i, j, new_maps = 0; PersistentGrant *grant; + PersistentRegion *region; /* domids and refs variables will contain the information necessary * to map the grants that are needed to fulfill this request. * @@ -421,7 +447,22 @@ static int ioreq_map(struct ioreq *ioreq) } } } - if (ioreq->blkdev->feature_persistent) { + if (ioreq->blkdev->feature_persistent && new_maps != 0 && + (!batch_maps || (ioreq->blkdev->persistent_gnt_count + new_maps <= + ioreq->blkdev->max_grants))) { + /* + * If we are using persistent grants and batch mappings only + * add the new maps to the list of persistent grants if the whole + * area can be persistently mapped. + */ + if (batch_maps) { + region = g_malloc0(sizeof(*region)); + region->addr = ioreq->pages; + region->num = new_maps; + ioreq->blkdev->persistent_regions = g_slist_append( + ioreq->blkdev->persistent_regions, + region); + } while ((ioreq->blkdev->persistent_gnt_count < ioreq->blkdev->max_grants) && new_maps) { /* Go through the list of newly mapped grants and add as many @@ -447,6 +488,7 @@ static int ioreq_map(struct ioreq *ioreq) grant); ioreq->blkdev->persistent_gnt_count++; } + assert(!batch_maps || new_maps == 0); } for (i = 0; i < ioreq->v.niov; i++) { ioreq->v.iov[i].iov_base += (uintptr_t)page[i]; @@ -971,7 +1013,10 @@ static int blk_connect(struct XenDevice *xendev) blkdev->max_grants = max_requests * BLKIF_MAX_SEGMENTS_PER_REQUEST; blkdev->persistent_gnts = g_tree_new_full((GCompareDataFunc)int_cmp, NULL, NULL, + batch_maps ? + (GDestroyNotify)g_free : (GDestroyNotify)destroy_grant); + blkdev->persistent_regions = NULL; blkdev->persistent_gnt_count = 0; } @@ -1000,6 +1045,26 @@ static void blk_disconnect(struct XenDevice *xendev) blkdev->cnt_map--; blkdev->sring = NULL; } + + /* + * Unmap persistent grants before switching to the closed state + * so the frontend can free them. + * + * In the !batch_maps case g_tree_destroy will take care of unmapping + * the grant, but in the batch_maps case we need to iterate over every + * region in persistent_regions and unmap it. + */ + if (blkdev->feature_persistent) { + g_tree_destroy(blkdev->persistent_gnts); + assert(batch_maps || blkdev->persistent_gnt_count == 0); + if (batch_maps) { + blkdev->persistent_gnt_count = 0; + g_slist_foreach(blkdev->persistent_regions, + (GFunc)remove_persistent_region, blkdev); + g_slist_free(blkdev->persistent_regions); + } + blkdev->feature_persistent = false; + } } static int blk_free(struct XenDevice *xendev) @@ -1011,11 +1076,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 = QLIST_FIRST(&blkdev->freelist); QLIST_REMOVE(ioreq, list);