Messages in this thread | | | Date | Wed, 18 May 2011 22:26:46 +0200 | From | Ingo Molnar <> | Subject | Re: [PATCH v2 3/3] sched: increase SCHED_LOAD_SCALE resolution |
| |
* Nikhil Rao <ncrao@google.com> wrote:
> +#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
We want (w) in the other definitions as well, to protect potential operators with lower precedence than <<. (Roughly half of the C operators are such so it's a real issue, should anyone use these macros with operators.)
> +#define SCHED_LOAD_SHIFT (10 + (SCHED_LOAD_RESOLUTION))
that () is not needed actually, if you look at the definition of SCHED_LOAD_RESOLUTION.
So you could move the superfluous () from here up to the two definitions above and thus no parentheses would be hurt during the making of this patch.
> + if (BITS_PER_LONG > 32 && unlikely(w >= WMULT_CONST)) > lw->inv_weight = 1; > + else if (unlikely(!w)) > + lw->inv_weight = WMULT_CONST; > else > + lw->inv_weight = WMULT_CONST / w;
Ok, i just noticed that you made use of BITS_PER_LONG here too.
It's better to put that into a helper define, something like SCHED_LOAD_HIGHRES, which could thus be used like this:
if (SCHED_LOAD_HIGHRES && unlikely(w >= WMULT_CONST))
then, should anyone want to tweak the condition for SCHED_LOAD_HIGHRES, it could be done in a single place. It would also self-document.
Thanks,
Ingo
| |