Re: [RFC PATCH v2 0/7] Tunable sched_mc_power_savings=n

From: Suresh Siddha
Date: Mon Sep 08 2008 - 21:22:14 EST


On Mon, Sep 08, 2008 at 06:56:09AM -0700, Peter Zijlstra wrote:
> On Mon, 2008-09-08 at 19:18 +0530, Vaidyanathan Srinivasan wrote:
> > * Peter Zijlstra <a.p.zijlstra@xxxxxxxxx> [2008-09-08 15:25:46]:
> >
> > > May I again ask to first clean up the current power saving code before
> > > stacking more on top of it?
> >
> > :) I understand that you have asked for two things with respect to the
> > current power save balance code:
> >
> > 1. Detailed documentation
>
> Preferably in the form of in-code comments and code structure, this
> Documentation/* stuff always gets lost on me.

Peter, Almost every if() stmt/basic block in the power savings code has
comments around it. And also power-savings code is 50 lines (mostly comments)
in 320 lines of that function.

> But I also prefer to get rid of that power savings tweak in
> cpu_coregroup_map().

Why? Based on the power vs perf, we wanted to construct topologies
differently. Reason for the complexity is, in some of the Intel cpu's,
while the cores share the same package they have different last level caches.
So for performance, we want to differentiate based on last level caches
and for power, we want to consolidate based on the package information.

> But above all, readable code ;-)
>
> find_busiest_group() is the stuff of nightmares.

power-savings code is very small part of that nightmare :) That code
became complex over years with HT, smp-nice etc.

I haven't been following recent sched changes. I can take a look at it
and see what I can do to better organize find_busiest_group()

thanks,
suresh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/