Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751361AbbHJLcC (ORCPT ); Mon, 10 Aug 2015 07:32:02 -0400 Received: from smtp.citrix.com ([66.165.176.89]:47306 "EHLO SMTP.CITRIX.COM" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750913AbbHJLcA (ORCPT ); Mon, 10 Aug 2015 07:32:00 -0400 X-IronPort-AV: E=Sophos;i="5.15,645,1432598400"; d="scan'208";a="289609410" Message-ID: <55C88B77.8030700@citrix.com> Date: Mon, 10 Aug 2015 12:31:03 +0100 From: Julien Grall User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.8.0 MIME-Version: 1.0 To: Stefano Stabellini CC: Wei Liu , , "Konrad Rzeszutek Wilk" , , David Vrabel , , Boris Ostrovsky , Subject: Re: [PATCH v3 12/20] xen/balloon: Don't rely on the page granularity is the same for Xen and Linux References: <1438966019-19322-1-git-send-email-julien.grall@citrix.com> <1438966019-19322-13-git-send-email-julien.grall@citrix.com> In-Reply-To: Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-DLP: MIA2 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1647 Lines: 48 Hi Stefano, On 10/08/15 12:18, Stefano Stabellini wrote: >> /* Link back into the page tables if not highmem. */ >> @@ -396,14 +413,15 @@ static enum bp_state increase_reservation(unsigned long nr_pages) >> static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp) >> { >> enum bp_state state = BP_DONE; >> - unsigned long pfn, i; >> + unsigned long i; >> struct page *page; >> int ret; >> struct xen_memory_reservation reservation = { >> .address_bits = 0, >> - .extent_order = 0, >> + .extent_order = EXTENT_ORDER, >> .domid = DOMID_SELF >> }; >> + static struct page *pages[ARRAY_SIZE(frame_list)]; > > This array can be rather large: I would try to avoid it, see below. [..] > > I would simply and avoid introducing a new array: > pfn = (frame_list[i] << XEN_PAGE_SHIFT) >> PAGE_SHIFT; > page = pfn_to_page(pfn); Which won't work because the frame_list contains a gfn and not a pfn. We need to translate back the gfn into a pfn and the into a page. The cost of the translation may be big and I wanted to avoid anymore XEN_PAGE_SHIFT in the code. In general we should avoid to deal with 4KB PFN when it's not necessary, it make the code more confusing to read. If your only concern is the size of the array, we could decrease the number of frames by batch. Or allocation the variable once a boot time. Regards, -- Julien Grall -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/