[PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff

[PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff

[Alexander Clouter]

[-- Attachment #1.1.1: Type: text/plain, Size: 367 bytes --]

Adds support to cpufreq_ondemand to ignore 'nice' cpu time

Signed-off-by: Alexander Clouter <alex-kernel@digriz.org.uk>

-- 
 ________________________________ 
< A couch is as good as a chair. >
 -------------------------------- 
        \   ^__^
         \  (oo)\_______
            (__)\       )\/\
                ||----w |
                ||     ||

[-- Attachment #1.1.2: cpufreq_ondemand-2.6.11-rc3-mm2-01_ignore-nice.diff --]
[-- Type: text/plain, Size: 4617 bytes --]

--- linux-2.6.11-rc3-mm2.orig/drivers/cpufreq/cpufreq_ondemand.c	2005-02-20 11:10:54.726034128 +0000
+++ linux-2.6.11-rc3-mm2/drivers/cpufreq/cpufreq_ondemand.c	2005-02-20 11:29:24.196369000 +0000
@@ -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,46 @@
 	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 +
+			( !dbs_tuners_ins.ignore_nice
+			  ? kstat_cpu(j).cpustat.nice : NULL );
+		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 +244,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 +253,7 @@
 	&sampling_down_factor.attr,
 	&up_threshold.attr,
 	&down_threshold.attr,
+	&ignore_nice.attr,
 	NULL
 };
 
@@ -254,6 +298,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 +317,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 +348,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 +366,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;
@@ -399,10 +455,11 @@
 		
 			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 +
+				( !dbs_tuners_ins.ignore_nice
+				  ? kstat_cpu(j).cpustat.nice : NULL );
+			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);
@@ -422,6 +479,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();
 		}

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 147 bytes --]

_______________________________________________
Cpufreq mailing list
Cpufreq@lists.linux.org.uk
http://lists.linux.org.uk/mailman/listinfo/cpufreq

Re: [PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff

[Arjan van de Ven]

On Sun, 2005-02-20 at 13:15 +0000, Alexander Clouter wrote:
> Adds support to cpufreq_ondemand to ignore 'nice' cpu time

this increasingly starts to smell like it wants this policy in
userspace.....

Re: [PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff

[Dominik Brodowski]

On Sun, Feb 20, 2005 at 03:12:44PM +0100, Arjan van de Ven wrote:
> On Sun, 2005-02-20 at 13:15 +0000, Alexander Clouter wrote:
> > Adds support to cpufreq_ondemand to ignore 'nice' cpu time
> 
> this increasingly starts to smell like it wants this policy in
> userspace.....

There are enough userspace cpufreq daemons already. However, the policy
decision back then was to prefer governing CPU frequencies in the following
order:
a) hardware
b) kernelspace
c) userspace

http://marc.theaimsgroup.com/?t=103053598900004&r=2&w=2

and two decisive posts:

http://marc.theaimsgroup.com/?l=linux-kernel&m=103056055008566&w=2
http://marc.theaimsgroup.com/?l=linux-kernel&m=103056413012458&w=2

Thanks,
	Dominik

Re: [PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff

[Dominik Brodowski]

On Sun, Feb 20, 2005 at 01:15:18PM +0000, Alexander Clouter wrote:
> +			( !dbs_tuners_ins.ignore_nice
> +			  ? kstat_cpu(j).cpustat.nice : NULL );
NULL is a pointer, 0 is a number. You most probably want "0" here.

> +				( !dbs_tuners_ins.ignore_nice
> +				  ? kstat_cpu(j).cpustat.nice : NULL );
dito.

	Dominik

Re: [PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff

[Alexander Clouter]

[-- Attachment #1.1: Type: text/plain, Size: 1616 bytes --]

Morning,

On Feb 20, Dominik Brodowski wrote:
> On Sun, Feb 20, 2005 at 01:15:18PM +0000, Alexander Clouter wrote:
> > +			( !dbs_tuners_ins.ignore_nice
> > +			  ? kstat_cpu(j).cpustat.nice : NULL );
> NULL is a pointer, 0 is a number. You most probably want "0" here.
> 
> > +				( !dbs_tuners_ins.ignore_nice
> > +				  ? kstat_cpu(j).cpustat.nice : NULL );
> dito.
> 
NULL comes out as a nothing, there is no problem in adding NULL to a
variable; the c-compiler, I have been assured, drops this out (or rather it
should) as a NOOP.  Of course this is what I have been told, by my flatmate
whom works with a large number of different compilers for different
architectures from different software houses, and lives/breathes C
(concerningly :)

Along similar lines, I was told that initialising statics to zero *must* be
done and not assumed; friends have told me its a 'gcc specific extension'.  
Last time I was told this was not necessary...

I'm happy for it to be changed on reasons of clarity, but technically there
should be no problems.  You're the kernel guys and I of course am wielding my
"talking out my ass" disclaimer, so correct me where necessary :)

Cheers

Alex

-- 
 _________________________________________ 
/  A horse breeder has his young colts    \
| bottle-fed after they're three days     |
| old. He heard that a foal and his mummy |
\ are soon parted.                        /
 ----------------------------------------- 
        \   ^__^
         \  (oo)\_______
            (__)\       )\/\
                ||----w |
                ||     ||

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 147 bytes --]

_______________________________________________
Cpufreq mailing list
Cpufreq@lists.linux.org.uk
http://lists.linux.org.uk/mailman/listinfo/cpufreq

Re: [PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff

[PCS]

Le Dimanche 20 Février 2005 19.14, Alexander Clouter a écrit :
(snip)
> Along similar lines, I was told that initialising statics to zero *must* be
> done and not assumed; friends have told me its a 'gcc specific extension'.
> Last time I was told this was not necessary...

I'm really not sure it is a "gcc specific extension", re-read paragraph 6.7.8 
from the ISO 9899:1999 norm, especially point 10. It explicitly say :

10 If an object that has automatic storage duration is not initialized
     explicitly, its value is indeterminate. If an object that has static
     storage duration is not initialized explicitly, then:
   - if it has pointer type, it is initialized to a null pointer;
   - if it has arithmetic type, it is initialized to (positive or unsigned
      zero;
   - if it is an aggregate, every member is initialized (recursively)
     according to these rules;
   - if it is a union, the first named member is initialized (recursively)
     according to these rules.

Regards,
Pcs

Re: [PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff

[Bruno Ducrot]

On Sun, Feb 20, 2005 at 06:14:48PM +0000, Alexander Clouter wrote:
> Morning,
> 
> On Feb 20, Dominik Brodowski wrote:
> > On Sun, Feb 20, 2005 at 01:15:18PM +0000, Alexander Clouter wrote:
> > > +			( !dbs_tuners_ins.ignore_nice
> > > +			  ? kstat_cpu(j).cpustat.nice : NULL );
> > NULL is a pointer, 0 is a number. You most probably want "0" here.
> > 
> > > +				( !dbs_tuners_ins.ignore_nice
> > > +				  ? kstat_cpu(j).cpustat.nice : NULL );
> > dito.
> > 
> NULL comes out as a nothing, there is no problem in adding NULL to a
> variable; the c-compiler, I have been assured, drops this out (or rather it
> should) as a NOOP.  Of course this is what I have been told, by my flatmate
> whom works with a large number of different compilers for different
> architectures from different software houses, and lives/breathes C
> (concerningly :)

Please don't assume anything about NULL in kernelspace.  It's
defined in a header (stddef.h for C99) and standard headers can't be
included in kernel space.  NULL is defined elsewhere for kernel, but that's
another story.

Anyway, you are mixing pointers and integers types.  These lead *always*
to bugs, especially if you must change to another HW architecture
(and sometimes another software environment like DOS).

> Along similar lines, I was told that initialising statics to zero *must* be
> done and not assumed; friends have told me its a 'gcc specific extension'.  
> Last time I was told this was not necessary...

I don't get it.  I assume statics variables being always initialized as said by
a previous answer even on pre-ansi C compilers.

Cheers,

-- 
Bruno Ducrot

--  Which is worse:  ignorance or apathy?
--  Don't know.  Don't care.

Re: [PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff

[Eric Piel]

Alexander Clouter a écrit :
> Adds support to cpufreq_ondemand to ignore 'nice' cpu time
> 
IMO, this policy should be has lightweight as possible. Why not making 
this feature just hard-coded? Don't count the load that nice'd tasks do, 
period!

It would make the patch 5 lines instead of 50. Every userspace deamon 
have this feature enabled by default, so this is a very reasonable 
trade-off.

Eric

Re: [PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff

[Eric Piel]

[-- Attachment #1: Type: text/plain, Size: 880 bytes --]

Eric Piel a écrit :
> Alexander Clouter a écrit :
> 
>> Adds support to cpufreq_ondemand to ignore 'nice' cpu time
>>
> IMO, this policy should be has lightweight as possible. Why not making 
> this feature just hard-coded? Don't count the load that nice'd tasks do, 
> period!
> 
> It would make the patch 5 lines instead of 50. Every userspace deamon 
> have this feature enabled by default, so this is a very reasonable 
> trade-off.
> 
Here is the patch, it does exactly the same functionality but it's 
completly static and therefore much smaller. Just tested on my computer, 
it does the trick.

Eric
--
Avoid the cpufreq ondemand governor to count the nice'd tasks as load 
for the cpu.

  cpufreq_ondemand.c |   17 ++++++++++-------
  1 files changed, 10 insertions(+), 7 deletions(-)

Signed-off-by: Eric Piel <eric.piel@tremplin-utc.net>
--

[-- Attachment #2: ondemand-ignore-nice-static.patch --]
[-- Type: text/x-patch, Size: 2197 bytes --]

--- linux-2.6.11-rc4/drivers/cpufreq/cpufreq_ondemand.c.bak	2005-02-06 23:35:41.000000000 +0100
+++ linux-2.6.11-rc4/drivers/cpufreq/cpufreq_ondemand.c	2005-02-21 11:10:26.000000000 +0100
@@ -253,7 +253,8 @@ static void dbs_check_cpu(int cpu)
 
 	/* 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.nice;
 	idle_ticks = total_idle_ticks -
 		this_dbs_info->prev_cpu_idle_up;
 	this_dbs_info->prev_cpu_idle_up = total_idle_ticks;
@@ -269,7 +270,8 @@ static void dbs_check_cpu(int cpu)
 		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.nice;
 		tmp_idle_ticks = total_idle_ticks -
 			j_dbs_info->prev_cpu_idle_up;
 		j_dbs_info->prev_cpu_idle_up = total_idle_ticks;
@@ -297,7 +299,8 @@ static void dbs_check_cpu(int cpu)
 		return;
 
 	total_idle_ticks = kstat_cpu(cpu).cpustat.idle +
-		kstat_cpu(cpu).cpustat.iowait;
+		kstat_cpu(cpu).cpustat.iowait +
+		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;
@@ -312,7 +315,8 @@ static void dbs_check_cpu(int cpu)
 		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.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,11 +402,10 @@ static int cpufreq_governor_dbs(struct c
 			j_dbs_info->cur_policy = policy;
 		
 			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;
 		}
 		this_dbs_info->enable = 1;
 		sysfs_create_group(&policy->kobj, &dbs_attr_group);

[-- Attachment #3: Type: text/plain, Size: 147 bytes --]

_______________________________________________
Cpufreq mailing list
Cpufreq@lists.linux.org.uk
http://lists.linux.org.uk/mailman/listinfo/cpufreq

Re: [PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff

[Alexander Clouter]

[-- Attachment #1.1: Type: text/plain, Size: 1313 bytes --]

On Feb 21, Eric Piel wrote:
>
> Here is the patch, it does exactly the same functionality but it's 
> completly static and therefore much smaller. Just tested on my computer, 
> it does the trick.
>
I'm happy for this to go ahead as I will want it on always by default, 
however you pretty much kill the folk who prefer to 'nice' tasks to prevent 
interuption to other applications rather than using it to say "I do not care 
how long this task takes".

I know on our proxy server there are a few rebuilds which we run at a nice'd
level, not because we do not care how long they take (we want the rebuilds to
finish as soon as possible) but rather so that squid's performance is not
affected.

Cheers

Alex
 
> Eric
> --
> Avoid the cpufreq ondemand governor to count the nice'd tasks as load 
> for the cpu.
> 
>  cpufreq_ondemand.c |   17 ++++++++++-------
>  1 files changed, 10 insertions(+), 7 deletions(-)
> 
> Signed-off-by: Eric Piel <eric.piel@tremplin-utc.net>
> --

-- 
 _______________________________________ 
/ You can drive a horse to water, but a \
\ pencil must be lead.                  /
 --------------------------------------- 
        \   ^__^
         \  (oo)\_______
            (__)\       )\/\
                ||----w |
                ||     ||

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 147 bytes --]

_______________________________________________
Cpufreq mailing list
Cpufreq@lists.linux.org.uk
http://lists.linux.org.uk/mailman/listinfo/cpufreq

Re: [PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff

[Eric Piel]

Alexander Clouter a écrit :
> On Feb 21, Eric Piel wrote:
> 
>>Here is the patch, it does exactly the same functionality but it's 
>>completly static and therefore much smaller. Just tested on my computer, 
>>it does the trick.
>>
> 
> I'm happy for this to go ahead as I will want it on always by default, 
> however you pretty much kill the folk who prefer to 'nice' tasks to prevent 
> interuption to other applications rather than using it to say "I do not care 
> how long this task takes".
> 
> I know on our proxy server there are a few rebuilds which we run at a nice'd
> level, not because we do not care how long they take (we want the rebuilds to
> finish as soon as possible) but rather so that squid's performance is not
> affected.
> 
That's very unusual cases. You cannot take care of all unusual case with 
a governor that tries to be as simple as possible. Probably in the case 
you cite either you want to use a userspace governor which can be much 
more complex and sofisticated or maybe just:
renice -5 $(pidof) squid

Eric

Re: [PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff

[Dominik Brodowski]

On Mon, Feb 21, 2005 at 11:20:51AM +0100, Eric Piel wrote:
> Eric Piel a écrit :
> >Alexander Clouter a écrit :
> >
> >>Adds support to cpufreq_ondemand to ignore 'nice' cpu time
> >>
> >IMO, this policy should be has lightweight as possible. Why not making 
> >this feature just hard-coded? Don't count the load that nice'd tasks do, 
> >period!
> >
> >It would make the patch 5 lines instead of 50. Every userspace deamon 
> >have this feature enabled by default, so this is a very reasonable 
> >trade-off.
> >
> Here is the patch, it does exactly the same functionality but it's 
> completly static and therefore much smaller. Just tested on my computer, 
> it does the trick.

IMO it was a wise decision to do this as a tuneable parameter.

	Dominik

Re: [PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff

[Stefan Seyfried]

On Sun, Feb 20, 2005 at 05:14:32PM +0100, Eric Piel wrote:
 
> It would make the patch 5 lines instead of 50. Every userspace deamon 
> have this feature enabled by default, so this is a very reasonable 
> trade-off.

I'm sure that you cannot imagine how much beating i have gotten for making
CONSIDER_NICE=no the default for the powersaved on SUSE 9.1... :-)
It took man-weeks of debating what could be a sensible default for this.
This has to be tweakable.
-- 
Stefan Seyfried

Re: [PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff

[Eric Piel]

Stefan Seyfried a écrit :
> On Sun, Feb 20, 2005 at 05:14:32PM +0100, Eric Piel wrote:
>  
> 
>>It would make the patch 5 lines instead of 50. Every userspace deamon 
>>have this feature enabled by default, so this is a very reasonable 
>>trade-off.
> 
> 
> I'm sure that you cannot imagine how much beating i have gotten for making
> CONSIDER_NICE=no the default for the powersaved on SUSE 9.1... :-)
> It took man-weeks of debating what could be a sensible default for this.
> This has to be tweakable.
Well, my point was that if people want different things from the simple 
and normal behaviour, they use a userspace deamon... for instance 
powersaved .

Eric

RE: [PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff

[Pallipadi, Venkatesh]

>-----Original Message-----
>From: cpufreq-bounces@ZenII.linux.org.uk 
>[mailto:cpufreq-bounces@ZenII.linux.org.uk] On Behalf Of 
>Dominik Brodowski
>Sent: Monday, February 21, 2005 7:54 AM
>To: Eric Piel
>Cc: davej@redhat.com; cpufreq@ZenII.linux.org.uk; 
>alex-kernel@digriz.org.uk; Alexander Clouter
>Subject: Re: [PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff
>
>On Mon, Feb 21, 2005 at 11:20:51AM +0100, Eric Piel wrote:
>> Eric Piel a écrit :
>> >Alexander Clouter a écrit :
>> >
>> >>Adds support to cpufreq_ondemand to ignore 'nice' cpu time
>> >>
>> >IMO, this policy should be has lightweight as possible. Why 
>not making 
>> >this feature just hard-coded? Don't count the load that 
>nice'd tasks do, 
>> >period!
>> >
>> >It would make the patch 5 lines instead of 50. Every 
>userspace deamon 
>> >have this feature enabled by default, so this is a very reasonable 
>> >trade-off.
>> >
>> Here is the patch, it does exactly the same functionality but it's 
>> completly static and therefore much smaller. Just tested on 
>my computer, 
>> it does the trick.
>
>IMO it was a wise decision to do this as a tuneable parameter.
>

I too feel having a tunable parameter here is OK. This may make the patch big 
and the code size big as well. But, I don't think it is adding too much 
overhead at run-time. As long as no one changes these nice setting frequently,
the governor will still be lightweight. Right?

Thanks,
Venki

[PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff

[Alexander Clouter]

[-- Attachment #1.1.1: Type: text/plain, Size: 435 bytes --]

Adds support to cpufreq_ondemand to ignore 'nice' cpu time

Signed-off-by: Alexander Clouter <alex-kernel@digriz.org.uk>

-- 
 ________________________________________ 
/ Scintillation is not always            \
\ identification for an auric substance. /
 ---------------------------------------- 
        \   ^__^
         \  (oo)\_______
            (__)\       )\/\
                ||----w |
                ||     ||

[-- Attachment #1.1.2: cpufreq_ondemand-2.6.12-rc3-mm3-01_ignore-nice.diff --]
[-- Type: text/plain, Size: 4611 bytes --]

--- linux-2.6.12-rc3-mm3.orig/drivers/cpufreq/cpufreq_ondemand.c	2005-05-10 19:38:11.376638520 +0100
+++ linux-2.6.12-rc3-mm3/drivers/cpufreq/cpufreq_ondemand.c	2005-05-10 22:14:49.632886400 +0100
@@ -78,6 +78,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 = {
@@ -115,6 +116,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)
@@ -193,6 +195,46 @@
 	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 +
+			( !dbs_tuners_ins.ignore_nice
+			  ? kstat_cpu(j).cpustat.nice : 0 );
+		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)
@@ -201,6 +243,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,
@@ -209,6 +252,7 @@
 	&sampling_down_factor.attr,
 	&up_threshold.attr,
 	&down_threshold.attr,
+	&ignore_nice.attr,
 	NULL
 };
 
@@ -253,6 +297,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;
@@ -269,6 +316,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;
@@ -297,6 +347,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;
@@ -312,6 +365,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;
@@ -397,10 +453,11 @@
 		
 			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 +
+				( !dbs_tuners_ins.ignore_nice
+				  ? kstat_cpu(j).cpustat.nice : 0 );
+			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);
@@ -420,6 +477,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();
 		}

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 147 bytes --]

_______________________________________________
Cpufreq mailing list
Cpufreq@lists.linux.org.uk
http://lists.linux.org.uk/mailman/listinfo/cpufreq

Re: [PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff

[Dave Jones]

On Tue, May 10, 2005 at 11:30:31PM +0100, Alexander Clouter wrote:
 > Adds support to cpufreq_ondemand to ignore 'nice' cpu time
 > 
 > Signed-off-by: Alexander Clouter <alex-kernel@digriz.org.uk>
 > 
 
quoted-printable (more like unprintable) patches tend to get
horribly mangled.  This is what they ended up like..

@@ -193,6 +195,46 @@
    return count;
 }
=20
+static ssize_t store_ignore_nice(struct cpufreq_policy *policy,
+       const char *buf, size_t count)
+{
+   unsigned int input;
+   int ret;
+
+   unsigned int j;
+=09
+   ret =3D sscanf (buf, "%u", &input);
+   if ( ret !=3D 1 )
+       return -EINVAL;
+
+   if ( input > 1 )
+       input =3D 1;
+=09


(Note the =20/09 crap)

Please resend with a different mailer if necessary.

		Dave

Re: [PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff

[Dave Jones]

On Tue, May 10, 2005 at 09:30:45PM -0400, Dave Jones wrote:
 > On Tue, May 10, 2005 at 11:30:31PM +0100, Alexander Clouter wrote:
 >  > Adds support to cpufreq_ondemand to ignore 'nice' cpu time
 >  > 
 >  > Signed-off-by: Alexander Clouter <alex-kernel@digriz.org.uk>
 >  > 
 >  
 > quoted-printable (more like unprintable) patches tend to get
 > horribly mangled.  This is what they ended up like..
 > 
 > @@ -193,6 +195,46 @@
 >     return count;
 >  }
 > =20
 > +static ssize_t store_ignore_nice(struct cpufreq_policy *policy,
 > +       const char *buf, size_t count)
 > +{
 > +   unsigned int input;
 > +   int ret;
 > +
 > +   unsigned int j;
 > +=09
 > +   ret =3D sscanf (buf, "%u", &input);
 > +   if ( ret !=3D 1 )
 > +       return -EINVAL;
 > +
 > +   if ( input > 1 )
 > +       input =3D 1;
 > +=09
 > 
 > 
 > (Note the =20/09 crap)
 > 
 > Please resend with a different mailer if necessary.

Ignore this, I ran them through mutt on a different box, and
they showed up just fine. Wierd.

		Dave