Hello HATAYAMA-san, On Thu, 15 Nov 2012 01:55:04 +0000 "Hatayama, Daisuke" <d.hatayama at jp.fujitsu.com> wrote: > > -----Original Message----- > > From: Atsushi Kumagai [mailto:kumagai-atsushi at mxc.nes.nec.co.jp] > > Sent: Wednesday, November 14, 2012 4:31 PM > [...] > > 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 > [...] > > 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. > > > > I'll first hard code PAGE_BUDDY_MAPCOUNT_VALUE for v2.6.38 and later. This is > a workaround until it is exported from kernel's VMCOREINFO. At the same time > I'll check NUMBER(PAGE_BUDDY_MAPCOUNT_VALUE) first and then SIZE(pageflags) in > setup_page_is_buddy(). By this, we can avoid the issue that page_is_buddy_v1 > is wrongly used on recent kernels. > > The remaining problematic kernel versions are the ones where: > > - PG_buddy is not present, and > - enum pageflags is present but > - enum pageflags is not exported from VMCOREINFO. > > These are exactly from v2.6.27 to 2.6.37. I'll cover this range by hard coding > PG_buddy. One concern was the fact that value of PG_buddy in this range varies > depending on CONFIG_PAGEFLAGS_EXTENDED. But as below, this config is not relevant > to the architectures supported by makedumpfile. It's no problem. > > BTW, PAGE_BUDDY_MAPCOUNT_VALUE is now defined as macro. This is not useful > since we cannot get its value from vmlinux. I'll make a patch to redefine it > as enumeration later. It's originally signed integer, -128, so I think it's no > problem. > > $ git grep ".*CONFIG_PAGEFLAGS_EXTEND.*" ./arch/ > arch/um/defconfig:CONFIG_PAGEFLAGS_EXTENDED=y > arch/xtensa/configs/iss_defconfig:CONFIG_PAGEFLAGS_EXTENDED=y > arch/xtensa/configs/s6105_defconfig:CONFIG_PAGEFLAGS_EXTENDED=y > > Next diff shows the idea I explain above. I'll add the same explanation in codes as > comments. The assumption behind these seems complicated and maintainance would be > difficult without any comments explaining why these are being done this way. I think it's OK on the logic of selection for page_is_buddy_vX(). But there remain some cases where page_is_buddy_vX() doesn't work correctly because OFFSET(page.private) and OFFSET(page._mapcount) do not exist. And it's difficult to hard code for them. So I think setup_page_is_buddy() should be changed like below: static void setup_page_is_buddy(void) { if (OFFSET(page.private) == NOT_FOUND_STRUCTURE) info->page_is_buddy = NULL; else if (NUMBER(PG_buddy) == NOT_FOUND_NUMBER) { if (NUMBER(PAGE_BUDDY_MAPCOUNT_VALUE) != NOT_FOUND_NUMBER) { if (OFFSET(page._mapcout) != NOT_FOUND_STRUCTURE) { info->page_is_buddy = page_is_buddy_v3; return; } } else if (SIZE(pageflags) == NOT_FOUND_STRUCTURE) { info->page_is_buddy = page_is_buddy_v1; return; } } else { info->page_is_buddy = page_is_buddy_v2; return; } DEBUG_MSG("Can't select page_is_buddy handler; " "follow freelist instead of mem_map.\n"); } Additionally, I found a small mistake, please see below: > > $ git diff > diff --git a/makedumpfile.c b/makedumpfile.c > index 98eb640..a1a39d5 100644 > --- a/makedumpfile.c > +++ b/makedumpfile.c > @@ -1215,10 +1215,24 @@ get_value_for_old_linux(void) > NUMBER(PG_swapcache) = PG_swapcache_ORIGINAL; > if (NUMBER(PG_slab) == NOT_FOUND_NUMBER) > NUMBER(PG_slab) = PG_slab_ORIGINAL; > - if (NUMBER(PG_buddy) == NOT_FOUND_NUMBER > - && info->kernel_version >= KERNEL_VERSION(2, 6, 17) > - && info->kernel_version <= KERNEL_VERSION(2, 6, 26)) > - NUMBER(PG_buddy) = PG_buddy_v2_6_17_to_v2_6_26; > + if (NUMBER(PG_buddy) == NOT_FOUND_NUMBER) { > + if (info->kernel_version >= KERNEL_VERSION(2, 6, 17) > + && info->kernel_version <= KERNEL_VERSION(2, 6, 26)) > + NUMBER(PG_buddy) = PG_buddy_v2_6_17_to_v2_6_26; > + if (info->kernel_version >= KERNEL_VERSION(2, 6, 27) > + && info->kernel_version >= KERNEL_VERSION(2, 6, 37)) ^^^ should be '<=' Thanks Atsushi Kumagai > + NUMBER(PG_buddy) = 18; > + } > + if (NUMBER(PAGE_BUDDY_MAPCOUNT_VALUE) == NOT_FOUND_NUMBER) { > + if (info->kernel_version == KERNEL_VERSION(2, 6, 38)) > + NUMBER(PAGE_BUDDY_MAPCOUNT_VALUE) = -2; > + if (info->kernel_version >= KERNEL_VERSION(2, 6, 39)) > + NUMBER(PAGE_BUDDY_MAPCOUNT_VALUE) = -128; > + } > + if (SIZE(pageflags) == NOT_FOUND_STRUCTURE) { > + if (info->kernel_version >= KERNEL_VERSION(2, 6, 27)) > + SIZE(pageflags) = 4; > + } > return TRUE; > } > > @@ -3725,13 +3739,13 @@ static void > setup_page_is_buddy(void) > { > if (NUMBER(PG_buddy) == NOT_FOUND_NUMBER) { > - if (SIZE(pageflags) == NOT_FOUND_STRUCTURE) > - info->page_is_buddy = page_is_buddy_v1; > - else if (NUMBER(PAGE_BUDDY_MAPCOUNT_VALUE) != NOT_FOUND_NUMBER) > + if (NUMBER(PAGE_BUDDY_MAPCOUNT_VALUE) != NOT_FOUND_NUMBER) > info->page_is_buddy = page_is_buddy_v3; > + else if (SIZE(pageflags) == NOT_FOUND_STRUCTURE) > + info->page_is_buddy = page_is_buddy_v1; > 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; > > Thanks. > HATAYAMA, Daisuke
