nvdimm.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Alexander Duyck <alexander.duyck@gmail.com>
To: Dave Hansen <dave.hansen@intel.com>,
	mhocko@kernel.org, pavel.tatashin@microsoft.com,
	dan.j.williams@intel.com
Cc: linux-mm <linux-mm@kvack.org>,
	LKML <linux-kernel@vger.kernel.org>,
	linux-nvdimm@lists.01.org, dave.jiang@intel.com,
	Ingo Molnar <mingo@kernel.org>,
	jglisse@redhat.com, Andrew Morton <akpm@linux-foundation.org>,
	logang@deltatee.com,
	"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Subject: Re: [PATCH 1/4] mm: Provide kernel parameter to allow disabling page init poisoning
Date: Wed, 12 Sep 2018 09:36:37 -0700	[thread overview]
Message-ID: <CAKgT0UeKnaY4XebOmtGozbjEJN4k3cwyhdBLPPJLc677-QU+Sw@mail.gmail.com> (raw)
In-Reply-To: <841e8101-40db-9ff2-f688-5f175d91fc31@intel.com>

On Wed, Sep 12, 2018 at 8:25 AM Dave Hansen <dave.hansen@intel.com> wrote:
>
> On 09/12/2018 07:49 AM, Alexander Duyck wrote:
> >>> +     page_init_poison=       [KNL] Boot-time parameter changing the
> >>> +                     state of poisoning of page structures during early
> >>> +                     boot. Used to verify page metadata is not accessed
> >>> +                     prior to initialization. Available with
> >>> +                     CONFIG_DEBUG_VM=y.
> >>> +                     off: turn off poisoning
> >>> +                     on: turn on poisoning (default)
> >>> +
> >> what about the following wording or something along those lines
> >>
> >> Boot-time parameter to control struct page poisoning which is a
> >> debugging feature to catch unitialized struct page access. This option
> >> is available only for CONFIG_DEBUG_VM=y and it affects boot time
> >> (especially on large systems). If there are no poisoning bugs reported
> >> on the particular system and workload it should be safe to disable it to
> >> speed up the boot time.
> > That works for me. I will update it for the next release.
>
> FWIW, I rather liked Dan's idea of wrapping this under
> vm_debug=<something>.  We've got a zoo of boot options and it's really
> hard to _remember_ what does what.  For this case, we're creating one
> that's only available under a specific debug option and I think it makes
> total sense to name the boot option accordingly.
>
> For now, I think it makes total sense to do vm_debug=all/off.  If, in
> the future, we get more options, we can do things like slab does and do
> vm_debug=P (for Page poison) for this feature specifically.
>
>         vm_debug =      [KNL] Available with CONFIG_DEBUG_VM=y.
>                         May slow down boot speed, especially on larger-
>                         memory systems when enabled.
>                         off: turn off all runtime VM debug features
>                         all: turn on all debug features (default)

This would introduce a significant amount of code change if we do it
as a parameter that has control over everything.

I would be open to something like "vm_debug_disables=" where we could
then pass individual values like 'P' for disabling page poisoning.
However doing this as a generic interface that could disable
everything now would be messy. I could then also update the print
message so that it lists what is disabled, and what was left enabled.
Then as we need to disable things in the future we could add
additional letters for individual features. I just don't want us
preemptively adding control flags for features that may never need to
be toggled.

I would want to hear from Michal on this before I get too deep into it
as he seemed to be of the opinion that we were already doing too much
code for this and it seems like this is starting to veer off in that
direction.

- Alex

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

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-10 23:43 [PATCH 0/4] Address issues slowing persistent memory initialization Alexander Duyck
2018-09-10 23:43 ` [PATCH 1/4] mm: Provide kernel parameter to allow disabling page init poisoning Alexander Duyck
2018-09-11  0:35   ` Alexander Duyck
2018-09-11 16:50   ` Dan Williams
2018-09-11 20:01     ` Alexander Duyck
2018-09-11 20:24       ` Dan Williams
2018-09-12 13:24   ` Pasha Tatashin
2018-09-12 14:10   ` Michal Hocko
2018-09-12 14:49     ` Alexander Duyck
2018-09-12 15:23       ` Dave Hansen
2018-09-12 16:36         ` Alexander Duyck [this message]
2018-09-12 16:43           ` Dave Hansen
2018-09-10 23:43 ` [PATCH 2/4] mm: Create non-atomic version of SetPageReserved for init use Alexander Duyck
2018-09-12 13:28   ` Pasha Tatashin
2018-09-10 23:43 ` [PATCH 3/4] mm: Defer ZONE_DEVICE page initialization to the point where we init pgmap Alexander Duyck
2018-09-11  7:49   ` kbuild test robot
2018-09-11  7:54   ` kbuild test robot
2018-09-11 22:35   ` Dan Williams
2018-09-12  0:51     ` Alexander Duyck
2018-09-12  0:59       ` Dan Williams
2018-09-12 13:59   ` Pasha Tatashin
2018-09-12 15:48     ` Alexander Duyck
2018-09-12 15:54       ` Pasha Tatashin
2018-09-12 16:44         ` Alexander Duyck
2018-09-12 16:50       ` Dan Williams
2018-09-12 17:46         ` Pasha Tatashin
2018-09-12 19:11           ` Dan Williams
2018-09-10 23:44 ` [PATCH 4/4] nvdimm: Trigger the device probe on a cpu local to the device Alexander Duyck
2018-09-11  0:37   ` Alexander Duyck
2018-09-12  5:48   ` Dan Williams
2018-09-12 13:44   ` Pasha Tatashin

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=CAKgT0UeKnaY4XebOmtGozbjEJN4k3cwyhdBLPPJLc677-QU+Sw@mail.gmail.com \
    --to=alexander.duyck@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=dan.j.williams@intel.com \
    --cc=dave.hansen@intel.com \
    --cc=dave.jiang@intel.com \
    --cc=jglisse@redhat.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-nvdimm@lists.01.org \
    --cc=logang@deltatee.com \
    --cc=mhocko@kernel.org \
    --cc=mingo@kernel.org \
    --cc=pavel.tatashin@microsoft.com \
    --subject='Re: [PATCH 1/4] mm: Provide kernel parameter to allow disabling page init poisoning' \
    /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

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).