Re: [PATCH] x86/mm, hibernate: Fix misjudgment of register setup_data page to nosave region

[joeyli <jlee@suse.com>]

Hi,

On Thu, Jan 29, 2015 at 11:35:49PM -0800, Yinghai Lu wrote:
> On Thu, Jan 29, 2015 at 7:58 PM, Lee, Chun-Yi <joeyli.kernel@gmail.com> wrote:
> >
> > diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
> > index 49f8864..6eae021 100644
> > --- a/arch/x86/kernel/e820.c
> > +++ b/arch/x86/kernel/e820.c
> > @@ -687,15 +687,16 @@ void __init parse_e820_ext(u64 phys_addr, u32 data_len)
> >  void __init e820_mark_nosave_regions(unsigned long limit_pfn)
> >  {
> >         int i;
> > -       unsigned long pfn = 0;
> > +       unsigned long pfn = 0, pfnaddr = 0;
> >
> >         for (i = 0; i < e820.nr_map; i++) {
> >                 struct e820entry *ei = &e820.map[i];
> >
> > -               if (pfn < PFN_UP(ei->addr))
> > +               if (pfnaddr < ei->addr)
> >                         register_nosave_region(pfn, PFN_UP(ei->addr));
> >
> > -               pfn = PFN_DOWN(ei->addr + ei->size);
> > +               pfnaddr = ei->addr + ei->size;
> > +               pfn = PFN_DOWN(pfnaddr);
> >                 if (ei->type != E820_RAM && ei->type != E820_RESERVED_KERN)
> >                         register_nosave_region(PFN_UP(ei->addr), pfn);
> >
> Those changes may not fix the problem.
> 
> those E820_RESERVED_KERN and E820_RAM should be continuous.

Yes, they are continuous but not aligned.

> 
> So you need to find the real end for those continuous ranges.
> 

Sorry for I didn't capture the point for need to find the real end
because those misjudgments are happened in the boundary between
E820_RESERVED_KERN and E820_RAM.

> Thanks
> 
> Yinghai

e.g.
reserve setup_data: [mem 0x0000000000100000-0x0000000093c4f017] usable
reserve setup_data: [mem 0x0000000093c4f018-0x0000000093c5e057] usable

pfn = PFN_DOWN(0x93c4f017+1) = 0x93c4f
PFN_UP(0x93c4f018) = (0x93c4f018 + 0x1000 - 1 >> PAGE_SHIFT) = 0x93C50

The above case match with "if (pfn < PFN_UP(ei->addr))" logic, it causes
e820_mark_nosave_regions() add one usable page to nosave region:

PM: Registered nosave memory: [mem 0x93c4f000-0x93c4ffff]

Comparing the pfn of regions is not enough so my patch compares the address
instead of pfn. It works to me to avoid those one page area show in nosave
region.


Thanks a lot!
Joey Lee