Re: [PATCH] - Reduce overhead of calc_load

[Andrew Morton <akpm@osdl.org>]

Jack Steiner <steiner@sgi.com> wrote:
>
> +unsigned long nr_active(void)
> +{
> +	unsigned long i, running = 0, uninterruptible = 0;
> +
> +	for_each_online_cpu(i) {
> +		running += cpu_rq(i)->nr_running;
> +		uninterruptible += cpu_rq(i)->nr_uninterruptible;
> +	}
> +
> +	if (unlikely((long)uninterruptible < 0))
> +		uninterruptible = 0;
> +
> +	return running + uninterruptible;
> +}

Is that check for (uninterruptible < 0) (copied from nr_uninterruptible)
really needed?  Can rq->nr_uninterruptible actually go negative?

Perhaps nr_context_switches() and nr_iowait() should also go into this
function, then we rename it all to

	struct sched_stuff {
		unsigned nr_uninterruptible;
		unsigned nr_running;
		unsigned nr_active;
		unsigned long nr_context_switches;
	};

	void get_sched_stuff(struct sched_stuff *);

and then convert all those random little counter-upper-callers we have.

And then give get_sched_stuff() a hotplug handler (probably unneeded) and
then scratch our heads over why nr_uninterruptible() iterates across all
possible CPUs while this new nr_active() iterates over all online CPUs like
nr_running() and unlike nr_context_switches().


IOW: this code's an inefficient mess and needs some caring for.
-
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/