Mike Galbraith <[email protected]> wrote:
>
> > Does this have to be a macro?
> >
>
> I suppose not, now inlined.
>
It would be nice to uninline the function and then to modify it in a
followup patch. That way, we get to see what changed, which is one of the
reasons to not use megamacros (sorry).
> +static inline int expired_starving(runqueue_t *rq)
> +{
> + int limit = STARVATION_LIMIT * rq->nr_running, starving;
> +
> + if (!limit || !rq->expired_timestamp)
> + return 0;
> + starving = jiffies - rq->expired_timestamp >= limit;
> + starving += rq->curr->static_prio > rq->best_expired_prio;
> +
> + return starving;
> +}
ick. Is that really what that macros does??
The function returns a boolean, so we should short-circuit the evaluation
where possible.
static inline int expired_starving(runqueue_t *rq)
{
int limit;
/* Comment goes here */
if (!rq->expired_timestamp)
return 0;
limit = STARVATION_LIMIT * rq->nr_running;
/* Here too */
if (!limit)
return 0;
/* And here */
if (jiffies - rq->expired_timestamp >= limit)
return 1;
/* And here */
if (rq->curr->static_prio > rq->best_expired_prio)
return 1;
/* And here */
return 0;
}
This way
a) We get somewhere to put comments describing each step of the logic.
b) We get to select the order of the comparisons in decreasing
(probability*expensiveness) order.
See how you're performing an unneeded multiplication if
!rq->expired_timestamp?
c) See how the first test of `limit' comes after that multiplication?
STARVATION_LIMIT is a constant (isn't it?) If so, we need only test
rq->nr_running.
d) The next guy who comes along has to update the comments ;)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
