From: Pavel Machek <pavel@ucw.cz>
To: "Chen, Yu C" <yu.c.chen@intel.com>
Cc: "rjw@rjwysocki.net" <rjw@rjwysocki.net>,
"Brown, Len" <len.brown@intel.com>,
"linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"Zhang, Rui" <rui.zhang@intel.com>,
"jlee@suse.com" <jlee@suse.com>,
"joeyli.kernel@gmail.com" <joeyli.kernel@gmail.com>,
"yinghai@kernel.org" <yinghai@kernel.org>,
Ingo Molnar <mingo@kernel.org>
Subject: Re: [PATCH] [v4] PM / hibernate: Fix hibernation panic caused by inconsistent e820 map
Date: Fri, 18 Sep 2015 07:47:20 +0200 [thread overview]
Message-ID: <20150918054719.GA18701@amd> (raw)
In-Reply-To: <36DF59CE26D8EE47B0655C516E9CE64026584EB5@SHSMSX101.ccr.corp.intel.com>
Hi!
> > But I still don't like the patch.
> >
> > 0) BIOS is broken, and this does not completely work around it. Users will still
> > see the failed hibernation when the memory that is now unavailable was
> > actually used.
> >
> Unfortunately , yes :( the patch is trying to replace the 'panic' with 'failure', because I guess 'failure'
> is a little better than a 'panic' ? It is a workaround for
> inconsistent BIOS.
I don't see the failure being better than panic here... and it is
certainly not worth the extra code.
And you can improve the panic message (or turn it into some other
printk) without any extra changes.
> Actually, according to ACPI spec 5, page 626, BIOS should not change its memory table
> during S4:
> " The memory information returned from the system address map
> reporting interfaces should be the same before and after an S4 sleep"
Good.
> > 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.
Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
next prev parent reply other threads:[~2015-09-18 5:47 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-16 17:42 [PATCH] [v4] PM / hibernate: Fix hibernation panic caused by inconsistent e820 map Chen Yu
2015-09-17 6:07 ` Pavel Machek
2015-09-17 18:11 ` Chen, Yu C
2015-09-17 20:43 ` Pavel Machek
2015-09-18 3:47 ` Chen, Yu C
2015-09-18 5:47 ` Pavel Machek [this message]
2015-09-18 6:11 ` Chen, Yu C
2015-10-04 15:15 ` Pavel Machek
2015-10-09 9:32 ` Chen, Yu C
2015-09-17 18:21 ` Chen, Yu C
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150918054719.GA18701@amd \
--to=pavel@ucw.cz \
--cc=jlee@suse.com \
--cc=joeyli.kernel@gmail.com \
--cc=len.brown@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=rjw@rjwysocki.net \
--cc=rui.zhang@intel.com \
--cc=yinghai@kernel.org \
--cc=yu.c.chen@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).