From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758399Ab0CaW5O (ORCPT ); Wed, 31 Mar 2010 18:57:14 -0400 Received: from mx2.mail.elte.hu ([157.181.151.9]:43815 "EHLO mx2.mail.elte.hu" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758360Ab0CaW5N (ORCPT ); Wed, 31 Mar 2010 18:57:13 -0400 Date: Thu, 1 Apr 2010 00:56:58 +0200 From: Ingo Molnar To: Yinghai Lu Cc: James Morris , "H. Peter Anvin" , linux-kernel@vger.kernel.org, airlied@linux.ie, Linus Torvalds , Pekka Enberg Subject: Re: Config NO_BOOTMEM breaks my amd64 box Message-ID: <20100331225658.GA22975@elte.hu> References: <4BB2EB1B.8090303@zytor.com> <4BB3C739.2020106@kernel.org> <20100331221341.GA20441@elte.hu> <4BB3C9B0.4000205@kernel.org> <20100331224108.GA11284@elte.hu> <4BB3D0F4.902@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4BB3D0F4.902@kernel.org> User-Agent: Mutt/1.5.20 (2009-08-17) X-ELTE-SpamScore: 0.0 X-ELTE-SpamLevel: X-ELTE-SpamCheck: no X-ELTE-SpamVersion: ELTE 2.0 X-ELTE-SpamCheck-Details: score=0.0 required=5.9 tests=none autolearn=no SpamAssassin version=3.2.5 _SUMMARY_ Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Yinghai Lu wrote: > On 03/31/2010 03:41 PM, Ingo Molnar wrote: > > > > * Yinghai Lu wrote: > > > >> On 03/31/2010 03:13 PM, Ingo Molnar wrote: > >>> > >>> * Yinghai Lu 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