Re: [PATCH v2] Hibernate: check unsafe page should not in e820 reserved region

[joeyli <jlee@suse.com>]

On Thu, Aug 07, 2014 at 11:05:33PM +0200, Pavel Machek wrote:
> On Thu 2014-08-07 18:17:34, joeyli wrote:
> > On Thu, Aug 07, 2014 at 11:39:57AM +0200, Pavel Machek wrote:
> > > > > Actually, if you are doing such a check... it makes sense to check for
> > > > > _all_ the regions, nosave or not. If e820 map changed at all, it is
> > > > > not safe to resume.
> > > > > 									Pavel
> > > > 
> > > > Currently nosave region only called register by e820 code, so hibernate's nosave region included e820
> > > > reserved, ACPI data and ACPI NVS region.
> > > > 
> > > > I thought hashing the start/end pfn of above regions is enough.
> > > 
> > > If ammount of memory changed, for example, it is unsafe to
> > > resume. So if you are doing the check, anyway, please hash
> > > whole e820 table.
> > 
> > There already have num_physpages in header for check the total physical page number.
> 
> Good, but if ammount of memory stayed the same, but offsets
> changed (for example), resume is unsafe, too.

I agreed

> 
> When I wrote that num_physpages check, I should have checked
> whole e820  table, instead. (If anything at all changed there,
> "new" kernel is running with wrong e820 info).
> 
> You seem to be in great position to fix that mistake now...
> 
> 						Pavel

Hashing e820 is fine, but it can not provide detail information to user/developer when issue
happened. We only know the e820 table changed but not more information for bug tracking, not
too many shipping machine have serial console.

After checked the space of swsusp_info, I hope can store whole e820 table to snapshot header
for compare the range. The maximum space consumed 20 * E820MAX = 20 * 128 = 2560 bytes.
Currrent swsusp_info is used 430 bytes in one page, it's enough to us for keep whole e820
table.

And, I thought don't need compare the range of E820_RAM and E820_RESERVED_KERN type
because they are using by OS and stored in snapshot image.


Thanks a lot!
Joey Lee