From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from psmtp.com (na3sys010amx177.postini.com [74.125.245.177]) by kanga.kvack.org (Postfix) with SMTP id 3FEEA6B0093 for ; Fri, 19 Apr 2013 04:10:26 -0400 (EDT) Received: from m3.gw.fujitsu.co.jp (unknown [10.0.50.73]) by fgwmail5.fujitsu.co.jp (Postfix) with ESMTP id A9BE53EE0C3 for ; Fri, 19 Apr 2013 17:10:24 +0900 (JST) Received: from smail (m3 [127.0.0.1]) by outgoing.m3.gw.fujitsu.co.jp (Postfix) with ESMTP id 9011245DEBB for ; Fri, 19 Apr 2013 17:10:24 +0900 (JST) Received: from s3.gw.fujitsu.co.jp (s3.gw.fujitsu.co.jp [10.0.50.93]) by m3.gw.fujitsu.co.jp (Postfix) with ESMTP id 6DC3345DEBA for ; Fri, 19 Apr 2013 17:10:24 +0900 (JST) Received: from s3.gw.fujitsu.co.jp (localhost.localdomain [127.0.0.1]) by s3.gw.fujitsu.co.jp (Postfix) with ESMTP id 4B9021DB803C for ; Fri, 19 Apr 2013 17:10:24 +0900 (JST) Received: from G01JPEXCHKW14.g01.fujitsu.local (G01JPEXCHKW14.g01.fujitsu.local [10.0.194.53]) by s3.gw.fujitsu.co.jp (Postfix) with ESMTP id 089C21DB803B for ; Fri, 19 Apr 2013 17:10:24 +0900 (JST) Message-ID: <5170FBD7.8060605@jp.fujitsu.com> Date: Fri, 19 Apr 2013 17:09:59 +0900 From: Yasuaki Ishimatsu MIME-Version: 1.0 Subject: Re: [Bug fix PATCH v4] Reusing a resource structure allocated by bootmem References: <516FB07C.9010603@jp.fujitsu.com> <1366295000.3824.47.camel@misato.fc.hp.com> <517082B9.7050708@jp.fujitsu.com> In-Reply-To: <517082B9.7050708@jp.fujitsu.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org List-ID: To: Toshi Kani Cc: akpm@linux-foundation.org, linuxram@us.ibm.com, rientjes@google.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org Hi Toshi, 2013/04/19 8:33, Yasuaki Ishimatsu wrote: > Hi Toshi, > > 2013/04/18 23:23, Toshi Kani wrote: >> On Thu, 2013-04-18 at 17:36 +0900, Yasuaki Ishimatsu wrote: >>> When hot removing memory presented at boot time, following messages are shown: >> : >>> diff --git a/kernel/resource.c b/kernel/resource.c >>> index 4aef886..637e8d2 100644 >>> --- a/kernel/resource.c >>> +++ b/kernel/resource.c >>> @@ -21,6 +21,7 @@ >>> #include >>> #include >>> #include >>> +#include >>> #include >>> >>> >>> @@ -50,6 +51,16 @@ struct resource_constraint { >>> >>> static DEFINE_RWLOCK(resource_lock); >>> >>> +/* >>> + * For memory hotplug, there is no way to free resource entries allocated >>> + * by boot mem after the system is up. So for reusing the resource entry >>> + * we need to remember the resource. >>> + */ >>> +struct resource bootmem_resource = { >>> + .sibling = NULL, >>> +}; >> > >> This should be a pointer of struct resource and declared as static, such >> as: >> >> static struct resource *bootmem_resource_free; > > O.K. I'll update it. Oh, I missed "should be pointer of struct resource" part. Please teach me your detailed idea. If this is defined as pointer, how do we initialize this and manage bootmem resources? I'm thinking it but have no idea. Thanks, Yasuaki Ishimatsu > >> >>> +static DEFINE_SPINLOCK(bootmem_resource_lock); >>> + >>> static void *r_next(struct seq_file *m, void *v, loff_t *pos) >>> { >>> struct resource *p = v; >>> @@ -151,6 +162,39 @@ __initcall(ioresources_init); >>> >>> #endif /* CONFIG_PROC_FS */ >>> >>> +static void free_resource(struct resource *res) >>> +{ >>> + if (!res) >>> + return; >>> + >>> + if (PageSlab(virt_to_head_page(res))) { >>> + spin_lock(&bootmem_resource_lock); >>> + res->sibling = bootmem_resource.sibling; >>> + bootmem_resource.sibling = res; >>> + spin_unlock(&bootmem_resource_lock); >>> + } else { >>> + kfree(res); >>> + } >>> +} >> > >> I second with Johannes. > > I'll update it. > >> >>> +static struct resource *get_resource(gfp_t flags) >>> +{ >>> + struct resource *res = NULL; >>> + >>> + spin_lock(&bootmem_resource_lock); >>> + if (bootmem_resource.sibling) { >>> + res = bootmem_resource.sibling; >>> + bootmem_resource.sibling = res->sibling; >>> + memset(res, 0, sizeof(struct resource)); >>> + } >>> + spin_unlock(&bootmem_resource_lock); >> > >> I prefer to keep memset() outside of the spin lock. >> >> spin_lock(&bootmem_resource_lock); >> if (..) { >> : >> spin_unlock(&bootmem_resource_lock); >> memset(res, 0, sizeof(struct resource)); >> } else { >> spin_unlock(&bootmem_resource_lock); >> res = kzalloc(sizeof(struct resource), flags); >> } > > Hmm. It is a little ugly. How about it? > > spin_lock(&bootmem_resource_lock); > if (bootmem_resource.sibling) { > res = bootmem_resource.sibling; > bootmem_resource.sibling = res->sibling; > } > spin_unlock(&bootmem_resource_lock); > > if (res) > memset(res, 0, sizeof(struct resource)); > else > res = kzalloc(sizeof(struct resource), flags); > > Thanks, > Yasuaki Ishimatsu > >> >> Thanks, >> -Toshi >> >>> + >>> + >>> + return res; >>> +} >>> + >> >> > > > -- > 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/ -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org