linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Pasha Tatashin <Pavel.Tatashin@microsoft.com>
To: Alexander Duyck <alexander.duyck@gmail.com>,
	"mhocko@kernel.org" <mhocko@kernel.org>
Cc: Dave Hansen <dave.hansen@intel.com>,
	linux-mm <linux-mm@kvack.org>,
	LKML <linux-kernel@vger.kernel.org>,
	"Duyck, Alexander H" <alexander.h.duyck@intel.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Ingo Molnar <mingo@kernel.org>,
	"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Subject: Re: [PATCH v2 1/2] mm: Move page struct poisoning to CONFIG_DEBUG_VM_PAGE_INIT_POISON
Date: Thu, 6 Sep 2018 16:12:36 +0000	[thread overview]
Message-ID: <4f154937-118c-96cf-cf8e-c95a2ca68d44@microsoft.com> (raw)
In-Reply-To: <CAKgT0UfiKWZO6hyjc1RpRTgD+CvM=KnbYokSueLFi7X5h+GMKQ@mail.gmail.com>



On 9/6/18 11:41 AM, Alexander Duyck wrote:
> On Thu, Sep 6, 2018 at 8:13 AM Michal Hocko <mhocko@kernel.org> wrote:
>>
>> On Thu 06-09-18 07:59:03, Dave Hansen wrote:
>>> On 09/05/2018 10:47 PM, Michal Hocko wrote:
>>>> why do you have to keep DEBUG_VM enabled for workloads where the boot
>>>> time matters so much that few seconds matter?
>>>
>>> There are a number of distributions that run with it enabled in the
>>> default build.  Fedora, for one.  We've basically assumed for a while
>>> that we have to live with it in production environments.
>>>
>>> So, where does leave us?  I think we either need a _generic_ debug
>>> option like:
>>>
>>>       CONFIG_DEBUG_VM_SLOW_AS_HECK
>>>
>>> under which we can put this an other really slow VM debugging.  Or, we
>>> need some kind of boot-time parameter to trigger the extra checking
>>> instead of a new CONFIG option.
>>
>> I strongly suspect nobody will ever enable such a scary looking config
>> TBH. Besides I am not sure what should go under that config option.
>> Something that takes few cycles but it is called often or one time stuff
>> that takes quite a long but less than aggregated overhead of the former?
>>
>> Just consider this particular case. It basically re-adds an overhead
>> that has always been there before the struct page init optimization
>> went it. The poisoning just returns it in a different form to catch
>> potential left overs. And we would like to have as many people willing
>> to running in debug mode to test for those paths because they are
>> basically impossible to review by the code inspection. More importantnly
>> the major overhead is boot time so my question still stands. Is this
>> worth a separate config option almost nobody is going to enable?
>>
>> Enabling DEBUG_VM by Fedora and others serves us a very good testing
>> coverage and I appreciate that because it has generated some useful bug
>> reports. Those people are paying quite a lot of overhead in runtime
>> which can aggregate over time is it so much to ask about one time boot
>> overhead?
> 
> The kind of boot time add-on I saw as a result of this was about 170
> seconds, or 2 minutes and 50 seconds on a 12TB system. I spent a
> couple minutes wondering if I had built a bad kernel or not as I was
> staring at a dead console the entire time after the grub prompt since
> I hit this so early in the boot. That is the reason why I am so eager
> to slice this off and make it something separate. I could easily see
> this as something that would get in the way of other debugging that is
> going on in a system.
> 
> If we don't want to do a config option, then what about adding a
> kernel parameter to put a limit on how much memory we will initialize
> like this before we just start skipping it. We could put a default
> limit on it like 256GB and then once we cross that threshold we just
> don't bother poisoning any more memory. With that we would probably be
> able to at least cover most of the early memory init, and that value
> should cover most systems without getting into delays on the order of
> minutes.

I am OK with a boot parameter to optionally disable it when DEBUG_VM is
enabled. But, I do not think it is a good idea to make that parameter
"smart" basically always poison memory with DEBUG_VM unless bootet with
a parameter that tells not to poison memory.

CONFIG_DEBUG_VM is disbled on:

RedHat, Oracle Linux, CentOS, Ubuntu, Arch Linux, SUSE

Enabled on:

Fedora

Are there other distros where it is enabled? I think, this could be
filed as a performance bug against Fedora distro, and let the decide
what to do about it.

I do not want to make this feature less tested. Poisoning memory allowed
us to catch corner case bugs like these:

ab1e8d8960b68f54af42b6484b5950bd13a4054b
mm: don't allow deferred pages with NEED_PER_CPU_KM

e181ae0c5db9544de9c53239eb22bc012ce75033
mm: zero unavailable pages before memmap init

And several more that were fixed by other people.

For a very long linux relied on assumption that boot memory is zeroed,
and I am sure we will continue detect more bugs over time.

Thank you,
Pavel

> 
> - Alex
> 

  reply	other threads:[~2018-09-06 16:12 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-05 21:13 [PATCH v2 0/2] Address issues slowing memory init Alexander Duyck
2018-09-05 21:13 ` [PATCH v2 1/2] mm: Move page struct poisoning to CONFIG_DEBUG_VM_PAGE_INIT_POISON Alexander Duyck
2018-09-05 21:22   ` Pasha Tatashin
2018-09-05 21:29     ` Alexander Duyck
2018-09-05 21:38       ` Pasha Tatashin
2018-09-05 21:54         ` Alexander Duyck
2018-09-05 21:34   ` Dave Hansen
2018-09-06  5:47   ` Michal Hocko
2018-09-06 14:59     ` Dave Hansen
2018-09-06 15:13       ` Michal Hocko
2018-09-06 15:41         ` Alexander Duyck
2018-09-06 16:12           ` Pasha Tatashin [this message]
2018-09-06 17:07             ` Dave Hansen
2018-09-06 18:08               ` Michal Hocko
2018-09-06 17:03           ` Michal Hocko
2018-09-06 17:23             ` Pasha Tatashin
2018-09-06 16:09         ` Dave Hansen
2018-09-06 17:08           ` Michal Hocko
2018-09-05 21:13 ` [PATCH v2 2/2] mm: Create non-atomic version of SetPageReserved for init use Alexander Duyck
2018-09-06  5:49   ` Michal Hocko

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=4f154937-118c-96cf-cf8e-c95a2ca68d44@microsoft.com \
    --to=pavel.tatashin@microsoft.com \
    --cc=akpm@linux-foundation.org \
    --cc=alexander.duyck@gmail.com \
    --cc=alexander.h.duyck@intel.com \
    --cc=dave.hansen@intel.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.org \
    --cc=mingo@kernel.org \
    /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).