All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johannes Weiner <hannes@saeurebad.de>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-kernel@vger.kernel.org, mingo@elte.hu, andi@firstfloor.org,
	yhlu.kernel@gmail.com, ralf@linux-mips.org, rth@twiddle.net,
	rmk@arm.linux.org.uk, tony.luck@intel.com, takata@linux-m32r.org,
	geert@linux-m68k.org, kyle@parisc-linux.org, paulus@samba.org,
	lethal@linux-sh.org, davem@davemloft.net
Subject: Re: [RFC 0/3] bootmem rewrite
Date: Thu, 22 May 2008 02:33:45 +0200	[thread overview]
Message-ID: <87hccr2n4m.fsf@saeurebad.de> (raw)
In-Reply-To: <20080521165735.10c500dc.akpm@linux-foundation.org> (Andrew Morton's message of "Wed, 21 May 2008 16:57:35 -0700")

Hi Andrew,

Andrew Morton <akpm@linux-foundation.org> writes:

> On Wed, 21 May 2008 03:37:35 +0200
> Johannes Weiner <hannes@saeurebad.de> wrote:
>
>> Hi,
>> 
>> This is a complete overhaul of the bootmem allocator while preserving
>> its original functionality, excluding bugs.
>
> Where angels fear to tread.

Anyone who has seen this code realizes that this kind of advertisment is
essential to get people looking at patches for it ;)

>> free_bootmem and reserve_bootmem become a bit stricter than they are
>> right now, callsites have to make sure that the PFN range is
>> contiguous but it might go across node boundaries.
>> 
>> alloc_bootmem satisfying the allocation goal is more likely as the
>> routines will try to allocate on the node holding the goal first
>> before falling back as opposed to the original behaviour that
>> satisfies the goal only if it is on the first node.
>> 
>> All in all, I think the code has become simpler and cleaner.  All
>> public interfaces have been documented, too.
>> 
>> The first patch moves the bootmem node descriptor definitions into
>> bootmem.c where they belong.
>> 
>> The second patch is the new allocator itself.
>> 
>> The third patch converts all users of ->node_boot_start to
>> ->node_min_pfn as this is what they really use.  It then removes the
>> unused ->node_boot_start.
>> 
>> Compile and runtime tested on X86_32, therefor RFC only.
>> 
>>  arch/alpha/mm/numa.c             |    8 +-
>>  arch/arm/mm/discontig.c          |   34 +-
>>  arch/arm/plat-omap/fb.c          |    4 +-
>>  arch/avr32/mm/init.c             |    3 +-
>>  arch/ia64/mm/discontig.c         |   30 +-
>>  arch/m32r/mm/discontig.c         |    4 +-
>>  arch/m32r/mm/init.c              |    4 +-
>>  arch/m68k/mm/init.c              |    4 +-
>>  arch/mips/sgi-ip27/ip27-memory.c |    3 +-
>>  arch/mn10300/mm/init.c           |    6 +-
>>  arch/parisc/mm/init.c            |    3 +-
>>  arch/powerpc/mm/numa.c           |    3 +-
>>  arch/sh/mm/init.c                |    2 +-
>>  arch/sh/mm/numa.c                |    5 +-
>>  arch/sparc64/mm/init.c           |    3 +-
>>  arch/x86/mm/discontig_32.c       |    3 +-
>>  arch/x86/mm/numa_64.c            |    6 +-
>>  include/linux/bootmem.h          |  115 ++---
>>  mm/bootmem.c                     |  914 +++++++++++++++++++-------------------
>>  mm/page_alloc.c                  |    4 +-
>
> Oh gee.

Okay, seeing it again it looks a bit brutal.  But it's not.  The basic
principles are the same, it's not that I completely changed the
implementation.  Okay, perhaps I did.

And the arch changes are trivial.

> bootmem is an area where large numbers of people have done hit-and-run
> jobs over a lot of years.  Nobody owns it and I'm sure that you are now
> the world's expert.  We just need to push ahead with this, I guess.
>
> I expect there will be problems - so many architectures which do such
> different things, and all the configuration options churning things
> around.

I expect problems too.  I just can not go any further with it on the
resources I have.

> So how to move ahead with this?
>
> - I think I'd prefer not to drop
>
>   mm-fix-free_all_bootmem_core-alignment-check.patch
>   mm-normalize-internal-argument-passing-of-bootmem-data.patch
>   mm-unexport-__alloc_bootmem_core.patch
>
>   because those are small, simple things which are on track for
>   2.6.27 whereas a massive rewrite may take longer to get merged, and
>   may never get there at all, in which case we lost those little
>   fixes.

I can see that.

> - It would suit my purposes to have these patches right at the tail
>   of the -mm patch queue so that I can drop them easily if problems
>   occur, and so that others can revert them easily when diagnosing
>   problems.

Good.

> - It would be nice to get some review attention from architecture
>   guys, but I can understand them finding other things to do, when
>   bootmem is presumably good-enough-for-now.
>
> - Is x86_32 the only test platform which you have available?  Awkward.

Yes, unfortunately.  Hardware offers via private mail, please!

> Anyway, if you can redo these patches against most-recent-mm or,
> better, against http://userweb.kernel.org/~akpm/mmotm then it would
> make things easier for me to handle.  I can then at least test it all
> on my seven-odd test boxes.  Please feel free to ping me if you want a
> single rolled-up patch - that's always trivial and I can do it in three
> minutes.

I guess I just apply the quilt series to the tree you forked off?

> Finally, if you haven't done so, I'd encourage you to stuff as many
> handy debugging printks into this code as you possibly can.  Just fill
> 'er up with them.  So that when people start running it and it goes
> boom, they can send you their debug output _without_ having to go
> through another handful of email-email-patch-rebuild-retest cycles.  We
> can pull them all out later on.

Okay, I will make it gossip and send you a -mmotm-based version of it.

	Hannes

  reply	other threads:[~2008-05-22  0:34 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-05-21  1:37 [RFC 0/3] bootmem rewrite Johannes Weiner
2008-05-21  1:37 ` [RFC 1/3] mm: Move bootmem descriptors to a single place Johannes Weiner
2008-05-21  1:37 ` [RFC 2/3] mm: Reimplement bootmem Johannes Weiner
2008-05-21  1:37 ` [RFC 3/3] mm: Remove node_boot_start from bootmem_data_t Johannes Weiner
2008-05-21 23:57 ` [RFC 0/3] bootmem rewrite Andrew Morton
2008-05-22  0:33   ` Johannes Weiner [this message]
2008-05-22  1:10     ` 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=87hccr2n4m.fsf@saeurebad.de \
    --to=hannes@saeurebad.de \
    --cc=akpm@linux-foundation.org \
    --cc=andi@firstfloor.org \
    --cc=davem@davemloft.net \
    --cc=geert@linux-m68k.org \
    --cc=kyle@parisc-linux.org \
    --cc=lethal@linux-sh.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=paulus@samba.org \
    --cc=ralf@linux-mips.org \
    --cc=rmk@arm.linux.org.uk \
    --cc=rth@twiddle.net \
    --cc=takata@linux-m32r.org \
    --cc=tony.luck@intel.com \
    --cc=yhlu.kernel@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 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.