All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff
@ 2005-02-20 13:15 Alexander Clouter
  2005-02-20 14:12 ` Arjan van de Ven
                   ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Alexander Clouter @ 2005-02-20 13:15 UTC (permalink / raw)
  To: cpufreq; +Cc: davej, linux, alex-kernel


[-- 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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff
  2005-02-20 13:15 [PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff Alexander Clouter
@ 2005-02-20 14:12 ` Arjan van de Ven
  2005-02-20 14:51   ` Dominik Brodowski
  2005-02-20 14:53 ` Dominik Brodowski
  2005-02-20 16:14 ` Eric Piel
  2 siblings, 1 reply; 18+ messages in thread
From: Arjan van de Ven @ 2005-02-20 14:12 UTC (permalink / raw)
  To: Alexander Clouter; +Cc: davej, cpufreq, alex-kernel, linux

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.....

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff
  2005-02-20 14:12 ` Arjan van de Ven
@ 2005-02-20 14:51   ` Dominik Brodowski
  0 siblings, 0 replies; 18+ messages in thread
From: Dominik Brodowski @ 2005-02-20 14:51 UTC (permalink / raw)
  To: Arjan van de Ven; +Cc: davej, alex-kernel, Alexander Clouter, cpufreq

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff
  2005-02-20 13:15 [PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff Alexander Clouter
  2005-02-20 14:12 ` Arjan van de Ven
@ 2005-02-20 14:53 ` Dominik Brodowski
  2005-02-20 18:14   ` Alexander Clouter
  2005-02-20 16:14 ` Eric Piel
  2 siblings, 1 reply; 18+ messages in thread
From: Dominik Brodowski @ 2005-02-20 14:53 UTC (permalink / raw)
  To: Alexander Clouter; +Cc: cpufreq, alex-kernel, davej

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff
  2005-02-20 13:15 [PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff Alexander Clouter
  2005-02-20 14:12 ` Arjan van de Ven
  2005-02-20 14:53 ` Dominik Brodowski
@ 2005-02-20 16:14 ` Eric Piel
  2005-02-21 10:20   ` Eric Piel
  2005-02-21 13:11   ` Stefan Seyfried
  2 siblings, 2 replies; 18+ messages in thread
From: Eric Piel @ 2005-02-20 16:14 UTC (permalink / raw)
  To: Alexander Clouter; +Cc: davej, cpufreq, alex-kernel, linux

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff
  2005-02-20 14:53 ` Dominik Brodowski
@ 2005-02-20 18:14   ` Alexander Clouter
  2005-02-20 20:59     ` PCS
  2005-02-21 11:41     ` Bruno Ducrot
  0 siblings, 2 replies; 18+ messages in thread
From: Alexander Clouter @ 2005-02-20 18:14 UTC (permalink / raw)
  To: Dominik Brodowski; +Cc: cpufreq, davej


[-- 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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff
  2005-02-20 18:14   ` Alexander Clouter
@ 2005-02-20 20:59     ` PCS
  2005-02-21 11:41     ` Bruno Ducrot
  1 sibling, 0 replies; 18+ messages in thread
From: PCS @ 2005-02-20 20:59 UTC (permalink / raw)
  To: cpufreq

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff
  2005-02-20 16:14 ` Eric Piel
@ 2005-02-21 10:20   ` Eric Piel
  2005-02-21 10:27     ` Alexander Clouter
  2005-02-21 15:54     ` Dominik Brodowski
  2005-02-21 13:11   ` Stefan Seyfried
  1 sibling, 2 replies; 18+ messages in thread
From: Eric Piel @ 2005-02-21 10:20 UTC (permalink / raw)
  To: Eric Piel; +Cc: davej, cpufreq, alex-kernel, Alexander Clouter, linux

[-- 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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff
  2005-02-21 10:20   ` Eric Piel
@ 2005-02-21 10:27     ` Alexander Clouter
  2005-02-21 12:58       ` Eric Piel
  2005-02-21 15:54     ` Dominik Brodowski
  1 sibling, 1 reply; 18+ messages in thread
From: Alexander Clouter @ 2005-02-21 10:27 UTC (permalink / raw)
  To: Eric Piel; +Cc: davej, cpufreq, linux


[-- 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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff
  2005-02-20 18:14   ` Alexander Clouter
  2005-02-20 20:59     ` PCS
@ 2005-02-21 11:41     ` Bruno Ducrot
  1 sibling, 0 replies; 18+ messages in thread
From: Bruno Ducrot @ 2005-02-21 11:41 UTC (permalink / raw)
  To: Alexander Clouter; +Cc: davej, cpufreq, Dominik Brodowski

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.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff
  2005-02-21 10:27     ` Alexander Clouter
@ 2005-02-21 12:58       ` Eric Piel
  0 siblings, 0 replies; 18+ messages in thread
From: Eric Piel @ 2005-02-21 12:58 UTC (permalink / raw)
  To: Alexander Clouter; +Cc: davej, cpufreq, linux

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff
  2005-02-20 16:14 ` Eric Piel
  2005-02-21 10:20   ` Eric Piel
@ 2005-02-21 13:11   ` Stefan Seyfried
  2005-02-21 13:31     ` Eric Piel
  1 sibling, 1 reply; 18+ messages in thread
From: Stefan Seyfried @ 2005-02-21 13:11 UTC (permalink / raw)
  To: cpufreq

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff
  2005-02-21 13:11   ` Stefan Seyfried
@ 2005-02-21 13:31     ` Eric Piel
  0 siblings, 0 replies; 18+ messages in thread
From: Eric Piel @ 2005-02-21 13:31 UTC (permalink / raw)
  To: Stefan Seyfried; +Cc: cpufreq

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff
  2005-02-21 10:20   ` Eric Piel
  2005-02-21 10:27     ` Alexander Clouter
@ 2005-02-21 15:54     ` Dominik Brodowski
  1 sibling, 0 replies; 18+ messages in thread
From: Dominik Brodowski @ 2005-02-21 15:54 UTC (permalink / raw)
  To: Eric Piel; +Cc: davej, cpufreq, alex-kernel, Alexander Clouter

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff
@ 2005-02-21 18:05 Pallipadi, Venkatesh
  0 siblings, 0 replies; 18+ messages in thread
From: Pallipadi, Venkatesh @ 2005-02-21 18:05 UTC (permalink / raw)
  To: Dominik Brodowski, Eric Piel
  Cc: davej, cpufreq, alex-kernel, Alexander Clouter


>-----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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff
@ 2005-05-10 22:30 Alexander Clouter
  2005-05-11  1:30 ` Dave Jones
  0 siblings, 1 reply; 18+ messages in thread
From: Alexander Clouter @ 2005-05-10 22:30 UTC (permalink / raw)
  To: cpufreq; +Cc: davej, linux, alex-kernel


[-- 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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff
  2005-05-10 22:30 Alexander Clouter
@ 2005-05-11  1:30 ` Dave Jones
  2005-05-11  2:38   ` Dave Jones
  0 siblings, 1 reply; 18+ messages in thread
From: Dave Jones @ 2005-05-11  1:30 UTC (permalink / raw)
  To: Alexander Clouter; +Cc: cpufreq, linux, alex-kernel

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff
  2005-05-11  1:30 ` Dave Jones
@ 2005-05-11  2:38   ` Dave Jones
  0 siblings, 0 replies; 18+ messages in thread
From: Dave Jones @ 2005-05-11  2:38 UTC (permalink / raw)
  To: Alexander Clouter; +Cc: cpufreq, linux, alex-kernel

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2005-05-11  2:38 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-02-20 13:15 [PATCH] (1/3) cpufreq_ondemand - 01_ignore-nice.diff Alexander Clouter
2005-02-20 14:12 ` Arjan van de Ven
2005-02-20 14:51   ` Dominik Brodowski
2005-02-20 14:53 ` Dominik Brodowski
2005-02-20 18:14   ` Alexander Clouter
2005-02-20 20:59     ` PCS
2005-02-21 11:41     ` Bruno Ducrot
2005-02-20 16:14 ` Eric Piel
2005-02-21 10:20   ` Eric Piel
2005-02-21 10:27     ` Alexander Clouter
2005-02-21 12:58       ` Eric Piel
2005-02-21 15:54     ` Dominik Brodowski
2005-02-21 13:11   ` Stefan Seyfried
2005-02-21 13:31     ` Eric Piel
  -- strict thread matches above, loose matches on Subject: below --
2005-02-21 18:05 Pallipadi, Venkatesh
2005-05-10 22:30 Alexander Clouter
2005-05-11  1:30 ` Dave Jones
2005-05-11  2:38   ` Dave Jones

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.