Re: + mm-remove-find_max_pfn_with_active_regions.patch added to -mm tree

["Yinghai Lu" <yhlu.kernel@gmail.com>]

On Mon, Jul 28, 2008 at 4:38 AM, Mel Gorman <mel@csn.ul.ie> wrote:
> On (28/07/08 04:15), Yinghai Lu didst pronounce:
>> On Mon, Jul 28, 2008 at 2:16 AM, Mel Gorman <mel@csn.ul.ie> wrote:
>> > On (27/07/08 20:13), akpm@linux-foundation.org didst pronounce:
>> >>
>> >> The patch titled
>> >>      mm: remove find_max_pfn_with_active_regions
>> >> has been added to the -mm tree.  Its filename is
>> >>      mm-remove-find_max_pfn_with_active_regions.patch
>> >>
>> >> Before you just go and hit "reply", please:
>> >>    a) Consider who else should be cc'ed
>> >>    b) Prefer to cc a suitable mailing list as well
>> >>    c) Ideally: find the original patch on the mailing list and do a
>> >>       reply-to-all to that, adding suitable additional cc's
>> >>
>> >> *** Remember to use Documentation/SubmitChecklist when testing your code ***
>> >>
>> >> See http://www.zip.com.au/~akpm/linux/patches/stuff/added-to-mm.txt to find
>> >> out what to do about this
>> >>
>> >> The current -mm tree may be found at http://userweb.kernel.org/~akpm/mmotm/
>> >>
>> >> ------------------------------------------------------
>> >> Subject: mm: remove find_max_pfn_with_active_regions
>> >> From: Yinghai Lu <yhlu.kernel@gmail.com>
>> >>
>> >> It has no user now
>> >>
>> >> Also print out info about adding/removing active regions.
>> >>
>> >> Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>
>> >> Cc: Mel Gorman <mel@csn.ul.ie>
>> >> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>> >> ---
>> >>
>> >>  include/linux/mm.h |    1 -
>> >>  mm/page_alloc.c    |   22 ++--------------------
>> >>  2 files changed, 2 insertions(+), 21 deletions(-)
>> >>
>> >> diff -puN include/linux/mm.h~mm-remove-find_max_pfn_with_active_regions include/linux/mm.h
>> >> --- a/include/linux/mm.h~mm-remove-find_max_pfn_with_active_regions
>> >> +++ a/include/linux/mm.h
>> >> @@ -1041,7 +1041,6 @@ extern unsigned long absent_pages_in_ran
>> >>  extern void get_pfn_range_for_nid(unsigned int nid,
>> >>                       unsigned long *start_pfn, unsigned long *end_pfn);
>> >>  extern unsigned long find_min_pfn_with_active_regions(void);
>> >> -extern unsigned long find_max_pfn_with_active_regions(void);
>> >>  extern void free_bootmem_with_active_regions(int nid,
>> >>                                               unsigned long max_low_pfn);
>> >>  typedef int (*work_fn_t)(unsigned long, unsigned long, void *);
>> >> diff -puN mm/page_alloc.c~mm-remove-find_max_pfn_with_active_regions mm/page_alloc.c
>> >> --- a/mm/page_alloc.c~mm-remove-find_max_pfn_with_active_regions
>> >> +++ a/mm/page_alloc.c
>> >> @@ -3572,8 +3572,7 @@ void __init add_active_range(unsigned in
>> >>  {
>> >>       int i;
>> >>
>> >> -     mminit_dprintk(MMINIT_TRACE, "memory_register",
>> >> -                     "Entering add_active_range(%d, %#lx, %#lx) "
>> >> +     printk(KERN_INFO "Adding active range (%d, %#lx, %#lx) "
>> >>                       "%d entries of %d used\n",
>> >>                       nid, start_pfn, end_pfn,
>> >>                       nr_nodemap_entries, MAX_ACTIVE_REGIONS);
>> >
>> > Why are the mminit_dprintk() calls being converted to printk(KERN_INFO)?  On
>> > some machines, this will be very noisy. For example, some POWER configurations
>> > will print out one line for every 16MB of memory with this patch.
>>
>> I don't know, on x86 esp the first node, that is some informative.
>> or change that back to printk(KERN_DEBUG) ?
>>
>> hope the user put debug on command_line to get enough info.
>>
>> otherwise without "mminit_loglevel=" will get that debug info.
>>
>
> It's the type of information that is only useful when debugging memory
> initialisation problems. The more friendly information can be found at
> the lines starting with
>
> early_node_map[1] active PFN ranges
>
> and this is already logged. The fact that mminit_loglevel needs loglevel
> needs to be at KERN_DEBUG level is already documented for the mminit_loglevel=
> parameter. I still am not convinced that these needs to be logged at
> KERN_INFO level.

I hope: when ask user to append "debug" we can get enough debug info
without other extra ...

and then if want add-on spew info, could ask them to append
initcall_debug, mminit_debug.

current "mminit_loglevel is default to 0 " doesn't look reasonable, it
hide some info when we could get if append "debug" only that we got
for years.

YH

--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>