[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Pine.LNX.4.64.0704231402220.29560@skynet.skynet.ie>
Date: Mon, 23 Apr 2007 14:06:13 +0100 (IST)
From: Mel Gorman <mel@....ul.ie>
To: Yasunori Goto <y-goto@...fujitsu.com>
cc: Andrew Morton <akpm@...ux-foundation.org>,
Linux Kernel ML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH]Fix parsing kernelcore boot option for ia64
On Mon, 23 Apr 2007, Yasunori Goto wrote:
>> On Fri, 13 Apr 2007 14:26:22 +0900 Yasunori Goto <y-goto@...fujitsu.com> wrote:
>>
>>> Hello.
>>>
>>> cmdline_parse_kernelcore() should return the next pointer of boot option
>>> like memparse() doing. If not, it is cause of eternal loop on ia64 box.
>>> This patch is for 2.6.21-rc6-mm1.
>>>
>>> Signed-off-by: Yasunori Goto <y-goto@...fujitsu.com>
>>>
>>> ----
>>>
>>> arch/ia64/kernel/efi.c | 2 +-
>>> include/linux/mm.h | 2 +-
>>> mm/page_alloc.c | 4 ++--
>>> 3 files changed, 4 insertions(+), 4 deletions(-)
>>>
>>> Index: current_test/arch/ia64/kernel/efi.c
>>> ===================================================================
>>> --- current_test.orig/arch/ia64/kernel/efi.c 2007-04-12 17:33:28.000000000 +0900
>>> +++ current_test/arch/ia64/kernel/efi.c 2007-04-13 12:13:21.000000000 +0900
>>> @@ -424,7 +424,7 @@ efi_init (void)
>>> } else if (memcmp(cp, "max_addr=", 9) == 0) {
>>> max_addr = GRANULEROUNDDOWN(memparse(cp + 9, &cp));
>>> } else if (memcmp(cp, "kernelcore=",11) == 0) {
>>> - cmdline_parse_kernelcore(cp+11);
>>> + cmdline_parse_kernelcore(cp+11, &cp);
>>> } else if (memcmp(cp, "min_addr=", 9) == 0) {
>>> min_addr = GRANULEROUNDDOWN(memparse(cp + 9, &cp));
>>> } else {
>>> Index: current_test/mm/page_alloc.c
>>> ===================================================================
>>> --- current_test.orig/mm/page_alloc.c 2007-04-12 18:25:37.000000000 +0900
>>> +++ current_test/mm/page_alloc.c 2007-04-13 12:12:58.000000000 +0900
>>> @@ -3736,13 +3736,13 @@ void __init free_area_init_nodes(unsigne
>>> * kernelcore=size sets the amount of memory for use for allocations that
>>> * cannot be reclaimed or migrated.
>>> */
>>> -int __init cmdline_parse_kernelcore(char *p)
>>> +int __init cmdline_parse_kernelcore(char *p, char **retp)
>>> {
>>> unsigned long long coremem;
>>> if (!p)
>>> return -EINVAL;
>>>
>>> - coremem = memparse(p, &p);
>>> + coremem = memparse(p, retp);
>>> required_kernelcore = coremem >> PAGE_SHIFT;
>>>
>>> /* Paranoid check that UL is enough for required_kernelcore */
>>> Index: current_test/include/linux/mm.h
>>> ===================================================================
>>> --- current_test.orig/include/linux/mm.h 2007-04-11 14:15:33.000000000 +0900
>>> +++ current_test/include/linux/mm.h 2007-04-13 12:12:20.000000000 +0900
>>> @@ -1051,7 +1051,7 @@ extern unsigned long find_max_pfn_with_a
>>> extern void free_bootmem_with_active_regions(int nid,
>>> unsigned long max_low_pfn);
>>> extern void sparse_memory_present_with_active_regions(int nid);
>>> -extern int cmdline_parse_kernelcore(char *p);
>>> +extern int cmdline_parse_kernelcore(char *p, char **retp);
>>> #ifndef CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID
>>> extern int early_pfn_to_nid(unsigned long pfn);
>>> #endif /* CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID */
>>>
>>
>> This will cause all other architectures to crash when kernelcore= is used.
>>
>> I wasn't even aware of this kernelcore thing. It's pretty nasty-looking.
>> yet another reminder that this code hasn't been properly reviewed in the
>> past year or three.
>
> Just now, I'm making memory-unplug patches with current MOVABLE_ZONE
> code. So, I might be the first user of it on ia64.
>
> Anyway, I'll try to fix it.
>
I'm looking at this as well. There may be more than one problem in there
so I'm being more thorough with testing on other arches.
Andrew, the option is part of the zone-based solution to fragmentation
avoidance which hasn't been mentioned in a while. That's probably why it
fell off your radar. It's nasty looking because sizing the portion of
memory usable by any allocation so that it is evenly spread throughout all
nodes is not trivial. I think that having mem= parsing broken on NUMA at
various times in the past (and probably still broken) highlights that.
--
Mel Gorman
Part-time Phd Student Linux Technology Center
University of Limerick IBM Dublin Software Lab
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists