Re: [PATCH v1 0/2] mm/kdump: exclude reserved pages in dumps

From: David Hildenbrand <david@redhat.com>
To: Michal Hocko <mhocko@kernel.org>
Cc: "Vlastimil Babka" <vbabka@suse.cz>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"Baoquan He" <bhe@redhat.com>, "Dave Young" <dyoung@redhat.com>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Hari Bathini" <hbathini@linux.vnet.ibm.com>,
	"Huang Ying" <ying.huang@intel.com>,
	"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
	"Marc-André Lureau" <marcandre.lureau@redhat.com>,
	"Matthew Wilcox" <mawilcox@microsoft.com>,
	"Miles Chen" <miles.chen@mediatek.com>,
	"Pavel Tatashin" <pasha.tatashin@oracle.com>,
	"Petr Tesarik" <ptesarik@suse.cz>
Subject: Re: [PATCH v1 0/2] mm/kdump: exclude reserved pages in dumps
Date: Thu, 26 Jul 2018 10:22:41 +0200	[thread overview]
Message-ID: <e5264f8e-2bb5-7a9b-6352-ad18f04d49c2@redhat.com> (raw)
In-Reply-To: <20180724072237.GA28386@dhcp22.suse.cz>

On 24.07.2018 09:22, Michal Hocko wrote:
> On Mon 23-07-18 19:12:58, David Hildenbrand wrote:
>> On 23.07.2018 13:45, Vlastimil Babka wrote:
>>> On 07/20/2018 02:34 PM, David Hildenbrand wrote:
>>>> Dumping tools (like makedumpfile) right now don't exclude reserved pages.
>>>> So reserved pages might be access by dump tools although nobody except
>>>> the owner should touch them.
>>>
>>> Are you sure about that? Or maybe I understand wrong. Maybe it changed
>>> recently, but IIRC pages that are backing memmap (struct pages) are also
>>> PG_reserved. And you definitely do want those in the dump.
>>
>> I proposed a new flag/value to mask pages that are logically offline but
>> Michal wanted me to go into this direction.
>>
>> While we can special case struct pages in dump tools ("we have to
>> read/interpret them either way, so we can also dump them"), it smells
>> like my original attempt was cleaner. Michal?
> 
> But we do not have many page flags spare and even if we have one or two
> this doesn't look like the use for them. So I still think we should try
> the PageReserved way.
> 

So as a summary, the only real approach that would be acceptable is
using PageReserved + some other identifier to mark pages as "logically
offline".

I wonder what identifier could be used, as this has to be consistent for
all reserved pages (to avoid false positives).

Using other pageflags in combination might be possible, but then we have
to make assumptions about all users of PageReserved right now.

As far as I can see (and as has been discussed), page_type could be
used. If we don't want to consume a new bit, we could overload/reuse the
"PG_balloon" bit.

E.g. "PG_balloon" set -> exclude page from dump

PG_balloon + !PG_reserved -> ballooned page (state as of now)
PG_balloon + PG_reserved -> offline page

This way, even pages inflated by virtio_balloon would not be touched.

Opinions?

-- 

Thanks,

David / dhildenb