lkml.org 
[lkml]   [2006]   [Mar]   [18]   [last100]   RSS Feed
Views: [more markup]  [less markup]  [headers]  [forward] 
 
Messages in this thread
/
DateFri, 17 Mar 2006 22:22:03 -0800
FromAndrew Morton <>
SubjectRe: [2.6.16-rc6 patch] fix interactive task starvation
Mike Galbraith <efault@gmx.de> 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 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: 2006-03-18 06:28    [from the cache]
©2003-2008