From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1762181AbbA3OmA (ORCPT <rfc822;w@1wt.eu>);
	Fri, 30 Jan 2015 09:42:00 -0500
Received: from smtp.nue.novell.com ([195.135.221.5]:54830 "EHLO
	smtp.nue.novell.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1756262AbbA3Ol7 (ORCPT
	<rfc822;groupwise-linux-kernel@vger.kernel.org:0:0>);
	Fri, 30 Jan 2015 09:41:59 -0500
Date: Fri, 30 Jan 2015 22:41:41 +0800
From: joeyli <jlee@suse.com>
To: Yinghai Lu <yinghai@kernel.org>
Cc: "Lee, Chun-Yi" <joeyli.kernel@gmail.com>,
        "Rafael J. Wysocki" <rjw@rjwysocki.net>,
        Ingo Molnar <mingo@kernel.org>, Pavel Machek <pavel@ucw.cz>,
        Thomas Gleixner <tglx@linutronix.de>,
        the arch/x86 maintainers <x86@kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Len Brown <len.brown@intel.com>, "H. Peter Anvin" <hpa@zytor.com>,
        Takashi Iwai <tiwai@suse.de>
Subject: Re: [PATCH] x86/mm, hibernate: Fix misjudgment of register
 setup_data page to nosave region
Message-ID: <20150130144141.GJ28432@linux-rxt1.site>
References: <1422590300-13307-1-git-send-email-jlee@suse.com>
 <CAE9FiQUCokG_B2ymmVxD0icGcZ5AEY+pS3LipL1vQcP1ZV6D2g@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAE9FiQUCokG_B2ymmVxD0icGcZ5AEY+pS3LipL1vQcP1ZV6D2g@mail.gmail.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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