Nr_variables!
Okay, henceforth decided to give the opening and closing mood a break!
Anyway, I am currently reading the Linux scheduler load balancing code. And there’s this one function called find_busiest_group() which looks something like this:
/*
* find_busiest_group finds and returns the busiest CPU group within the
* domain. It calculates and returns the amount of weighted load which
* should be moved to restore balance via the imbalance parameter.
*/
static struct sched_group *
find_busiest_group(struct sched_domain *sd, int this_cpu,
unsigned long *imbalance, enum cpu_idle_type idle,
int *sd_idle, cpumask_t *cpus, int *balance)
{
struct sched_group *busiest = NULL, *this = NULL, *group = sd->groups;
unsigned long max_load, avg_load, total_load, this_load, total_pwr;
unsigned long max_pull;
unsigned long busiest_load_per_task, busiest_nr_running;
unsigned long this_load_per_task, this_nr_running;
int load_idx, group_imb = 0;
#if defined(CONFIG_SCHED_MC) || defined(CONFIG_SCHED_SMT)
int power_savings_balance = 1;
unsigned long leader_nr_running = 0, min_load_per_task = 0;
unsigned long min_nr_running = ULONG_MAX;
struct sched_group *group_min = NULL, *group_leader = NULL;
#endif
max_load = this_load = total_load = total_pwr = 0;
busiest_load_per_task = busiest_nr_running = 0;
this_load_per_task = this_nr_running = 0;
if (idle == CPU_NOT_IDLE)
load_idx = sd->busy_idx;
else if (idle == CPU_NEWLY_IDLE)
load_idx = sd->newidle_idx;
else
load_idx = sd->idle_idx;
do {
unsigned long load, group_capacity, max_cpu_load, min_cpu_load;
int local_group;
int i;
int __group_imb = 0;
unsigned int balance_cpu = -1, first_idle_cpu = 0;
unsigned long sum_nr_running, sum_weighted_load;
Now by the time I reach this point, I am kinda confused asto what variable holds what value and represents what load exactly!
I guess, I need to increase the stack size of my mind to keep track of soo many local variables!
PS: I apologise for the formatting. The [sourcecode] macro of wordpress cannot do any better than this.
Sathya is planning to write a simple script to convert C code into color coded html instead.
ok, one -> i am unsubscribing from your blog. How could you put code of my dre*cough*nightmares*cough* here?!
Anyway, hint, use paper. draw diagrams. best way to understand the scheduler code.
BTW, if this is killing you, wait till you see Peter’s latest load balancing patches. Don’t really seem to work, but..
@theG
One of my immediate goals is to make this code more readable. Boy, it’s so confusing by the time you reach the end, you will be wondering, if you have the right load with you now!
And threats of unsubscription won’t work
what’s so unreadable about it?
Like I mentioned, there are way too many local variables. Any function, with so much data to process is trying to do too many things at the sametime. It can be designed better.
IMO, code readability is not just about right indentation or choosing suitable names or not using confusing constructs like the ternary operator. The main test should be this: when a new person is reading that code, how easy is it for him to review it. In this particular case, the number of state variables whose names are very similar to each other, can easily put off a new reader.
Instead of ranting, let me take a jab at cleaning up that piece of code
enscript does a neat job of formatting C code to HTML. Save satya the trouble
enscript -E –color -whtml -pblog.html snippet.c