Content-class: urn:content-classes:message
MIME-Version: 1.0
Content-Type: text/plain;
	charset="us-ascii"
Content-Transfer-Encoding: 8BIT
Subject: RE: [PATCH] cpufreq_(ondemand|conservative) (round three)
Date: Wed, 10 Nov 2004 17:27:07 -0800
Message-ID: <88056F38E9E48644A0F562A38C64FB600351CB3F@scsmsx403.amr.corp.intel.com>
Thread-Topic: [PATCH] cpufreq_(ondemand|conservative) (round three)
Thread-Index: AcTGliPCALv37s1sRbOBn5TanQegdAA8uO/g
From: "Pallipadi, Venkatesh" <venkatesh.pallipadi@intel.com>
To: "Alexander Clouter" <alex@digriz.org.uk>, <linux-kernel@vger.kernel.org>,
       <cpufreq@www.linux.org.uk>
Cc: <linux@dominikbrodowski.de>
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1356
Lines: 35


Thanks for the patches. Here are some comments about _ondemand patches.

(1) 00_consistency patch
I think it is OK to do this for sampling_rate. But, we may have some
nasty races / corner conditions if we do this for up_threashold and
down_threshold. The race I am thinking about is: we check the new
down_value with old up_value and we may end up finally with
down_threshold greater than up_threshold.

(2) 01_ignore-nice
The idea to have this is good. Somehow I am thinking of some corner
cases here too.
If the prev_cpu_idle_up and prev_cpu_idle_down have unconditionally
include nice and total_ticks includes it conditionally, then we cannot
do the proper subtract and compare of idle times. Am I missing anything
here?

(3) 02_check-rate-and-break-out
This looks good and ready to go. 

(4) 03_sys_freq_step
Looks good. One minor issue. Policy->max can change at run time (when
a/c power and battery power). So behaviour might change if you
initialize freq_step once instead of checking 5% of max during each
switching. But, doing it this way should be OK too, as 5% is not a
strict number.

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