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

[Pavel Machek]

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
--
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/