From patchwork Wed Jul 17 19:33:31 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tamas K Lengyel X-Patchwork-Id: 11048181 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id BF0B26C5 for ; Wed, 17 Jul 2019 19:35:28 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B04F128784 for ; Wed, 17 Jul 2019 19:35:28 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id A408428686; Wed, 17 Jul 2019 19:35:28 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 304FE28686 for ; Wed, 17 Jul 2019 19:35:28 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1hnpgf-0000Hd-01; Wed, 17 Jul 2019 19:34:00 +0000 Received: from all-amaz-eas1.inumbo.com ([34.197.232.57] helo=us1-amaz-eas2.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1hnpgd-0000HL-KS for xen-devel@lists.xenproject.org; Wed, 17 Jul 2019 19:33:59 +0000 X-Inumbo-ID: d0043070-a8c9-11e9-be02-ebb5fa9022dc Received: from mail-io1-f68.google.com (unknown [209.85.166.68]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTPS id d0043070-a8c9-11e9-be02-ebb5fa9022dc; Wed, 17 Jul 2019 19:33:54 +0000 (UTC) Received: by mail-io1-f68.google.com with SMTP id k20so47594701ios.10 for ; Wed, 17 Jul 2019 12:33:54 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=JBw7l33La8xVqfg8L17ATjz/BcMg+hI1A6l2yXVZeo0=; b=Y3zTD7jqvwRSnSm5ATNyLUuheKGYsi1AD5YmxIE/F4OQUc3Q4wcfTT6cmG3Dgr3o58 euNHSEUvjBEfKtQ+y7v02eFIH8YF9WE1H3KdjRMCSuQU0BWBOMkYt2H7v5HWv66s6xBn eNC1rJbvY82Bbx81fevk2vOu9TDayhssYJRIqDgYU+JNlHwpX+9nU6ubSx3BIANUMb5R J7xySNEulGo4+EBR++qrAnrV9YzAmcoQLhHMvEjN7pg0qBBs/8kMlC4XhNojR4H1HLx3 FBsOkXVwtOVk4dmpPG9/mFBQqueKq1qRxTbfEXiway20YIjwAMIJkVM3P7/AHO0pxcrx n/oA== X-Gm-Message-State: APjAAAVis7ZJD6SQiY0iK/X0JkX513j17YvQLIHY8YkrA6cg+lM9zx60 ULIt25LZZOfQOfbO73QYFOsUExKq X-Google-Smtp-Source: APXvYqxg5NYQFGxMFuUPXTARLsjdGTi74JswUYe2OqezGVQUMo2s4PVGcQioR4sbN17+sHslUc7/+g== X-Received: by 2002:a6b:b985:: with SMTP id j127mr38797426iof.186.1563392033796; Wed, 17 Jul 2019 12:33:53 -0700 (PDT) Received: from l1.lan (c-71-205-12-124.hsd1.co.comcast.net. [71.205.12.124]) by smtp.gmail.com with ESMTPSA id v13sm21743421ioq.13.2019.07.17.12.33.52 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Wed, 17 Jul 2019 12:33:53 -0700 (PDT) From: Tamas K Lengyel To: xen-devel@lists.xenproject.org Date: Wed, 17 Jul 2019 13:33:31 -0600 Message-Id: <20190717193335.11991-2-tamas@tklengyel.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20190717193335.11991-1-tamas@tklengyel.com> References: <20190717193335.11991-1-tamas@tklengyel.com> MIME-Version: 1.0 Subject: [Xen-devel] [PATCH v6 1/5] x86/mem_sharing: reorder when pages are unlocked and released X-BeenThere: xen-devel@lists.xenproject.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Cc: Tamas K Lengyel , Wei Liu , George Dunlap , Andrew Cooper , Jan Beulich , Roger Pau Monne Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" X-Virus-Scanned: ClamAV using ClamSMTP Calling _put_page_type while also holding the page_lock for that page can cause a deadlock. The comment being dropped is incorrect since it's now out-of-date. Signed-off-by: Tamas K Lengyel Cc: Jan Beulich Cc: Andrew Cooper Cc: George Dunlap Cc: Wei Liu Cc: Roger Pau Monne --- v6: rebase on staging v5: BUG_ON early before releasing references --- xen/arch/x86/mm/mem_sharing.c | 40 +++++++++++------------------------ 1 file changed, 12 insertions(+), 28 deletions(-) diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c index 58d9157fa8..6c033d1488 100644 --- a/xen/arch/x86/mm/mem_sharing.c +++ b/xen/arch/x86/mm/mem_sharing.c @@ -648,10 +648,6 @@ static int page_make_private(struct domain *d, struct page_info *page) return -EBUSY; } - /* We can only change the type if count is one */ - /* Because we are locking pages individually, we need to drop - * the lock here, while the page is typed. We cannot risk the - * race of page_unlock and then put_page_type. */ expected_type = (PGT_shared_page | PGT_validated | PGT_locked | 2); if ( page->u.inuse.type_info != expected_type ) { @@ -660,12 +656,11 @@ static int page_make_private(struct domain *d, struct page_info *page) return -EEXIST; } + mem_sharing_page_unlock(page); + /* Drop the final typecount */ put_page_and_type(page); - /* Now that we've dropped the type, we can unlock */ - mem_sharing_page_unlock(page); - /* Change the owner */ ASSERT(page_get_owner(page) == dom_cow); page_set_owner(page, d); @@ -900,6 +895,7 @@ static int share_pages(struct domain *sd, gfn_t sgfn, shr_handle_t sh, p2m_type_t smfn_type, cmfn_type; struct two_gfns tg; struct rmap_iterator ri; + unsigned long put_count = 0; get_two_gfns(sd, sgfn, &smfn_type, NULL, &smfn, cd, cgfn, &cmfn_type, NULL, &cmfn, 0, &tg); @@ -964,15 +960,6 @@ static int share_pages(struct domain *sd, gfn_t sgfn, shr_handle_t sh, goto err_out; } - /* Acquire an extra reference, for the freeing below to be safe. */ - if ( !get_page(cpage, dom_cow) ) - { - ret = -EOVERFLOW; - mem_sharing_page_unlock(secondpg); - mem_sharing_page_unlock(firstpg); - goto err_out; - } - /* Merge the lists together */ rmap_seed_iterator(cpage, &ri); while ( (gfn = rmap_iterate(cpage, &ri)) != NULL) @@ -984,13 +971,14 @@ static int share_pages(struct domain *sd, gfn_t sgfn, shr_handle_t sh, * Don't change the type of rmap for the client page. */ rmap_del(gfn, cpage, 0); rmap_add(gfn, spage); - put_page_and_type(cpage); + put_count++; d = get_domain_by_id(gfn->domain); BUG_ON(!d); BUG_ON(set_shared_p2m_entry(d, gfn->gfn, smfn)); put_domain(d); } ASSERT(list_empty(&cpage->sharing->gfns)); + BUG_ON(!put_count); /* Clear the rest of the shared state */ page_sharing_dispose(cpage); @@ -1001,7 +989,9 @@ static int share_pages(struct domain *sd, gfn_t sgfn, shr_handle_t sh, /* Free the client page */ put_page_alloc_ref(cpage); - put_page(cpage); + + while ( put_count-- ) + put_page_and_type(cpage); /* We managed to free a domain page. */ atomic_dec(&nr_shared_mfns); @@ -1165,19 +1155,13 @@ int __mem_sharing_unshare_page(struct domain *d, { if ( !last_gfn ) mem_sharing_gfn_destroy(page, d, gfn_info); - put_page_and_type(page); + mem_sharing_page_unlock(page); + if ( last_gfn ) - { - if ( !get_page(page, dom_cow) ) - { - put_gfn(d, gfn); - domain_crash(d); - return -EOVERFLOW; - } put_page_alloc_ref(page); - put_page(page); - } + + put_page_and_type(page); put_gfn(d, gfn); return 0;