linux-arch.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mike Yoknis <mike.yoknis@hp.com>
To: Mel Gorman <mgorman@suse.de>
Cc: mingo@redhat.com, akpm@linux-foundation.org,
	linux-arch@vger.kernel.org, mmarek@suse.cz, tglx@linutronix.de,
	hpa@zytor.com, arnd@arndb.de, sam@ravnborg.org,
	minchan@kernel.org, kamezawa.hiroyu@jp.fujitsu.com,
	mhocko@suse.cz, linux-kbuild@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [PATCH] mm: memmap_init_zone() performance improvement
Date: Tue, 09 Oct 2012 08:56:37 -0600	[thread overview]
Message-ID: <1349794597.29752.10.camel@MikesLinux.fc.hp.com> (raw)
In-Reply-To: <20121008151656.GM29125@suse.de>

On Mon, 2012-10-08 at 16:16 +0100, Mel Gorman wrote:
> On Wed, Oct 03, 2012 at 08:56:14AM -0600, Mike Yoknis wrote:
> > memmap_init_zone() loops through every Page Frame Number (pfn),
> > including pfn values that are within the gaps between existing
> > memory sections.  The unneeded looping will become a boot
> > performance issue when machines configure larger memory ranges
> > that will contain larger and more numerous gaps.
> > 
> > The code will skip across invalid sections to reduce the
> > number of loops executed.
> > 
> > Signed-off-by: Mike Yoknis <mike.yoknis@hp.com>
> 
> This only helps SPARSEMEM and changes more headers than should be
> necessary. It would have been easier to do something simple like
> 
> if (!early_pfn_valid(pfn)) {
> 	pfn = ALIGN(pfn + MAX_ORDER_NR_PAGES, MAX_ORDER_NR_PAGES) - 1;
> 	continue;
> }
> 
> because that would obey the expectation that pages within a
> MAX_ORDER_NR_PAGES-aligned range are all valid or all invalid (ARM is the
> exception that breaks this rule). It would be less efficient on
> SPARSEMEM than what you're trying to merge but I do not see the need for
> the additional complexity unless you can show it makes a big difference
> to boot times.
> 

Mel,
I, too, was concerned that pfn_valid() was defined in so many header
files.  But, I did not feel that it was appropriate for me to try to
restructure things to consolidate those definitions just to add this one
new function.  Being a kernel newbie I did not believe that I had a good
enough understanding of what combinations and permutations of CONFIG and
architecture may have made all of those different definitions necessary,
so I left them in.

Yes, indeed, this fix is targeted for systems that have holes in memory.
That is where we see the problem.  We are creating large computer
systems and we would like for those machines to perform well, including
boot times.

Let me pass along the numbers I have.  We have what we call an
"architectural simulator".  It is a computer program that pretends that
it is a computer system.  We use it to test the firmware before real
hardware is available.  We have booted Linux on our simulator.  As you
would expect it takes longer to boot on the simulator than it does on
real hardware.

With my patch - boot time 41 minutes
Without patch - boot time 94 minutes

These numbers do not scale linearly to real hardware.  But indicate to
me a place where Linux can be improved.

Mike Yoknis

  parent reply	other threads:[~2012-10-09 14:56 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-03 14:56 [PATCH] mm: memmap_init_zone() performance improvement Mike Yoknis
2012-10-06 23:59 ` Ni zhan Chen
2012-10-06 23:59   ` Ni zhan Chen
2012-10-08 15:16 ` Mel Gorman
2012-10-08 15:16   ` Mel Gorman
2012-10-09  0:42   ` Ni zhan Chen
2012-10-09  0:42     ` Ni zhan Chen
2012-10-09 14:56   ` Mike Yoknis [this message]
2012-10-19 19:53     ` Mike Yoknis
2012-10-20  8:29       ` Mel Gorman
2012-10-20  8:29         ` Mel Gorman
2012-10-24 15:47         ` Mike Yoknis
2012-10-24 15:47           ` Mike Yoknis
2012-10-25  9:44           ` Mel Gorman
2012-10-26 22:47             ` [PATCH v2] " Mike Yoknis
2012-10-26 22:47               ` Mike Yoknis
2012-10-30 22:31               ` Andrew Morton
2012-10-30 22:31                 ` Andrew Morton
2012-10-30 15:14         ` [PATCH] " Dave Hansen
2012-10-30 15:14           ` Dave Hansen
2012-11-06 16:03           ` Mike Yoknis
2012-11-06 16:03             ` Mike Yoknis
2012-12-18 23:03             ` Andrew Morton

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=1349794597.29752.10.camel@MikesLinux.fc.hp.com \
    --to=mike.yoknis@hp.com \
    --cc=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=hpa@zytor.com \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@suse.de \
    --cc=mhocko@suse.cz \
    --cc=minchan@kernel.org \
    --cc=mingo@redhat.com \
    --cc=mmarek@suse.cz \
    --cc=sam@ravnborg.org \
    --cc=tglx@linutronix.de \
    /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).