[lkml]   [2012]   [Aug]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH 0/5] rbtree based interval tree as a prio_tree replacement
    On Thu, Aug 30, 2012 at 2:34 PM, Andrew Morton
    <> wrote:
    > On Tue, 7 Aug 2012 00:25:38 -0700
    > Michel Lespinasse <> wrote:
    >> This patchset goes over the rbtree changes that have been already integrated
    >> into Andrew's -mm tree, as well as the augmented rbtree proposal which is
    >> currently pending.
    > hm. Well I grabbed these for a bit of testing.
    > It's a large change in MM and it depends on code which hasn't yet been
    > merged in mainline. It's probably prudent to do all this in two steps
    > - we'll see.

    Makes sense to me. If we want to split the series as they get sent
    upstream, I would suggest sending all the rbtree and augmented rbtree
    infrastructure first, and then the rbtree usages (prio tree
    replacement, anon rmap interval tree which I'm going to send next, and
    rik's augmented rbtree based vma gap finder) in the next kernel.

    > The templates-with-CPP thing is not terribly appealing. It's not
    > obvious that it really needed to be done this way - we've avoided it in
    > plenty of other places. It would be nice to see that alternatives have
    > been thoroughly explored, and why they were rejected.

    I am actually wondering if the interval_tree_tmpl.h include file
    shouldn't be done as one large preprocessor #define instead. The
    ITSTRUCT, ITRB, etc... definitions would then become arguments to that
    large definition. It would also be possible to break up that #define
    into smaller ones - most likely, one for insertion, one for removal,
    and one for the subtree_search / iter_first / iter_next functions. Do
    you think this might help ?

    I don't really see other workable alternatives that don't involve code

    > The code uses the lame-and-useless "inline" absolutely all over the
    > place. I do think that for new code it would be better to get down and
    > actually make proper engineering decisions about which functions should
    > be inlined and mark them __always_inline.

    You mentionned this before, but I'm not convinced that __always_inline
    would be better. The kernel is full of 2-line functions that we really
    want inlined, and I don't see what the value would be in converting
    these all to __always_inline. I am tempted to stick with the current
    usage, which I understand as being:

    - use inline when the programmer believes a function should be inlined
    (this includes the static inline functions in header files - if the
    programmer didn't believe this should be inlined, he wouldn't put the
    function in a header file)

    - use __always_inline if the function absolutely needs to be inlined
    for correct operation (I believe scheduler has some of these, which
    need to be included in the parent in order to end up in the correct
    section), OR in rare cases if the compiler is known to generate bad
    code with a mere inline and the programmer wants to force the issue.

    I would also note that replacing inline with __always_inline is not a
    no-op change, even when the compiler was already inlining the original
    (marked inline) function. Sometimes the generated code ends up being
    different with __always_inline and I would rather not apply these
    changes blindly.

    > Hillf has made a review suggestion which AFAICT remains unresponded to.

    To be honest, I wasn't quite sure what he was suggesting ?

    Michel "Walken" Lespinasse
    A program is never fully debugged until the last user dies.

     \ /
      Last update: 2012-08-31 01:01    [W:0.028 / U:2.024 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site