[PATCH 2/2 v2] powerpc: Enable NO_BOOTMEM
Emilian.Medve at Freescale.com
Thu May 8 07:26:55 EST 2014
On 05/06/2014 09:44 PM, Scott Wood wrote:
> On Tue, 2014-05-06 at 19:16 -0500, Emil Medve wrote:
>> Hello Scott,
>> On 05/06/2014 04:49 PM, Scott Wood wrote:
>>> On Tue, 2014-05-06 at 13:48 -0500, Emil Medve wrote:
>>>> Currently bootmem is just a wrapper around memblock. This gets rid of
>>>> the wrapper code just as other ARHC(es) did: x86, arm, etc.
>>>> For now only cover !NUMA systems/builds
>>>> Signed-off-by: Emil Medve <Emilian.Medve at Freescale.com>
>>>> v2: Acknowledge that NUMA systems/builds are not covered by this patch
>>>> arch/powerpc/Kconfig | 3 +++
>>>> arch/powerpc/mm/mem.c | 8 ++++++++
>>>> 2 files changed, 11 insertions(+)
>>>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>>>> index e099899..07b164b 100644
>>>> --- a/arch/powerpc/Kconfig
>>>> +++ b/arch/powerpc/Kconfig
>>>> @@ -475,6 +475,9 @@ config SYS_SUPPORTS_HUGETLBFS
>>>> source "mm/Kconfig"
>>>> +config NO_BOOTMEM
>>>> + def_bool !NUMA
>>> This will allow a user to manually turn on CONFIG_NO_BOOTMEM in the
>>> presence of NUMA. From the changelog it sounds like this is not what
>>> you intended.
>>> What are the issues with NUMA?
>> Well, I don't have access to a NUMA box/board. I could enable NUMA for a
>> !NUMA board but I'd feel better if I could actually test/debug on a
>> relevant system
> You could first test with NUMA on a non-NUMA board, and then if that
> works ask the list for help testing on NUMA hardware (and various
> non-Freescale non-NUMA hardware, for that matter).
> Is there a specific issue that would need to be addressed to make it
> work on NUMA?
Nope. Just different code/file(s)... with NUMA specific details...
>>> As is, you're not getting rid of wrapper code -- only adding ifdefs.
>> First, you're talking about the bootmem initialization wrapper code for
>> powerpc. The actual bootmem code is in include/linux/bootmem.h and
>> mm/bootmem.c. We can't remove those files as they are still used by
>> other arches. Also, the word wrapper is somewhat imprecise as in powerpc
>> land bootmem sort of runs on top of memblock
> My point was just that the changelog says "This gets rid of wrapper
> code" when it actually removes no source code, and adds configuration
The patch gets rid of the wrapper code, bootmem itself and arch specific
initialization, from the build/kernel image. I guess I'll re-word that
so it doesn't sound so literal
>> When NO_BOOTMEM is configured the mm/nobootmem.c is used that is the
>> bootmem API actually re-implemented with memblock. The bootmem API is
>> used in various places in the arch independent code
>> This patch wants to isolate for removal the bootmem initialization code
>> for powerpc and to exclude mm/bootmem.c from being built. This being the
>> first step I didn't want to actually remove the code, so it will be easy
>> to debug if some issues crop up. Also, people that want the use the
>> bootmem code for some reason can easily do that. Once this change spends
>> some time in the tree, we can actually remove the bootmem initialization
> Is there a plausible reason someone would "want to use the bootmem
Don't know, but as before it wasn't even possible to make a build with
NO_BOOTMEM I decided to err on the side of caution
> While the "ifdef it for a while" approach is sometimes sensible, usually
> it's better to just make the change rather than ifdef it.
I felt it was sensible in this case
> Consider what
> the code would look like if there were ifdefs for a ton of random
> changes, half of which nobody ever bothered to go back and clean up
> after the change got widespread testing.
Well, this is not a random change, but I certainly don't disagree with
the principle of what you're saying
> Why is this patch risky enough to warrant such an approach?
I don't think it's risky and we've been using it for months on various
SoC(s)/boards. Just didn't want to be very abrupt in removing the option
of using bootmem
> Shouldn't boot-time issues be pretty obvious?
Gross errors should be obvious. But the devil is in the details, and
even tough I've debugged/compared the memory layout/allocations with
bootmem and memblock only, I'm prepared to admit I might have missed
something (especially in the first patch of the sequence)
More information about the Linuxppc-dev