Skip to contentSkip to conversation

cpufreq_(ondemand|conservative)

PATCH In review Alexander Clouter 2 participants 4 messages Jan 23, 2005 – Jan 29, 2005 CPU frequency scaling framework

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 cpufreq_(ondemand|conservative)

Hi All,

Well after Dominik reminded me about the updates to cpufreq_ondemand I had made some time back and also my cpufreq governor called cpufreq_conservative, I dug out my coffee and started fixing them up for another round of peer review. My governor is pretty much a minor rewrite of the cpufreq_ondemand governor but instead gracefully increases and decreases the cpufreq which should make it much more suitable for battery power environments.

All the patches are for 2.6.11-rc1-mm1 but also apply cleanly to 2.6.11-rc1-mm2. Unfortunately 2.6.11-rc1-mm2 oops'es in key_scancode (drivers/char/keyboard.c) when I presses any keys on my text console (tty1) after init has loaded so I am waiting till 2.6.11-rc2-mm1 appears before I file a bug report.

The 'improvements' I have made to cpufreq_ondemand are:

1) cpufreq_ondemand-2.6.11-rc1-mm1-01_ignore-nice.diff
this adds a new parameter in /sys called /sys/.../cpufreq/ondemand/ignore_nice which by default is set to '0'. Any 'nice' tasks running will be considered as idle time unless you set the value in ignore_nice to '1', then it will be simply considered as a regular cpu time sucking program.

Last time I did this Venki mentioned some possible corner case conditions[1] and so this time I make it recalculate the idle times when the value of ignore_nice is flipped. If I am right this should fix any possible issues that would have arisen from this...?

2) cpufreq_ondemand-2.6.11-rc1-mm1-02_check-rate-and-break-out.diff a very simple patch which prevents us from changing the cpufreq from 'x' to 'x' un-necessarily. No-one could find any problems with this so it has pretty much remained untouched.

3) cpufreq_ondemand-2.6.11-rc1-mm1-03_sys_freq_step.diff
this feature also adds a new parameter in /sys called /sys/.../cpufreq/ondemand/freq_step which by default is set to '5'. You can change this to any value between '0' (why?) and '100' to alter how much the cpu will change its frequency by on the way down.

4) cpufreq_ondemand-2.6.11-rc1-mm1-04_ignore_steal.diff
I noticed a new cpustat has appeared called 'steal'[2] which from what I can tell should be treated like an iowait stat. 'steal' only seems supported by S/390 but I think it should be 'considered'. This is a minor patch and if I have gotten confused then obviously it should be removed (and from cpufreq_conservative)

5) cpufreq_ondemand-2.6.11-rc1-mm1-05_safe_down_skip.diff although I have not noticed any problems without it being done a little alarm bell fires off in my head about how down_skip really should be initialised (what if the cpu-freq is not at a minimum when we start off or ac power is unplugged and we get policy->min changing to a lower value?). Again a minor patch, if not worth it obviously it can be removed and should also be from cpufreq_conservative

Now cpufreq_conservative started off as a copy of cpufreq_ondemand with all the above patches and then amended from there. If you install the patches you can see with a diff (attached for _show_ and not use) that there is not much in the way of difference between them. It works by me creating and initialising an array to each cpu's policy->cur (this could should be nice in an SMP environment, bug reports would be appreciated) and then changing the contents by freq_step each time we need to increase or decrease the cpufreq. This results in a smoother transition on the way up and down. Also by the nature of this governor it polls 100 times fewer than cpufreq_ondemand.

Let me know what you think, the patches work for me, the question is do they work for you :)

Cheers all

Alex

[1] http://marc.theaimsgroup.com/?l=linux-kernel&m=110013659005496&w=2
[2] http://marc.theaimsgroup.com/?l=linux-kernel&m=109888788719020&w=2

Signature · 11 lines
 ________________________________________ 
/ A foolish consistency is the hobgoblin \
| of little minds.                       |
|                                        |
\ -- Ralph Waldo Emerson                 /
 ---------------------------------------- 
        \   ^__^
         \  (oo)\_______
            (__)\       )\/\
                ||----w |
                ||     ||
drivers/cpufreq/cpufreq_ondemand.c +62 -4
@@ -79,6 +79,7 @@
 	unsigned int		sampling_down_factor;
 	unsigned int		up_threshold;
 	unsigned int		down_threshold;
+	unsigned int		ignore_nice;
 };
 
 static struct dbs_tuners dbs_tuners_ins = {
@@ -116,6 +117,7 @@
 show_one(sampling_down_factor, sampling_down_factor);
 show_one(up_threshold, up_threshold);
 show_one(down_threshold, down_threshold);
+show_one(ignore_nice, ignore_nice);
 
 static ssize_t store_sampling_down_factor(struct cpufreq_policy *unused, 
 		const char *buf, size_t count)
@@ -194,6 +196,47 @@
 	return count;
 }
 
+static ssize_t store_ignore_nice(struct cpufreq_policy *policy,
+		const char *buf, size_t count)
+{
+	unsigned int input;
+	int ret;
+
+	unsigned int j;
+	
+	ret = sscanf (buf, "%u", &input);
+	if ( ret != 1 )
+		return -EINVAL;
+
+	if ( input > 1 )
+		input = 1;
+	
+	down(&dbs_sem);
+	if ( input == dbs_tuners_ins.ignore_nice ) { /* nothing to do */
+		up(&dbs_sem);
+		return count;
+	}
+	dbs_tuners_ins.ignore_nice = input;
+
+	/* we need to re-evaluate prev_cpu_idle_up and prev_cpu_idle_down */
+	for_each_cpu_mask(j, policy->cpus) {
+		struct cpu_dbs_info_s *j_dbs_info;
+		j_dbs_info = &per_cpu(cpu_dbs_info, j);
+		j_dbs_info->cur_policy = policy;
+
+		j_dbs_info->prev_cpu_idle_up =
+			kstat_cpu(j).cpustat.idle +
+			kstat_cpu(j).cpustat.iowait;
+		  if (dbs_tuners_ins.ignore_nice == 0)
+			j_dbs_info->prev_cpu_idle_up +=
+				kstat_cpu(j).cpustat.nice;
+		j_dbs_info->prev_cpu_idle_down = j_dbs_info->prev_cpu_idle_up;
+	}
+	up(&dbs_sem);
+
+	return count;
+}
+
 #define define_one_rw(_name) \
 static struct freq_attr _name = \
 __ATTR(_name, 0644, show_##_name, store_##_name)
@@ -202,6 +245,7 @@
 define_one_rw(sampling_down_factor);
 define_one_rw(up_threshold);
 define_one_rw(down_threshold);
+define_one_rw(ignore_nice);
 
 static struct attribute * dbs_attributes[] = {
 	&sampling_rate_max.attr,
@@ -210,6 +254,7 @@
 	&sampling_down_factor.attr,
 	&up_threshold.attr,
 	&down_threshold.attr,
+	&ignore_nice.attr,
 	NULL
 };
 
@@ -254,6 +299,9 @@
 	/* Check for frequency increase */
 	total_idle_ticks = kstat_cpu(cpu).cpustat.idle +
 		kstat_cpu(cpu).cpustat.iowait;
+	  /* consider 'nice' tasks as 'idle' time too if required */
+	  if (dbs_tuners_ins.ignore_nice == 0)
+		total_idle_ticks += kstat_cpu(cpu).cpustat.nice;
 	idle_ticks = total_idle_ticks -
 		this_dbs_info->prev_cpu_idle_up;
 	this_dbs_info->prev_cpu_idle_up = total_idle_ticks;
@@ -270,6 +318,9 @@
 		/* Check for frequency increase */
 		total_idle_ticks = kstat_cpu(j).cpustat.idle +
 			kstat_cpu(j).cpustat.iowait;
+		  /* consider 'nice' too? */
+		  if (dbs_tuners_ins.ignore_nice == 0)
+			   total_idle_ticks += kstat_cpu(j).cpustat.nice;
 		tmp_idle_ticks = total_idle_ticks -
 			j_dbs_info->prev_cpu_idle_up;
 		j_dbs_info->prev_cpu_idle_up = total_idle_ticks;
@@ -298,6 +349,9 @@
 
 	total_idle_ticks = kstat_cpu(cpu).cpustat.idle +
 		kstat_cpu(cpu).cpustat.iowait;
+	  /* consider 'nice' too? */
+	  if (dbs_tuners_ins.ignore_nice == 0)
+		  total_idle_ticks += kstat_cpu(cpu).cpustat.nice;
 	idle_ticks = total_idle_ticks -
 		this_dbs_info->prev_cpu_idle_down;
 	this_dbs_info->prev_cpu_idle_down = total_idle_ticks;
@@ -313,6 +367,9 @@
 		/* Check for frequency increase */
 		total_idle_ticks = kstat_cpu(j).cpustat.idle +
 			kstat_cpu(j).cpustat.iowait;
+		  /* consider 'nice' too? */
+		  if (dbs_tuners_ins.ignore_nice == 0)
+			total_idle_ticks += kstat_cpu(j).cpustat.nice;
 		tmp_idle_ticks = total_idle_ticks -
 			j_dbs_info->prev_cpu_idle_down;
 		j_dbs_info->prev_cpu_idle_down = total_idle_ticks;
@@ -398,10 +455,10 @@
 		
 			j_dbs_info->prev_cpu_idle_up = 
 				kstat_cpu(j).cpustat.idle +
-				kstat_cpu(j).cpustat.iowait;
-			j_dbs_info->prev_cpu_idle_down = 
-				kstat_cpu(j).cpustat.idle +
-				kstat_cpu(j).cpustat.iowait;
+				kstat_cpu(j).cpustat.iowait +
+				kstat_cpu(j).cpustat.nice;
+			j_dbs_info->prev_cpu_idle_down
+				= j_dbs_info->prev_cpu_idle_up;
 		}
 		this_dbs_info->enable = 1;
 		sysfs_create_group(&policy->kobj, &dbs_attr_group);
@@ -421,6 +478,7 @@
 			def_sampling_rate = (latency / 1000) *
 					DEF_SAMPLING_RATE_LATENCY_MULTIPLIER;
 			dbs_tuners_ins.sampling_rate = def_sampling_rate;
+			dbs_tuners_ins.ignore_nice = 0;
 
 			dbs_timer_init();
 		}
drivers/cpufreq/cpufreq_ondemand.c +8 -0
@@ -335,6 +335,10 @@
 			sampling_rate_in_HZ(dbs_tuners_ins.sampling_rate);
 
 	if (idle_ticks < up_idle_ticks) {
+		/* if we are already at full speed then break out early */
+		if (policy->cur == policy->max)
+			return;
+		
 		__cpufreq_driver_target(policy, policy->max, 
 			CPUFREQ_RELATION_H);
 		down_skip[cpu] = 0;
@@ -388,6 +392,10 @@
 			sampling_rate_in_HZ(freq_down_sampling_rate);
 
 	if (idle_ticks > down_idle_ticks ) {
+		/* if we are already at the lowest speed then break out early */
+		if (policy->cur == policy->min)
+			return;
+		
 		freq_down_step = (5 * policy->max) / 100;
 
 		/* max freq cannot be less than 100. But who knows.... */
drivers/cpufreq/cpufreq_ondemand.c +36 -6

97 lines not on this page · raw message

drivers/cpufreq/cpufreq_ondemand.c +8 -5
@@ -325,7 +325,7 @@
 
 	/* Check for frequency increase */
 	total_idle_ticks = kstat_cpu(cpu).cpustat.idle +
-		kstat_cpu(cpu).cpustat.iowait;
+		kstat_cpu(cpu).cpustat.iowait + kstat_cpu(cpu).cpustat.steal;
 	  /* consider 'nice' tasks as 'idle' time too if required */
 	  if (dbs_tuners_ins.ignore_nice == 0)
 		total_idle_ticks += kstat_cpu(cpu).cpustat.nice;
@@ -344,7 +344,8 @@
 		j_dbs_info = &per_cpu(cpu_dbs_info, j);
 		/* Check for frequency increase */
 		total_idle_ticks = kstat_cpu(j).cpustat.idle +
-			kstat_cpu(j).cpustat.iowait;
+			kstat_cpu(j).cpustat.iowait +
+			kstat_cpu(j).cpustat.steal;
 		  /* consider 'nice' too? */
 		  if (dbs_tuners_ins.ignore_nice == 0)
 			   total_idle_ticks += kstat_cpu(j).cpustat.nice;
@@ -379,7 +380,7 @@
 		return;
 
 	total_idle_ticks = kstat_cpu(cpu).cpustat.idle +
-		kstat_cpu(cpu).cpustat.iowait;
+		kstat_cpu(cpu).cpustat.iowait + kstat_cpu(cpu).cpustat.steal;
 	  /* consider 'nice' too? */
 	  if (dbs_tuners_ins.ignore_nice == 0)
 		  total_idle_ticks += kstat_cpu(cpu).cpustat.nice;
@@ -397,7 +398,8 @@
 		j_dbs_info = &per_cpu(cpu_dbs_info, j);
 		/* Check for frequency increase */
 		total_idle_ticks = kstat_cpu(j).cpustat.idle +
-			kstat_cpu(j).cpustat.iowait;
+			kstat_cpu(j).cpustat.iowait +
+			kstat_cpu(j).cpustat.steal;
 		  /* consider 'nice' too? */
 		  if (dbs_tuners_ins.ignore_nice == 0)
 			total_idle_ticks += kstat_cpu(j).cpustat.nice;
@@ -493,7 +495,8 @@
 			j_dbs_info->prev_cpu_idle_up = 
 				kstat_cpu(j).cpustat.idle +
 				kstat_cpu(j).cpustat.iowait +
-				kstat_cpu(j).cpustat.nice;
+				kstat_cpu(j).cpustat.nice +
+				kstat_cpu(j).cpustat.steal;
 			j_dbs_info->prev_cpu_idle_down
 				= j_dbs_info->prev_cpu_idle_up;
 		}
drivers/cpufreq/cpufreq_ondemand.c +9 -0
@@ -299,6 +299,7 @@
 	unsigned int freq_down_step;
 	unsigned int freq_down_sampling_rate;
 	static int down_skip[NR_CPUS];
+	static unsigned short init_flag = 0;
 	struct cpu_dbs_info_s *this_dbs_info;
 
 	struct cpufreq_policy *policy;
@@ -309,6 +310,14 @@
 		return;
 
 	policy = this_dbs_info->cur_policy;
+
+	if ( init_flag == 0 ) {
+		for ( /* NULL */; init_flag < NR_CPUS; init_flag++ ) {
+			down_skip[cpu] = 0;
+		}
+		init_flag = 1;
+	}
+
 	/* 
 	 * The default safe range is 20% to 80% 
 	 * Every sampling_rate, we check
drivers/cpufreq/Kconfig +20 -0
diff -u -U 2 -r -N -d linux-2.6.11-rc1-mm1.orig/drivers/cpufreq/Kconfig linux-2.6.11-rc1-mm1/drivers/cpufreq/Kconfig
@@ -116,3 +116,23 @@
 	  For details, take a look at linux/Documentation/cpu-freq.
 
 	  If in doubt, say N.
+
+config CPU_FREQ_GOV_CONSERVATIVE
+	tristate "'conservative' cpufreq governor"
+	depends on CPU_FREQ
+	help
+	  'conservative' - this driver is rather similar to the 'ondemand'
+	  governor both in its source code and its purpose, the difference is
+	  its optimisation for better suitability in a battery powered
+	  environment.  The frequency is gracefully increased and decreased
+	  rather than jumping to 100% when speed is required.
+
+	  If you have a desktop machine then you should really be considering
+	  the 'ondemand' governor instead, however if you are using a laptop,
+	  PDA or even an AMD64 based computer (due to the unacceptable
+	  step-by-step latency issues between the minimum and maximum frequency 
+	  transitions in the CPU) you will probably want to use this governor.
+
+	  For details, take a look at linux/Documentation/cpu-freq.
+
+	  If in doubt, say N.
drivers/cpufreq/Makefile +1 -0
diff -u -U 2 -r -N -d linux-2.6.11-rc1-mm1.orig/drivers/cpufreq/Makefile linux-2.6.11-rc1-mm1/drivers/cpufreq/Makefile
@@ -8,6 +8,7 @@
 obj-$(CONFIG_CPU_FREQ_GOV_POWERSAVE)	+= cpufreq_powersave.o
 obj-$(CONFIG_CPU_FREQ_GOV_USERSPACE)	+= cpufreq_userspace.o
 obj-$(CONFIG_CPU_FREQ_GOV_ONDEMAND)	+= cpufreq_ondemand.o
+obj-$(CONFIG_CPU_FREQ_GOV_CONSERVATIVE)	+= cpufreq_conservative.o
 
 # CPUfreq cross-arch helpers
 obj-$(CONFIG_CPU_FREQ_TABLE)		+= freq_table.o
drivers/cpufreq/cpufreq_conservative.c +620 -0

620 lines not on this page · raw message

drivers/cpufreq/cpufreq_conservative.c renamed from drivers/cpufreq/cpufreq_ondemand.c +40 -18

136 lines not on this page · raw message

_______________________________________________
Cpufreq mailing list
[email protected]
http://lists.linux.org.uk/mailman/listinfo/cpufreq

#1 lore raw

concerning patch #1:

drivers/cpufreq/cpufreq_ondemand.c line 459

Quoted 10 lines · Alexander Clouter
drivers/cpufreq/cpufreq_ondemand.c +2 -4
@@ -398,10 +455,10 @@
 
 			j_dbs_info->prev_cpu_idle_up = 
 				kstat_cpu(j).cpustat.idle +
-				kstat_cpu(j).cpustat.iowait;
-			j_dbs_info->prev_cpu_idle_down = 
-				kstat_cpu(j).cpustat.idle +
-				kstat_cpu(j).cpustat.iowait;
+				kstat_cpu(j).cpustat.iowait +
+				kstat_cpu(j).cpustat.nice;

Why are you adding cpustat.nice here unconditionally? You don't do so in the sysfs write method, so I assume you should make this dependant on ignore_nice here as well.

Dominik

#2 lore raw

Dominik,

I was starting to wonder if anyone received my patches as so far _zero_ feekback :)

Quoted 5 lines · Dominik Brodowski

concerning patch #1:

Quoted 2 lines · Alexander Clouter
(quoted diff) +0 -0
@@ -398,10 +455,10 @@
 
Quoted 13 lines

drivers/cpufreq/cpufreq_ondemand.c line 459

Quoted 8 lines
drivers/cpufreq/cpufreq_ondemand.c +2 -4
 			j_dbs_info->prev_cpu_idle_up = 
 				kstat_cpu(j).cpustat.idle +
-				kstat_cpu(j).cpustat.iowait;
-			j_dbs_info->prev_cpu_idle_down = 
-				kstat_cpu(j).cpustat.idle +
-				kstat_cpu(j).cpustat.iowait;
+				kstat_cpu(j).cpustat.iowait +
+				kstat_cpu(j).cpustat.nice;

Why are you adding cpustat.nice here unconditionally? You don't do so in the sysfs write method, so I assume you should make this dependant on ignore_nice here as well.

Well by default 'ignore_nice = 0' so is there a need? Look in the next hunk.

I probably have missed something, I am only a sysadmin/perl monkey with a lpatop afterall :)

Cheers

Alex

Signature · 11 lines
 _________________________________________ 
/ It is wise to keep in mind that neither \
| success nor failure is ever final.      |
|                                         |
\ -- Roger Babson                         /
 ----------------------------------------- 
        \   ^__^
         \  (oo)\_______
            (__)\       )\/\
                ||----w |
                ||     ||

_______________________________________________
Cpufreq mailing list
[email protected]
http://lists.linux.org.uk/mailman/listinfo/cpufreq

#3 lore raw
In reply to Alexander Clouter

Hi,

Quoted 1 line · Alexander Clouter

1) cpufreq_ondemand-2.6.11-rc1-mm1-01_ignore-nice.diff

Please see other mail

Quoted 6 lines

4) cpufreq_ondemand-2.6.11-rc1-mm1-04_ignore_steal.diff
I noticed a new cpustat has appeared called 'steal'[2] which from what I can tell should be treated like an iowait stat. 'steal' only seems supported by S/390 but I think it should be 'considered'. This is a minor patch and if I have gotten confused then obviously it should be removed (and from cpufreq_conservative)

As there's no cpufreq support for S/390 (yet?), I don't see a need for applying this.

Other patches look fine. If Venki agrees: could you re-send them (1 updated, 2,3,5 and conservative) to the cpufreq list and Dave Jones <[email protected]> (one patch per e-mail, Signed-off-by: line included)? IMHO they're ready for "upstream".

Many thanks,
Dominik