linux-arch.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Yinghai Lu <yinghai@kernel.org>
Cc: Ingo Molnar <mingo@elte.hu>, Thomas Gleixner <tglx@linutronix.de>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	David Miller <davem@davemloft.net>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Johannes Weiner <hannes@cmpxchg.org>,
	linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org
Subject: Re: [PATCH 01/16] x86, memblock: Add memblock_x86_find_in_range_size()
Date: Wed, 28 Jul 2010 14:49:18 +1000	[thread overview]
Message-ID: <1280292558.1970.222.camel@pasglop> (raw)
In-Reply-To: <1279824241-17582-2-git-send-email-yinghai@kernel.org>

On Thu, 2010-07-22 at 11:43 -0700, Yinghai Lu wrote:
> size is returned according free range.
> Will be used to find free ranges for early_memtest and memory corruption check
> 
> Do not mess it up with lib/memblock.c yet.

So I'm readying my branch which incudes your 6 patches, however I'm
still not happy with a few things.

For example, I really really don't like exporting
memblock_reserved_init_regions.

So you do those things for the sake of x86, so let's walk through your
x86 patches and see if I can figure out what you do wrong :-)

> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> ---
>  arch/x86/include/asm/memblock.h |    8 ++++
>  arch/x86/mm/Makefile            |    2 +
>  arch/x86/mm/memblock.c          |   88 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 98 insertions(+), 0 deletions(-)
>  create mode 100644 arch/x86/include/asm/memblock.h
>  create mode 100644 arch/x86/mm/memblock.c
> 
> diff --git a/arch/x86/include/asm/memblock.h b/arch/x86/include/asm/memblock.h
> new file mode 100644
> index 0000000..c14219a
> --- /dev/null
> +++ b/arch/x86/include/asm/memblock.h
> @@ -0,0 +1,8 @@
> +#ifndef _X86_MEMBLOCK_H
> +#define _X86_MEMBLOCK_H
> +
> +#define ARCH_DISCARD_MEMBLOCK

So I'm no fan of this ARCH_DISCARD_MEMBLOCK, especially since it makes a
lot of sense to keep the debugfs files around or maybe even move to
sysfs for diagnostic purposes.

Maybe we should consider something better by having memblock always be
init/initdata but we copy the arrays to a "final" location from an
initcall ? We can keep your patches for now, but I think we should
improve on that.
 
> +u64 memblock_x86_find_in_range_size(u64 start, u64 *sizep, u64 align);

I really really don't like your function naming. The above doesn't give
me any bloody idea about what the purpose of the function is...

Cheers,
Ben.

  parent reply	other threads:[~2010-07-28  4:49 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-07-22 18:43 [PATCH -v26 00/16] Use memblock with x86 Yinghai Lu
2010-07-22 18:43 ` Yinghai Lu
2010-07-22 18:43 ` [PATCH 01/16] x86, memblock: Add memblock_x86_find_in_range_size() Yinghai Lu
2010-07-22 18:43   ` Yinghai Lu
2010-07-28  4:49   ` Benjamin Herrenschmidt [this message]
2010-07-22 18:43 ` [PATCH 02/16] bootmem, x86: Add weak version of reserve_bootmem_generic Yinghai Lu
2010-07-22 18:43   ` Yinghai Lu
2010-07-28  4:51   ` Benjamin Herrenschmidt
2010-07-22 18:43 ` [PATCH 03/16] x86, memblock: Add memblock_x86_to_bootmem() Yinghai Lu
2010-07-22 18:43   ` Yinghai Lu
2010-07-28  5:00   ` Benjamin Herrenschmidt
2010-07-28  5:40     ` Yinghai Lu
2010-07-22 18:43 ` [PATCH 04/16] x86,memblock: Add memblock_x86_reserve_range/memblock_x86_free_range Yinghai Lu
2010-07-22 18:43   ` Yinghai Lu
2010-07-22 18:43 ` [PATCH 05/16] x86, memblock: Add get_free_all_memory_range() Yinghai Lu
2010-07-22 18:43   ` Yinghai Lu
2010-07-28  5:02   ` Benjamin Herrenschmidt
2010-07-28  5:41     ` Yinghai Lu
2010-07-28 19:15     ` Yinghai Lu
2010-07-22 18:43 ` [PATCH 06/16] x86, memblock: Add memblock_x86_register_active_regions() and memblock_x86_hole_size() Yinghai Lu
2010-07-22 18:43   ` Yinghai Lu
2010-07-22 18:43 ` [PATCH 07/16] memblock: Add find_memory_core_early() Yinghai Lu
2010-07-22 18:43   ` Yinghai Lu
2010-07-22 18:43 ` [PATCH 08/16] x86, memblock: Add memblock_x86_find_in_range_node() Yinghai Lu
2010-07-22 18:43   ` Yinghai Lu
2010-07-22 18:43 ` [PATCH 09/16] x86, memblock: Add memblock_x86_free_memory_in_range() Yinghai Lu
2010-07-22 18:43   ` Yinghai Lu
2010-07-22 18:43 ` [PATCH 10/16] x86, memblock: Add memblock_x86_memory_in_range() Yinghai Lu
2010-07-22 18:43   ` Yinghai Lu
2010-07-22 18:43 ` [PATCH 11/16] x86, memblock: Use memblock_debug to control debug message print out Yinghai Lu
2010-07-22 18:43   ` Yinghai Lu
2010-07-22 18:43 ` [PATCH 12/16] x86: Use memblock to replace early_res Yinghai Lu
2010-07-22 18:43   ` Yinghai Lu
2010-07-22 18:43 ` [PATCH 13/16] x86: Replace e820_/_early string with memblock_ Yinghai Lu
2010-07-22 18:43   ` Yinghai Lu
2010-07-22 18:43 ` [PATCH 14/16] x86: Remove not used early_res code Yinghai Lu
2010-07-22 18:43   ` Yinghai Lu
2010-07-22 18:44 ` [PATCH 15/16] x86, memblock: Use memblock_memory_size()/memblock_free_memory_size() to get correct dma_reserve Yinghai Lu
2010-07-22 18:44   ` Yinghai Lu
2010-07-22 18:44 ` [PATCH 16/16] x86: remove old bootmem code Yinghai Lu
2010-07-22 18:44   ` Yinghai Lu

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=1280292558.1970.222.camel@pasglop \
    --to=benh@kernel.crashing.org \
    --cc=akpm@linux-foundation.org \
    --cc=davem@davemloft.net \
    --cc=hannes@cmpxchg.org \
    --cc=hpa@zytor.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=yinghai@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).