Messages in this thread | | | Date | Tue, 23 Apr 2013 09:19:09 +0900 | From | Yasuaki Ishimatsu <> | Subject | Re: [Bug fix PATCH v4] Reusing a resource structure allocated by bootmem |
| |
2013/04/19 23:30, Toshi Kani wrote: > On Fri, 2013-04-19 at 17:09 +0900, Yasuaki Ishimatsu wrote: >> 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 <linux/seq_file.h> >>>>> #include <linux/device.h> >>>>> #include <linux/pfn.h> >>>>> +#include <linux/mm.h> >>>>> #include <asm/io.h> >>>>> >>>>> >>>>> @@ -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. >
> Sure. It's quite simple. Since only bootmem_resource.sibling is used, > it can be replaced with a pointer, such as bootmem_resource_free. This > avoids allocating a struct resource table as this code is supposed to > save memory. > > So, first, declare the pointer below, which is initialized to NULL as > BSS (checkpatch.pl complains if you set it to NULL explicitly). > > static struct resource *bootmem_resource_free; > > Then, in free_resource(), this pointer replaces bootmem_resource.sibling > as follows. > > res->sibling = bootmem_resource_free; > bootmem_resource_free = res; > > Similarly, in get_resource(). > > if (bootmem_resource_free) { > res = bootmem_resource_free; > bootmem_resource_free = res->sibling; > }
Thank you for your detail explanation. I clearly understood your idea. I'll update your comment and post a updated patch.
Thanks, Yasuaki Ishimatsu
> > > Thanks, > -Toshi >
| |