lkml.org 
[lkml]   [2013]   [Jun]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [Part1 PATCH v5 00/22] x86, ACPI, numa: Parse numa info earlier
    Hello, Tang.

    On Thu, Jun 20, 2013 at 01:52:50PM +0800, Tang Chen wrote:
    > 1. It is difficult to tell which memory allocation is temporary and
    > which one is permanent when memblock is allocating memory. So, we
    > can only wait till boot is complete, and see which remains.
    > But, we have the second difficulty.
    >
    > 2. In memblock.reserve[], we cannot tell why we allocated this memory
    > just from the array item, right? So it is difficult to do the
    > relocation. If in the future, we have to allocate permanent memory
    > for other new purposes, we have to do the relocation again and again.
    > (Not sure if I understand the point correctly. I think there isn't
    > a generic way to relocate memory used for different purposes.)

    I was suggesting two separate things.

    * As memblock allocator can relocate itself. There's no point in
    avoiding setting NUMA node while parsing and registering NUMA
    topology. Just parse and register NUMA info and later tell it to
    relocate itself out of hot-pluggable node. A number of patches in
    the series is doing this dancing - carefully reordering NUMA
    probing. No need to do that. It's really fragile thing to do.

    * Once you get the above out of the way, I don't think there are a lot
    of permanent allocations in the way before NUMA is initialized.
    Re-order the remaining ones if that's cleaner to do. If that gets
    overly messy / fragile, copying them around or freeing and reloading
    afterwards could be an option too. There isn't much point in being
    super-efficient about ACPI override table. Being cleaner and more
    robust is far more important.

    As for distinguishing temporary / permanent, it shouldn't be difficult
    to make memblock track all allocations before NUMA info becomes online
    and then verify that those areas are free by the time boot is
    complete. Just mark the reserved areas allocated before NUMA info is
    fully available.

    > If you also had a look at the Part2 patches, you will see that I
    > introduced a flags member into memblock to specify different types
    > of memory, which will help to recognize hotpluggable memory. My
    > thinking is that ensure memblock will not allocate hotpluggable
    > memory. I think this is the most safe and easy way to satisfy hotplug
    > requirement.

    And you can use exactly the same mechanism to track memory areas which
    were allocated before NUMA info was fully available, right?

    > So you don't agree to serialize the operations at boot time.

    No, I'm not disagreeing that some ordering is necessary. My point is
    that things seem to be going that way too far. Sure, some reordering
    is necessary but it doesn't have to be this fragile. Careful
    reordering isn't the only way to achieve it.

    > About this patch-set from Yinghai, actually he is doing a job that I
    > failed to do. And he also included a lot of other things in the
    > patch-set, such as extend max number of overridable acpi tables, local
    > node pagetable, and so on.

    Doing multiple things to achieve a goal in a patchset might not be
    optimal but is usually okay if properly explained. What's not okay is
    not explaining the overall goal, approach and design in the head
    message, poor quality of patch description and code documentation.

    This part of code is almost inherently fragile and difficult to debug
    and patchset like this would degrade the maintainability and I really
    don't want to spend hours trying to decipher what the overall approach
    is by trying to navigate maze of poorly documented patches only to
    find out that some of the basic approaches are not very agreeable. We
    could have had this exact discussion way earlier if the head message
    properly described what was going on and the review process would have
    been much more pleasant for all involved parties.

    I don't think it matters whose patches go in how as long as they are
    attributed correctly. The end result - what goes in the git tree as
    log and code changes - matters, and it needs to be whole lot better.

    Thanks.

    --
    tejun


    \
     
     \ /
      Last update: 2013-06-20 08:41    [W:6.545 / U:0.156 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site