Hello HATAYAMA-san, On Tue, 13 Nov 2012 10:03:37 +0000 "Hatayama, Daisuke" <d.hatayama at jp.fujitsu.com> wrote: > > -----Original Message----- > > From: kexec-bounces at lists.infradead.org > > [mailto:kexec-bounces at lists.infradead.org] On Behalf Of Hatayama, Daisuke > [..]> > > > -----Original Message----- > > > From: Atsushi Kumagai [mailto:kumagai-atsushi at mxc.nes.nec.co.jp] > > > Sent: Tuesday, November 13, 2012 4:56 PM > > [...] > > > > - OFFSET(page._mapcount) > > > > - OFFSET(page.private) > > > > - SIZE(pageflags) > > > > - NUMBER(PG_buddy) > > > > - NUMBER(PG_slab) > > > > - NUMBER(PAGE_BUDDY_MAPCOUNT_VALUE) > > > > > > And I will post the patch to add the values above to the upstream kernel, > > > so there is no problem on the future kernel. > > > > > > However, according to my tests, current makedumpfile can't get some of > > the > > > values above from VMLINUX and can't exclude free pages with the mem_map > > > array logic. > > > For example, page_is_buddy_v1() will be selected for kernel-3.4 with > > > setup_page_is_buddy() due to not founding NUMBER(PG_buddy) and > > > SIZE(pageflags), > > > it's miss selection. > > > > > > Therefore, I propose to change the policy for excluding free page as below > > > in order to take care of current kernels: > > > > > > 1. If VMCORE (or specified VMCOREINFO) includes the values above, > > > using the mem_map array logic. > > > > > > 2. In other cases, using the current logic which uses free_list even > > if > > > running on cyclic mode. > > > > > > Of course, page_is_buddy_v1() and page_is_buddy_v2() should be retain > > > because there is valuable for the user who can prepare VMCOREINFO. > > > > > > > You are corrent. I'll repost the patch set soon. Rc1 or beta, which is better? > > > > The patch I intend is like this, and this will be merged into the patch 06/10. It's basically good, but more fix is needed for setup_page_is_buddy(). 3706 static void 3707 setup_page_is_buddy(void) 3708 { 3709 if (NUMBER(PG_buddy) == NOT_FOUND_NUMBER) { 3710 if (SIZE(pageflags) == NOT_FOUND_STRUCTURE) 3711 info->page_is_buddy = page_is_buddy_v1; In case that none of the values needed for mem_map array logic is given, the condition on line 3710 will be met regardless of kernel version. As a result, invalid logic will be used and freelist logic isn't used because page_is_buddy isn't NULL. Additionally, the case above will be happen when using makedumpfile-1.5.0 for recent kernels, which doesn't export necessary symbols. So, I think that we need to change the statement on line 3710, but I have no idea without to check kernel version. If you have better way, I will adopt it. Thanks Atsushi Kumagai > $ git diff > diff --git a/makedumpfile.c b/makedumpfile.c > index 98eb640..f57612d 100644 > --- a/makedumpfile.c > +++ b/makedumpfile.c > @@ -3730,8 +3730,8 @@ setup_page_is_buddy(void) > else if (NUMBER(PAGE_BUDDY_MAPCOUNT_VALUE) != NOT_FOUND_NUMBER) > info->page_is_buddy = page_is_buddy_v3; > else { > - MSG("Can't select page_is_buddy handler; " > - "filtering free pages is disabled.\n"); > + DEBUG_MSG("Can't select page_is_buddy handler; " > + "follow freelist instead of mem_map.\n"); > } > } else > info->page_is_buddy = page_is_buddy_v2; > @@ -4118,7 +4118,7 @@ exclude_unnecessary_pages_cyclic(void) > if (info->dump_level & DL_EXCLUDE_CACHE || > info->dump_level & DL_EXCLUDE_CACHE_PRI || > info->dump_level & DL_EXCLUDE_USER_DATA || > - info->dump_level & DL_EXCLUDE_FREE) { > + ((info->dump_level & DL_EXCLUDE_FREE) && info->page_is_buddy)) { > > gettimeofday(&tv_start, NULL); > > @@ -4166,6 +4166,10 @@ update_cyclic_region(unsigned long long pfn) > if (!exclude_unnecessary_pages_cyclic()) > return FALSE; > > + if ((info->dump_level & DL_EXCLUDE_FREE) && !info->page_is_buddy) > + if (!exclude_free_pages()) > + return FALSE; > + > return TRUE; > } > > @@ -5171,6 +5175,9 @@ get_loads_dumpfile_cyclic(void) > return FALSE; > if (!exclude_unnecessary_pages_cyclic()) > return FALSE; > + if ((info->dump_level & DL_EXCLUDE_FREE) && !info->page_is_buddy) > + if (!exclude_free_pages()) > + return FALSE; > > if (!(phnum = get_phnum_memory())) > return FALSE; > diff --git a/makedumpfile.h b/makedumpfile.h > index 3ccc750..6ce8e23 100644 > --- a/makedumpfile.h > +++ b/makedumpfile.h > @@ -1034,6 +1034,15 @@ struct DumpInfo { > /* > * for filtering free pages managed by buddy system: > */ > + /* > + * Initialized by setup_page_is_buddy(). This can be NULL if > + * > + * 1) in non-cyclic mode, or > + * 2) in cyclic mode, proper page_is_buddy handler fails to > + * be detected. > + * > + * Then, conventional freelist logic is used instead. > + */ > int (*page_is_buddy)(unsigned long flags, unsigned int _mapcount, > unsigned long private, unsigned int _count); > }; > > Thanks. > HATAYAMA, Daisuke
