lkml.org 
[lkml]   [2011]   [May]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v2 3/3] sched: increase SCHED_LOAD_SCALE resolution
    On Wed, May 18, 2011 at 11:25 AM, Ingo Molnar <mingo@elte.hu> wrote:
    >
    > * Nikhil Rao <ncrao@google.com> wrote:
    >
    >> -#define SCHED_LOAD_SHIFT     10
    >> +#if BITS_PER_LONG > 32
    >> +#define SCHED_LOAD_RESOLUTION        10
    >> +#define scale_load(w)                (w << SCHED_LOAD_RESOLUTION)
    >> +#define scale_load_down(w)   (w >> SCHED_LOAD_RESOLUTION)
    >> +#else
    >> +#define SCHED_LOAD_RESOLUTION        0
    >> +#define scale_load(w)                w
    >> +#define scale_load_down(w)   w
    >> +#endif
    >
    > Please use the:
    >
    > #if X
    > # define Y A
    > #else
    > # define Y B
    > #endif
    >
    > type of nested CPP branching style.
    >
    > Also, the 'w' probably wants to be '(w)', just in case.

    Thanks for the review. Will fix the macros.

    >
    >>       if (!lw->inv_weight) {
    >> -             if (BITS_PER_LONG > 32 && unlikely(lw->weight >= WMULT_CONST))
    >> +             unsigned long w = scale_load_down(lw->weight);
    >> +             if (BITS_PER_LONG > 32 && unlikely(w >= WMULT_CONST))
    >>                       lw->inv_weight = 1;
    >
    > Please separate local variable declarations and the first statement following
    > it by an extra empty line.
    >

    Yes, will do.

    > Could you also put all the performance measurement description into the
    > changelog of this third commit - so that people can see (and enjoy) the
    > extensive testing you have done on this topic?
    >

    Yes, will add the performance changes to the commit description.

    >>  /*
    >> - * Increase resolution of nice-level calculations:
    >> + * Increase resolution of nice-level calculations for 64-bit architectures.
    >>   */
    >> -#define SCHED_LOAD_SHIFT     10
    >> +#if BITS_PER_LONG > 32
    >> +#define SCHED_LOAD_RESOLUTION        10
    >> +#define scale_load(w)                (w << SCHED_LOAD_RESOLUTION)
    >> +#define scale_load_down(w)   (w >> SCHED_LOAD_RESOLUTION)
    >> +#else
    >> +#define SCHED_LOAD_RESOLUTION        0
    >> +#define scale_load(w)                w
    >> +#define scale_load_down(w)   w
    >> +#endif
    >
    > Please also be a bit more verbose in the comment above why we treat 64-bit
    > architectures differently - it's not obvious.

    Added a more descriptive comment.


    Thanks for the review. I'll refresh the patch and send it with the fixes.

    -Thanks,
    Nikhil
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2011-05-18 21:43    [W:0.027 / U:31.304 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site