From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1751361AbbHJLcC (ORCPT <rfc822;w@1wt.eu>);
	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 <rfc822;linux-kernel@vger.kernel.org>);
	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 <julien.grall@citrix.com>
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 <stefano.stabellini@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>, <ian.campbell@citrix.com>,
        "Konrad Rzeszutek Wilk" <konrad.wilk@oracle.com>,
        <linux-kernel@vger.kernel.org>, David Vrabel <david.vrabel@citrix.com>,
        <xen-devel@lists.xenproject.org>,
        Boris Ostrovsky <boris.ostrovsky@oracle.com>,
        <linux-arm-kernel@lists.infradead.org>
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>
 <alpine.DEB.2.02.1508101207071.11337@kaball.uk.xensource.com>
In-Reply-To: <alpine.DEB.2.02.1508101207071.11337@kaball.uk.xensource.com>
Content-Type: text/plain; charset="windows-1252"
Content-Transfer-Encoding: 7bit
X-DLP: MIA2
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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