RE: [PATCH] [v4] PM / hibernate: Fix hibernation panic caused by inconsistent e820 map

[Chen, Yu C]

Hi,Pavel,

> -----Original Message-----
> From: Pavel Machek [mailto:pavel@xxxxxx]
> Sent: Sunday, October 04, 2015 11:16 PM
> To: Chen, Yu C
> Cc: rjw@xxxxxxxxxxxxx; Brown, Len; linux-pm@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; Zhang, Rui; jlee@xxxxxxxx;
> joeyli.kernel@xxxxxxxxx; yinghai@xxxxxxxxxx; Ingo Molnar
> Subject: Re: [PATCH] [v4] PM / hibernate: Fix hibernation panic caused by
> inconsistent e820 map
> 
> Hi!
> > > > > 3) I'm not sure I understand the changelog correctly. What
> > > > > happens when BIOS reports less memory on hibernation? Will you
> > > > > magically remove memory from kernel at runtime? Will
> > > > > /proc/meminfo be invalid
> > > after resume?
> > > > > Will all the memory management tuning need fixing?
> > > > >
> > > > Oh, I did not notice it before. So deleting the logic of '
> > > > info->num_physpages != get_num_physpages()' is not suitable.
> > > > The subset relationship should not be considered in this patch.
> > >
> > > Ok. So... if you really want, you can add some messages like "hey,
> > > this is bios bug, maybe updating bios is a good idea".. but please
> > > lets keep the original logic.
> > >
> > OK. I see, I'll not change its original code.
> > So can I add a function here that checks if current BIOS  e820 map is
> > strictly the same as it was before S4?  If it is not the same, we will
> > print some warnnings , and if we panic later, we will print that , the panic
> reason might be due to broken BIOS.
> > I think I can archive this by putting  the e820_saved array  into
> > struct swsusp_info,  and pass it to second kernel:
> > struct swsusp_info will always occupy one page size, and has a lot of
> > extra space left, meanwhile the total size of e820 map will not exceed
> > the PAGE_SIZE currently, it's safe to put it in struct swsusp_info.
> >
> > And this does not need much changing of current code. What do you think?
> Thanks.
> 
> Can we simply add explanation printk() before the panic(), without any other
> changes?
OK, I can add a hook in panic, and print the possible panic reason.
(and to print(confirm the bug reason) the explanation, we might still need to do 
some comparison on e820 map, this small code can be added in arch/x86/power/hibernate_64/32.c,
this might be used for future possible e820 checking), is it suitable?

 Thanks !

Best Regards,
Yu
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/