All of lore.kernel.org
 help / color / mirror / Atom feed
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: Tue, 24 Jul 2018 11:18:39 +0200	[thread overview]
Message-ID: <aae9f664-2a49-9e71-6eb5-a63f535c2079@redhat.com> (raw)
In-Reply-To: <20180724085358.GG28386@dhcp22.suse.cz>

On 24.07.2018 10:53, Michal Hocko wrote:
> On Tue 24-07-18 10:46:20, David Hildenbrand wrote:
>> On 24.07.2018 09:25, Michal Hocko wrote:
>>> On Mon 23-07-18 19:20:43, David Hildenbrand wrote:
>>>> On 23.07.2018 14:30, Michal Hocko wrote:
>>>>> On Mon 23-07-18 13:45:18, 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.
>>>>>
>>>>> You are right. reserve_bootmem_region will make all early bootmem
>>>>> allocations (including those backing memmaps) PageReserved. I have asked
>>>>> several times but I haven't seen a satisfactory answer yet. Why do we
>>>>> even care for kdump about those. If they are reserved the nobody should
>>>>> really look at those specific struct pages and manipulate them. Kdump
>>>>> tools are using a kernel interface to read the content. If the specific
>>>>> content is backed by a non-existing memory then they should simply not
>>>>> return anything.
>>>>>
>>>>
>>>> "new kernel" provides an interface to read memory from "old kernel".
>>>>
>>>> The new kernel has no idea about
>>>> - which memory was added/online in the old kernel
>>>> - where struct pages of the old kernel are and what their content is
>>>> - which memory is save to touch and which not
>>>>
>>>> Dump tools figure all that out by interpreting the VMCORE. They e.g.
>>>> identify "struct pages" and see if they should be dumped. The "new
>>>> kernel" only allows to read that memory. It cannot hinder to crash the
>>>> system (e.g. if a dump tool would try to read a hwpoison page).
>>>>
>>>> So how should the "new kernel" know if a page can be touched or not?
>>>
>>> I am sorry I am not familiar with kdump much. But from what I remember
>>> it reads from /proc/vmcore and implementation of this interface should
>>> simply return EINVAL or alike when you try to dump inaccessible memory
>>> range.
>>
>> I assume the main problem with this approach is that we would always
>> have to fallback to reading old memory from vmcore page by page. e.g.
>> makedumpfile will always try to read bigger bunches. I also assume the
>> reason HWPOISON is handled in dump tools instead of in the kernel using
>> the mechanism you describe is the case.
> 
> Is falling back to page-by-page for some ranges a real problem? I mean
> most of pages will simply be there so you can go in larger chunks. Once
> you get EINVAL, you just fall back to page-by-page for that particular
> range.
> 

Looking at makedumpfile code, I assume implementation wise it should be
possible. They always try to read 256 pages at a time. If we get an
-EINVAL (-EIO?) we could fallback to reading page by page.

This implies having to properly handle exceptions when accessing memory.
Not sure if that will be easy. Maybe is_ram_page() is the better
alternative, because it hinders us from trying to read invalid memory
(or memory with random content) in the first place.

Will have to think about this and look into the details.

-- 

Thanks,

David / dhildenb

  reply	other threads:[~2018-07-24  9:18 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-20 12:34 [PATCH v1 0/2] mm/kdump: exclude reserved pages in dumps David Hildenbrand
2018-07-20 12:34 ` [PATCH v1 1/2] mm: clarify semantics of reserved pages David Hildenbrand
2018-07-20 12:34   ` David Hildenbrand
2018-07-23 10:48   ` Michal Hocko
2018-07-23 10:48     ` Michal Hocko
2018-07-20 12:34 ` [PATCH v1 2/2] kdump: include PG_reserved value in VMCOREINFO David Hildenbrand
2018-07-23 11:45 ` [PATCH v1 0/2] mm/kdump: exclude reserved pages in dumps Vlastimil Babka
2018-07-23 12:30   ` Michal Hocko
2018-07-23 17:20     ` David Hildenbrand
2018-07-24  7:25       ` Michal Hocko
2018-07-24  8:46         ` David Hildenbrand
2018-07-24  8:53           ` Michal Hocko
2018-07-24  9:18             ` David Hildenbrand [this message]
2018-07-24 12:17         ` David Hildenbrand
2018-07-24 13:13           ` Michal Hocko
2018-07-24 13:27             ` David Hildenbrand
2018-07-24 13:35               ` Michal Hocko
2018-07-24 14:13                 ` David Hildenbrand
2018-07-25 13:51                   ` Michal Hocko
2018-07-25 14:20                     ` David Hildenbrand
2018-07-26  8:27                       ` Michal Hocko
2018-07-26  8:37                         ` David Hildenbrand
2018-07-24  9:47     ` Vlastimil Babka
2018-07-24 11:19       ` Michal Hocko
2018-07-24 12:22         ` Vlastimil Babka
2018-07-24 12:33           ` David Hildenbrand
2018-07-24 13:06           ` Michal Hocko
2018-07-23 17:12   ` David Hildenbrand
2018-07-24  7:22     ` Michal Hocko
2018-07-24  9:48       ` Vlastimil Babka
2018-07-26  8:22       ` David Hildenbrand
2018-07-26  8:30         ` Michal Hocko
2018-07-26  8:45           ` David Hildenbrand
2018-07-26 19:50             ` Andrew Morton
2018-07-30  8:17               ` David Hildenbrand

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=aae9f664-2a49-9e71-6eb5-a63f535c2079@redhat.com \
    --to=david@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=bhe@redhat.com \
    --cc=dyoung@redhat.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hbathini@linux.vnet.ibm.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=marcandre.lureau@redhat.com \
    --cc=mawilcox@microsoft.com \
    --cc=mhocko@kernel.org \
    --cc=miles.chen@mediatek.com \
    --cc=pasha.tatashin@oracle.com \
    --cc=ptesarik@suse.cz \
    --cc=vbabka@suse.cz \
    --cc=ying.huang@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.