Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751614AbbFYSRK (ORCPT ); Thu, 25 Jun 2015 14:17:10 -0400 Received: from userp1040.oracle.com ([156.151.31.81]:49983 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750777AbbFYSRC (ORCPT ); Thu, 25 Jun 2015 14:17:02 -0400 Date: Thu, 25 Jun 2015 20:16:51 +0200 From: Daniel Kiper To: David Vrabel Cc: xen-devel@lists.xenproject.org, Konrad Rzeszutek Wilk , Boris Ostrovsky , linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCHv1 4/8] xen/balloon: find non-conflicting regions to place hotplugged memory Message-ID: <20150625181651.GL14050@olila.local.net-space.pl> References: <1435252263-31952-1-git-send-email-david.vrabel@citrix.com> <1435252263-31952-5-git-send-email-david.vrabel@citrix.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1435252263-31952-5-git-send-email-david.vrabel@citrix.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-Source-IP: userv0022.oracle.com [156.151.31.74] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4161 Lines: 144 On Thu, Jun 25, 2015 at 06:10:59PM +0100, David Vrabel wrote: > Instead of placing hotplugged memory at the end of RAM (which may > conflict with PCI devices or reserved regions) use allocate_resource() > to get a new, suitably aligned resource that does not conflict. > > Signed-off-by: David Vrabel In general Reviewed-by: Daniel Kiper but two nitpicks below... > --- > drivers/xen/balloon.c | 63 +++++++++++++++++++++++++++++++++++++++++-------- > 1 file changed, 53 insertions(+), 10 deletions(-) > > diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c > index fd93369..d0121ee 100644 > --- a/drivers/xen/balloon.c > +++ b/drivers/xen/balloon.c > @@ -54,6 +54,7 @@ > #include > #include > #include > +#include > > #include > #include > @@ -208,6 +209,42 @@ static bool balloon_is_inflated(void) > return false; > } > > +static struct resource *additional_memory_resource(phys_addr_t size) > +{ > + struct resource *res; > + int ret; > + > + res = kzalloc(sizeof(*res), GFP_KERNEL); > + if (!res) > + return NULL; > + > + res->name = "System RAM"; > + res->flags = IORESOURCE_MEM | IORESOURCE_BUSY; > + > + ret = allocate_resource(&iomem_resource, res, > + size, 0, -1, > + PAGES_PER_SECTION * PAGE_SIZE, NULL, NULL); > + if (ret < 0) { > + pr_err("Cannot allocate new System RAM resource\n"); > + kfree(res); > + return NULL; > + } > + > + return res; > +} > + > +static void release_memory_resource(struct resource *resource) > +{ > + if (!resource) > + return; Please add one empty line here. > + /* > + * No need to reset region to identity mapped since we now > + * know that no I/O can be in this region > + */ > + release_resource(resource); > + kfree(resource); > +} > + > /* > * reserve_additional_memory() adds memory region of size >= credit above > * max_pfn. New region is section aligned and size is modified to be multiple > @@ -221,13 +258,17 @@ static bool balloon_is_inflated(void) > > static enum bp_state reserve_additional_memory(long credit) > { > + struct resource *resource; > int nid, rc; > - u64 hotplug_start_paddr; > - unsigned long balloon_hotplug = credit; > + unsigned long balloon_hotplug; > + > + balloon_hotplug = round_up(credit, PAGES_PER_SECTION); > + > + resource = additional_memory_resource(balloon_hotplug * PAGE_SIZE); > + if (!resource) > + goto err; > > - hotplug_start_paddr = PFN_PHYS(SECTION_ALIGN_UP(max_pfn)); > - balloon_hotplug = round_up(balloon_hotplug, PAGES_PER_SECTION); > - nid = memory_add_physaddr_to_nid(hotplug_start_paddr); > + nid = memory_add_physaddr_to_nid(resource->start); > > #ifdef CONFIG_XEN_HAVE_PVMMU > /* > @@ -242,21 +283,20 @@ static enum bp_state reserve_additional_memory(long credit) > if (!xen_feature(XENFEAT_auto_translated_physmap)) { > unsigned long pfn, i; > > - pfn = PFN_DOWN(hotplug_start_paddr); > + pfn = PFN_DOWN(resource->start); > for (i = 0; i < balloon_hotplug; i++) { > if (!set_phys_to_machine(pfn + i, INVALID_P2M_ENTRY)) { > pr_warn("set_phys_to_machine() failed, no memory added\n"); > - return BP_ECANCELED; > + goto err; > } > } > } > #endif > > - rc = add_memory(nid, hotplug_start_paddr, balloon_hotplug << PAGE_SHIFT); > - > + rc = add_memory_resource(nid, resource); > if (rc) { > pr_warn("Cannot add additional memory (%i)\n", rc); > - return BP_ECANCELED; > + goto err; > } > > balloon_hotplug -= credit; > @@ -265,6 +305,9 @@ static enum bp_state reserve_additional_memory(long credit) > balloon_stats.balloon_hotplug = balloon_hotplug; > > return BP_DONE; Ditto. > + err: > + release_memory_resource(resource); > + return BP_ECANCELED; > } > > static void xen_online_page(struct page *page) > -- > 1.7.10.4 Daniel -- 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/