Index Home About Blog
Date: 	Wed, 8 Aug 2001 09:40:07 -0700 (PDT)
From: Linus Torvalds <torvalds@transmeta.com>
Subject: Re: [RFC][PATCH] Scalable Scheduling
Newsgroups: fa.linux.kernel

On Wed, 8 Aug 2001, Mike Kravetz wrote:
>
> I have been working on scheduler scalability.  Specifically,
> the concern is running Linux on bigger machines (higher CPU
> count, SMP only for now).

Note that there is no way I will ever apply this particular patch for a
very simple reason: #ifdef's in code.

Why do you have things like

	#ifdef CONFIG_SMP
		.. use nr_running() ..
	#else
		.. use nr_running ..
	#endif

and

	#ifdef CONFIG_SMP
	       list_add(&p->run_list, &runqueue(task_to_runqueue(p)));
	#else
	       list_add(&p->run_list, &runqueue_head);
	#endif

when it just shows that you did NOT properly abstract your thinking to
realize that the non-SMP case should be the same as the SMP case with 1
CPU (+ optimization).

I find code like the above physically disgusting.

What's wrong with using

	nr_running()

unconditionally, and make sure that it degrades gracefully to just the
single-CPU case?

What's wrong with just using

	runqueue(task_to_runqueue(p))

and having the UP case realize that the "runqueue()" macro is a fixed
entry?

Same thing applies to that runqueue_lock stuff. That is some of the
ugliest code I've seen in a long time. Please use inline functions, sane
defines that work both ways, and take advantage of the fact that gcc will
optimize constant loops and numbers (it's ok to reference arrays in UP
with "array[smp_processor_id()]", and it's ok to have loops that look like
"for (i = 0; i < NR_CPUS; i++)" that will do the right thing on UP _and_
SMP.

And make your #ifdef's be _outside_ the code.

I hate code that has #ifdef's. It's a major design mistake, and shows
that the person who coded it didn't think of it as _one_ problem, but as
two.

So please spend some time cleaning it up, I can't look at it like this.

		Linus



Date: 	Wed, 8 Aug 2001 12:14:32 -0700 (PDT)
From: Linus Torvalds <torvalds@transmeta.com>
Subject: Re: [RFC][PATCH] Scalable Scheduling
Newsgroups: fa.linux.kernel

On Wed, 8 Aug 2001, Daniel Phillips wrote:
>
> write:
>
> 	inline int nr_running(void)
> 	{
> 	#ifdef CONFIG_SMP
> 		int i = 0, tot=nt_running(REALTIME_RQ);
> 		while (i < smp_num_cpus) {
> 			tot += nt_running(cpu_logical_map(i++));
> 		}
> 		return(tot);
> 	#else
> 		return nr_running;
> 	#endif
> 	}


Even more preferably, just have (in a header file)

	#ifdef CONFIG_SMP

	inline int nr_running(void)
	{
		...
	}

	.. other SMP cases ..

	#else

	#define nr_running() (__nr_running)

	.. other UP cases ..

	#endif

if you just cannot make an efficient function that just works for both.

No, we don't adhere to this everywhere. But we should (and largely _do_)
try to.

Having the #ifdef's outside the code tends to have two advantages:

 - it makes the code much more readable, and doesn't split things up.

 - you have to choose your abstraction interfaces more carefully, which in
   turn tends to make for better code.

Abstraction is nice - _especially_ when you have a compiler that sees
through the abstraction and can generate code as if it wasn't there.

		Linus



Index Home About Blog