Re: [PATCH 0/4] arch, mm: improve robustness of direct map manipulation

[David Hildenbrand]

On 28.10.20 12:09, Mike Rapoport wrote:
> On Tue, Oct 27, 2020 at 09:46:35AM +0100, David Hildenbrand wrote:
> > On 27.10.20 09:38, Mike Rapoport wrote:
> > > On Mon, Oct 26, 2020 at 06:05:30PM +0000, Edgecombe, Rick P wrote:
> > >
> > > > Beyond whatever you are seeing, for the latter case of new things
> > > > getting introduced to an interface with hidden dependencies... Another
> > > > edge case could be a new caller to set_memory_np() could result in
> > > > large NP pages. None of the callers today should cause this AFAICT, but
> > > > it's not great to rely on the callers to know these details.
>
> > > A caller of set_memory_*() or set_direct_map_*() should expect a failure
> > > and be ready for that. So adding a WARN to safe_copy_page() is the first
> > > step in that direction :)
> >
> > I am probably missing something important, but why are we saving/restoring
> > the content of pages that were explicitly removed from the identity mapping
> > such that nobody will access them?
>
> Actually, we should not be saving/restoring free pages during
> hibernation as there are several calls to mark_free_pages() that should
> exclude the free pages from the snapshot. I've tried to find why the fix
> that maps/unmaps a page to save it was required at the first place, but
> I could not find bug reports.
>
> The closest I've got is an email from Rafael that asked to update
> "hibernate: handle DEBUG_PAGEALLOC" patch:
>
> https://lore.kernel.org/linux-pm/200802200133.44098.rjw@xxxxxxx/
>
> Could it be that safe_copy_page() tries to workaround a non-existent
> problem?

Clould be! Also see

https://lkml.kernel.org/r/38de5bb0-5559-d069-0ce0-daec66ef2746@xxxxxxx

which restores free page content based on more kernel parameters, not 
based on the original content.

--
Thanks,

David / dhildenb