Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752898AbbHJM5L (ORCPT ); Mon, 10 Aug 2015 08:57:11 -0400 Received: from smtp02.citrix.com ([66.165.176.63]:49727 "EHLO SMTP02.CITRIX.COM" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751117AbbHJM5G (ORCPT ); Mon, 10 Aug 2015 08:57:06 -0400 X-IronPort-AV: E=Sophos;i="5.15,645,1432598400"; d="scan'208";a="292958934" Date: Mon, 10 Aug 2015 13:55:37 +0100 From: Stefano Stabellini X-X-Sender: sstabellini@kaball.uk.xensource.com To: Julien Grall CC: Stefano Stabellini , 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 In-Reply-To: <55C88B77.8030700@citrix.com> Message-ID: References: <1438966019-19322-1-git-send-email-julien.grall@citrix.com> <1438966019-19322-13-git-send-email-julien.grall@citrix.com> <55C88B77.8030700@citrix.com> User-Agent: Alpine 2.02 (DEB 1266 2009-07-14) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" X-DLP: MIA1 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1852 Lines: 50 On Mon, 10 Aug 2015, Julien Grall wrote: > 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. That is true > 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. Yes, that is my only concern. Allocating only nr_pages new struct page* would be good enough I guess. -- 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/