On Thu, Sep 16, 2010 at 1:39 PM, David C Niemi <dniemi@xxxxxxxxxxxx> wrote:
> I've been doing more testing, and have a couple of observations. I'm
> attaching a minimal form of my changes as a patch for the latest 2.6.pre36
> git version of the driver. However, it is difficult for me to test under
> anything other than 2.6.32 (RHEL 6 beta 2), and there are some minor
> differences, though I don't believe they are relevant to my results.
>
> It looks like "io_is_busy" set to 1 is quite beneficial for quickly reacting
> the onset of load.
>
> I do see a lot of downshifting from the top speed when a core is at "100%"
> CPU, presumably this means little stalls and lulls, so I expect
> "sampling_down_factor" values greater than 1 continue to be useful and the
> sampling_down_factor continues to be desirable.
>
> I've testing on a dual Xeon X5680 system (other times I've been testing on
> 2-year-old dual Opterons).
>
> I observe about a 10W power consumption reduction at idle between the
> "performance" governor and the "ondemand" governor. I've seen even bigger
> differences under load, as much as 40 watts, though that could be associated
> with some performance differences. I haven't tried to quantify the effect
> of the sampling_down_factor tunable on power consumption under load,
> presumably it increases it, but its usage is voluntary and that is to be
> expected.
>
> I have been unable to find a value of up_threshold that does not switch
> frequency on at least one core pretty frequently (ranging a couple of times
> a minute to several times a second). However, with fairly fast sampling
> intervals (10000 to 50000) I see pretty quick reaction to load even with
> UP_THRESHOLD set high (e.g. 50 or even 95). So it is likely my previous
> efforts to extend the possible values of UP_THRESHOLD from 11 to 5 are no
> longer necessary, and are not included in the attached patch. There are
> other things I would like to consider doing, however, that I'll bring up
> afterwards, but not in this minimal patch.
>
I do see this change in the patch. From the comment above, you did not
want that change?
#define MICRO_FREQUENCY_UP_THRESHOLD (95)
#define MICRO_FREQUENCY_MIN_SAMPLE_RATE (10000)
-#define MIN_FREQUENCY_UP_THRESHOLD (11)
+#define MIN_FREQUENCY_UP_THRESHOLD (5)
#define MAX_FREQUENCY_UP_THRESHOLD (100)
The problem with 5 is that when DEF_FREQUENCY_DOWN_DIFFERENTIAL is
used, up_threshold - down_differential calculation in the code can end
up being negative, which is not good.
One minor comment.
+
+ mutex_lock(&dbs_mutex);
+ if (ret != 1 || input > MAX_SAMPLING_DOWN_FACTOR || input < 1) {
+ mutex_unlock(&dbs_mutex);
+ return -EINVAL;
+ }
+
+ dbs_tuners_ins.sampling_down_factor = input;
You can move mutex_lock after input validation to make it a bit more clean.
Otherwise patch looks good. I agree that having sampling_down_factor
as a tunable will be useful under some situations.
Thanks,
Venki
--
To unsubscribe from this list: send the line "unsubscribe cpufreq" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
