From patchwork Tue Sep 19 11:34:28 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Julien Grall X-Patchwork-Id: 9958705 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 854D1601E9 for ; Tue, 19 Sep 2017 11:36:56 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 704E328438 for ; Tue, 19 Sep 2017 11:36:56 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6162428E0F; Tue, 19 Sep 2017 11:36:56 +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=-4.2 required=2.0 tests=BAYES_00, 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 AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id A0E7328E0C for ; Tue, 19 Sep 2017 11:36:55 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1duGnW-0006mD-QI; Tue, 19 Sep 2017 11:34:38 +0000 Received: from mail6.bemta6.messagelabs.com ([193.109.254.103]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1duGnV-0006m0-4T for xen-devel@lists.xen.org; Tue, 19 Sep 2017 11:34:37 +0000 Received: from [193.109.254.147] by server-3.bemta-6.messagelabs.com id BE/91-03093-CC001C95; Tue, 19 Sep 2017 11:34:36 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrOIsWRWlGSWpSXmKPExsVysyfVTfcEw8F Ig0nrDC2WfFzM4sDocXT3b6YAxijWzLyk/IoE1ozOxUuZCz5KVdzcOJW9gbFXuIuRi0NIYBOj xJbPd5ggnN2MEncP3GXpYuTkEBZwkpg09T0biC0ioCjx4OVNVpAiZoGDQB2np7CCJIQE0iV6T x0Ds9kENCXufP7EBGLzClhJXO8/AtbMIqAqse/NQbAaUYFwif3frzFD1AhKnJz5BGwZp4ClxK WJq8HizALqEn/mXYKyxSVuPZnPBGHLS2x/O4d5AiP/LCTts5C0zELSMgtJywJGllWM6sWpRWW pRbpmeklFmekZJbmJmTm6hgZmermpxcWJ6ak5iUnFesn5uZsYgQHKAAQ7GOed8D/EKMnBpCTK 2/frQKQQX1J+SmVGYnFGfFFpTmrxIUYZDg4lCd6m/0A5waLU9NSKtMwcYKzApCU4eJREeE+Ap HmLCxJzizPTIVKnGBWlxHmngSQEQBIZpXlwbbD4vMQoKyXMywh0iBBPQWpRbmYJqvwrRnEORi Vh3niQKTyZeSVw018BLWYCWpy9AWxxSSJCSqqBseDbiTmrk4I/P1Gaekd2Yej+Trk4t8eCJh3 eu9WYPNvzu55Ln7j+TXzG58nLV029d3+90OS9WzTDbrJ9ORNm/azIMf9Sn1Sxzsz+pzxCZ1nO 56zSfnSNRSZo90HpvRzNmwVX9Xd2zzig1n27f2/Umw2b1H5yL8y69O3tm+tnBWRi7UzaHzZve KHEUpyRaKjFXFScCAD6IB7fygIAAA== X-Env-Sender: julien.grall@arm.com X-Msg-Ref: server-5.tower-27.messagelabs.com!1505820872!111993869!1 X-Originating-IP: [217.140.101.70] X-SpamReason: No, hits=0.0 required=7.0 tests= X-StarScan-Received: X-StarScan-Version: 9.4.45; banners=-,-,- X-VirusChecked: Checked Received: (qmail 58123 invoked from network); 19 Sep 2017 11:34:32 -0000 Received: from foss.arm.com (HELO foss.arm.com) (217.140.101.70) by server-5.tower-27.messagelabs.com with SMTP; 19 Sep 2017 11:34:32 -0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id C9FBD80D; Tue, 19 Sep 2017 04:34:31 -0700 (PDT) Received: from [10.1.206.53] (e108454-lin.cambridge.arm.com [10.1.206.53]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 465863F483; Tue, 19 Sep 2017 04:34:30 -0700 (PDT) To: Wei Liu References: <20170919112228.22566-1-julien.grall@arm.com> <20170919112632.khprmdzwa6a36h6z@citrix.com> From: Julien Grall Message-ID: <0d8d2d1d-f097-d7b9-f7d2-850109eb709f@arm.com> Date: Tue, 19 Sep 2017 12:34:28 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <20170919112632.khprmdzwa6a36h6z@citrix.com> Content-Language: en-US Cc: tim@xen.org, sstabellini@kernel.org, George.Dunlap@eu.citrix.com, andrew.cooper3@citrix.com, ian.jackson@eu.citrix.com, xen-devel@lists.xen.org, jbeulich@suse.com Subject: Re: [Xen-devel] [PATCH v3] xen: grant-table: Simplify get_paged_frame X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" X-Virus-Scanned: ClamAV using ClamSMTP On 19/09/17 12:26, Wei Liu wrote: > On Tue, Sep 19, 2017 at 12:22:28PM +0100, Julien Grall wrote: >> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c >> index c3895e6201..b7deb57b85 100644 >> --- a/xen/common/grant_table.c >> +++ b/xen/common/grant_table.c >> @@ -259,34 +259,34 @@ static int get_paged_frame(unsigned long gfn, unsigned long *frame, >> struct domain *rd) >> { >> int rc = GNTST_okay; >> -#if defined(P2M_PAGED_TYPES) || defined(P2M_SHARED_TYPES) >> p2m_type_t p2mt; >> >> *page = get_page_from_gfn(rd, gfn, &p2mt, >> - (readonly) ? P2M_ALLOC : P2M_UNSHARE); >> - if ( !(*page) ) >> + readonly ? P2M_ALLOC : P2M_UNSHARE); >> + if ( !*page ) >> { >> *frame = mfn_x(INVALID_MFN); >> +#ifdef P2M_SHARED_TYPES >> if ( p2m_is_shared(p2mt) ) >> return GNTST_eagain; >> +#endif >> +#ifdef P2M_PAGES_TYPES >> if ( p2m_is_paging(p2mt) ) >> { >> p2m_mem_paging_populate(rd, gfn); >> return GNTST_eagain; >> } >> +#endif >> return GNTST_bad_page; >> } >> - *frame = page_to_mfn(*page); >> -#else >> - *frame = mfn_x(gfn_to_mfn(rd, _gfn(gfn))); >> - *page = mfn_valid(_mfn(*frame)) ? mfn_to_page(*frame) : NULL; >> - if ( (!(*page)) || (!get_page(*page, rd)) ) >> + >> + if ( p2m_is_foreign(p2mt) ) >> { >> - *frame = mfn_x(INVALID_MFN); >> - *page = NULL; >> - rc = GNTST_bad_page; >> + put_page(*page); > > Please set page to NULL and frame to INVALID_MFN to match the comment of > the function. > > I suppose you can set *frame = INVALID_MFN at the beginning of the > function to avoid code duplication in two error paths. > > With that: > > Reviewed-by: Wei Liu Would the below patch be fine? diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c index c3895e6201..dc1bacacb0 100644 --- a/xen/common/grant_table.c +++ b/xen/common/grant_table.c @@ -259,34 +259,36 @@ static int get_paged_frame(unsigned long gfn, unsigned long *frame, struct domain *rd) { int rc = GNTST_okay; -#if defined(P2M_PAGED_TYPES) || defined(P2M_SHARED_TYPES) p2m_type_t p2mt; + *frame = mfn_x(INVALID_MFN); *page = get_page_from_gfn(rd, gfn, &p2mt, - (readonly) ? P2M_ALLOC : P2M_UNSHARE); - if ( !(*page) ) + readonly ? P2M_ALLOC : P2M_UNSHARE); + if ( !*page ) { - *frame = mfn_x(INVALID_MFN); +#ifdef P2M_SHARED_TYPES if ( p2m_is_shared(p2mt) ) return GNTST_eagain; +#endif +#ifdef P2M_PAGES_TYPES if ( p2m_is_paging(p2mt) ) { p2m_mem_paging_populate(rd, gfn); return GNTST_eagain; } +#endif return GNTST_bad_page; } - *frame = page_to_mfn(*page); -#else - *frame = mfn_x(gfn_to_mfn(rd, _gfn(gfn))); - *page = mfn_valid(_mfn(*frame)) ? mfn_to_page(*frame) : NULL; - if ( (!(*page)) || (!get_page(*page, rd)) ) + + if ( p2m_is_foreign(p2mt) ) { - *frame = mfn_x(INVALID_MFN); + put_page(*page); *page = NULL; - rc = GNTST_bad_page; + + return GNTST_bad_page; } -#endif + + *frame = page_to_mfn(*page); return rc; }