From: Ingo Molnar <mingo@elte.hu>
To: Yinghai Lu <yinghai@kernel.org>
Cc: James Morris <jmorris@namei.org>,
"H. Peter Anvin" <hpa@zytor.com>,
linux-kernel@vger.kernel.org, airlied@linux.ie,
Linus Torvalds <torvalds@linux-foundation.org>,
Pekka Enberg <penberg@cs.helsinki.fi>
Subject: Re: Config NO_BOOTMEM breaks my amd64 box
Date: Thu, 1 Apr 2010 00:56:58 +0200 [thread overview]
Message-ID: <20100331225658.GA22975@elte.hu> (raw)
In-Reply-To: <4BB3D0F4.902@kernel.org>
* Yinghai Lu <yinghai@kernel.org> wrote:
> On 03/31/2010 03:41 PM, Ingo Molnar wrote:
> >
> > * Yinghai Lu <yinghai@kernel.org> wrote:
> >
> >> On 03/31/2010 03:13 PM, Ingo Molnar wrote:
> >>>
> >>> * Yinghai Lu <yinghai@kernel.org> wrote:
> >>>
> >>>> --- linux-2.6.orig/arch/x86/mm/init_32.c
> >>>> +++ linux-2.6/arch/x86/mm/init_32.c
> >>>> @@ -875,7 +875,12 @@ void __init mem_init(void)
> >>>> BUG_ON(!mem_map);
> >>>> #endif
> >>>> /* this will put all low memory onto the freelists */
> >>>> +#if defined(CONFIG_NO_BOOTMEM) && defined(MAX_NUMNODES)
> >>>> + /* In case some 32bit systems don't have RAM installed on node0 */
> >>>> + totalram_pages += free_all_memory_core_early(MAX_NUMNODES);
> >>>
> >>> (Note: tab whitespace damage)
> >>>
> >>>> +#else
> >>>> totalram_pages += free_all_bootmem();
> >>>
> >>> So we get into this branch if CONFIG_NO_BOOTMEM is enabled but MAX_NUMNODES is
> >>> not defined? Doesnt look right.
> >>
> >> yes.
> >>
> >> free_all_bootmem() will call
> >> free_all_memory_core_early(NODE_DATA(0)->node_id);
> >>
> >> Thanks
> >
> > Well and that whole #ifdeffery is disgusting as well - even if the goal was to
> > remove CONFIG_NO_BOOTMEM ASAP.
> >
> > Please learn to use proper intermediate helper functions and at minimum put
> > the conversion ugliness somewhere that doesnt intrude our daily flow in .c
> > files. The best rule is to _never ever_ put an #ifdef construct into a .c
> > file. It doesnt matter what the goal if the #ifdef is - such ugliness in code
> > is never justified.
> >
>
> if you agree that i can have one nobootmem.c in mm/
I think what we want is your lmb series, with CONFIG_NO_BOOTMEM eliminated
altogether and x86 converted to pure (extended) lmb facilities, and without
any traces of bootmem left in x86.
I.e. a really clean series with no CONFIG_NO_BOOTMEM kind of #ifdef crap left
around. This means 'nobootmem.c' (albeit saner than an #ifdef jungle) would be
moot as well.
We tried the dual model as it seemed prudent from a testing/conversion POV
(and it certainly allowed people to turn the new code off), but it's rather
ugly and we still have bugs left.
This means that if Linus likes that approach the conversion will be very
binary and very painful. The other option would be to go back to bootmem and
forget about the whole nobootmem and lmb thing.
Ingo
next prev parent reply other threads:[~2010-03-31 22:57 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-03-31 4:49 Config NO_BOOTMEM breaks my amd64 box James Morris
2010-03-31 6:26 ` H. Peter Anvin
2010-03-31 6:47 ` James Morris
2010-03-31 16:25 ` Yinghai Lu
2010-03-31 18:59 ` Ingo Molnar
2010-03-31 20:57 ` Dave Airlie
2010-03-31 21:02 ` Linus Torvalds
2010-03-31 21:40 ` Ingo Molnar
2010-03-31 21:47 ` Ingo Molnar
2010-03-31 21:14 ` Dave Airlie
2010-03-31 22:02 ` Yinghai Lu
2010-03-31 22:28 ` H. Peter Anvin
2010-03-31 22:58 ` James Morris
2010-03-31 23:02 ` Ingo Molnar
2010-03-31 23:35 ` H. Peter Anvin
2010-03-31 23:43 ` James Morris
2010-03-31 23:48 ` H. Peter Anvin
2010-04-01 1:00 ` James Morris
2010-04-01 12:52 ` Ingo Molnar
2010-04-08 6:32 ` Ingo Molnar
2010-04-08 7:00 ` Yinghai
2010-04-08 7:27 ` Ingo Molnar
2010-04-09 2:43 ` Dave Airlie
2010-04-08 8:05 ` James Morris
2010-04-08 8:22 ` Ingo Molnar
2010-03-31 22:05 ` Yinghai Lu
2010-03-31 22:13 ` Ingo Molnar
2010-03-31 22:16 ` Yinghai Lu
2010-03-31 22:41 ` Ingo Molnar
2010-03-31 22:47 ` Yinghai Lu
2010-03-31 22:56 ` Ingo Molnar [this message]
2010-04-01 0:01 ` Johannes Weiner
2010-03-31 23:34 ` H. Peter Anvin
2010-03-31 23:54 ` Yinghai Lu
2010-04-01 0:35 ` H. Peter Anvin
2010-04-01 1:07 ` Yinghai Lu
2010-04-01 2:02 ` [PATCH -v3] nobootmem/bootmem, x86: Fix 32bit numa system without RAM on Node0 Yinghai Lu
2010-04-01 3:18 ` H. Peter Anvin
2010-04-01 3:30 ` Yinghai Lu
2010-04-01 3:44 ` [PATCH -v4 1/2] nobootmem, " Yinghai Lu
2010-04-01 3:45 ` [PATCH -v4 2/2] bootmem, " Yinghai Lu
2010-04-01 22:57 ` [tip:x86/urgent] bootmem, x86: Fix 32bit numa system without RAM on node 0 tip-bot for Yinghai Lu
2010-04-01 22:57 ` [tip:x86/urgent] nobootmem, " tip-bot for Yinghai Lu
2010-03-31 10:51 ` Config NO_BOOTMEM breaks my amd64 box Stefan Richter
-- strict thread matches above, loose matches on Subject: below --
2010-04-01 3:16 H. Peter Anvin
2010-04-01 3:35 ` 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=20100331225658.GA22975@elte.hu \
--to=mingo@elte.hu \
--cc=airlied@linux.ie \
--cc=hpa@zytor.com \
--cc=jmorris@namei.org \
--cc=linux-kernel@vger.kernel.org \
--cc=penberg@cs.helsinki.fi \
--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).