linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Mel Gorman <mgorman@techsingularity.net>
To: Michal Hocko <mhocko@kernel.org>
Cc: YASUAKI ISHIMATSU <yasu.isimatu@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	koki.sanagi@us.fujitsu.com
Subject: Re: [PATCH] mm, meminit: Serially initialise deferred memory if trace_buf_size is specified
Date: Thu, 16 Nov 2017 10:06:33 +0000	[thread overview]
Message-ID: <20171116100633.moui6zu33ctzpjsf@techsingularity.net> (raw)
In-Reply-To: <20171116085433.qmz4w3y3ra42j2ih@dhcp22.suse.cz>

On Thu, Nov 16, 2017 at 09:54:33AM +0100, Michal Hocko wrote:
> On Wed 15-11-17 14:17:52, YASUAKI ISHIMATSU wrote:
> > Hi Michal and Mel,
> > 
> > To reproduce the issue, I specified the large trace buffer. The issue also occurs with
> > trace_buf_size=12M and movable_node on 4.14.0.
> 
> This is still 10x more than the default. Why do you need it in the first
> place? You can of course find a size that will not fit into the initial
> memory but I am questioning why do you want something like that during
> early boot in the first place.
> 

This confused me as well. I couldn't think of a sensible use-case for
increasing the buffer other than stuffing trace_printk in multiple places
for debugging purposes. Even in such cases, it would be feasible to disable
the option in Kconfig just to have the large buffer.  Otherwise, just wait
until the system is booted and set if from userspace.

The lack of a sensible use-case is why I took a fairly blunt approach to
the problem. Keep it (relatively) simple and all that.

> The whole deferred struct page allocation operates under assumption
> that there are no large page allocator consumers that early during
> the boot process.

Yes.

> If this assumption is not correct then we probably
> need a generic way to describe this. Add-hoc trace specific thing is
> far from idea, imho. If anything the first approach to disable the
> deferred initialization via kernel command line option sounds much more
> appropriate and simpler to me.

So while the first approach was blunt, there are multiple other options.

1. Parse trace_buf_size in __setup to record the information before
   page alloc init starts. Take that into account in reset_deferred_meminit
   to increase the amount of memory that is serially initialised

2. Have tracing init with a small buffer and then resize it after
   page_alloc_init_late. This modifies tracing a bit but most of the
   helpers that are required are there. It would be more complex but
   it's doable

3. Add a kernel command line parameter that explicitly disables deferred
   meminit. We used to have something like this but it was never merged
   as we should be able to estimate how much memory is needed to boot.

4. Put a check into the page allocator slowpath that triggers serialised
   init if the system is booting and an allocation is about to fail. It
   would be such a cold path that it would never be noticable although it
   would leave dead code in the kernel image once boot had completed

However, it would be preferable by far to have a sensible use-case as to
why trace_buf_size= would be specified with a large buffer. It's hard to
know what level of complexity is justified without it.

-- 
Mel Gorman
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2017-11-16 10:06 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-15  8:55 Mel Gorman
2017-11-15 11:55 ` Michal Hocko
2017-11-15 14:13   ` Mel Gorman
2017-11-15 14:28     ` Michal Hocko
2017-11-15 14:43       ` Mel Gorman
2017-11-15 14:57         ` Michal Hocko
2017-11-15 19:17           ` YASUAKI ISHIMATSU
2017-11-16  8:54             ` Michal Hocko
2017-11-16 10:06               ` Mel Gorman [this message]
2017-11-17 18:19                 ` Pavel Tatashin
2017-11-17 21:32                   ` Mel Gorman
2017-11-30  3:41                     ` Pavel Tatashin
2017-12-06 10:50                       ` Mel Gorman
2018-01-31 17:28                         ` Koki.Sanagi
2018-01-31 18:24                           ` Pavel Tatashin
2018-02-05 14:14                             ` Masayoshi Mizuma
2018-02-05 15:26                               ` Pavel Tatashin
2017-11-21  1:04                   ` Andrew Morton
2017-11-30  3:49                     ` Pavel Tatashin
2017-11-15 19:49 ` Andrew Morton
2017-11-16  8:39   ` Mel Gorman

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=20171116100633.moui6zu33ctzpjsf@techsingularity.net \
    --to=mgorman@techsingularity.net \
    --cc=akpm@linux-foundation.org \
    --cc=koki.sanagi@us.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.org \
    --cc=yasu.isimatu@gmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox