Re: [PATCH v2 linux-next] cpufreq: governors: Calculate iowait time only when necessary
On 03/22/2013 01:54 AM, Rafael J. Wysocki wrote: > > Applied to linux-pm.git/bleeding-edge and will be moved to linux-next if there > are no build problems in the bleeding-edge branch. > > Thanks, > Rafael Hi Rafael, I just noticed a regression with this patch with the calculation of wall time in get get_cpu_idle_time that prevents the CPU to increase to max frequency, when we use ondemand with io_is_busy = 1. I'm sorry about that, I'm sending a patch to fix it. Regards, Stratos -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 linux-next] cpufreq: governors: Calculate iowait time only when necessary
On 03/22/2013 01:54 AM, Rafael J. Wysocki wrote: Applied to linux-pm.git/bleeding-edge and will be moved to linux-next if there are no build problems in the bleeding-edge branch. Thanks, Rafael Hi Rafael, I just noticed a regression with this patch with the calculation of wall time in get get_cpu_idle_time that prevents the CPU to increase to max frequency, when we use ondemand with io_is_busy = 1. I'm sorry about that, I'm sending a patch to fix it. Regards, Stratos -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 linux-next] cpufreq: governors: Calculate iowait time only when necessary
On Friday, March 01, 2013 09:18:08 AM Viresh Kumar wrote: > On 28 February 2013 22:27, Stratos Karafotis wrote: > > Currently we always calculate the CPU iowait time and add it to idle time. > > If we are in ondemand and we use io_is_busy, we re-calculate iowait time > > and we subtract it from idle time. > > > > With this patch iowait time is calculated only when necessary avoiding > > the double call to get_cpu_iowait_time_us. We use a parameter in > > function get_cpu_idle_time to distinguish when the iowait time will be > > added to idle time or not, without the need of keeping the prev_io_wait. > > > > Signed-off-by: Stratos Karafotis > > --- > > drivers/cpufreq/cpufreq_conservative.c | 2 +- > > drivers/cpufreq/cpufreq_governor.c | 46 > > +- > > drivers/cpufreq/cpufreq_governor.h | 3 +-- > > drivers/cpufreq/cpufreq_ondemand.c | 11 +++- > > 4 files changed, 29 insertions(+), 33 deletions(-) > > > > diff --git a/drivers/cpufreq/cpufreq_conservative.c > > b/drivers/cpufreq/cpufreq_conservative.c > > index 4fd0006..dfe652c 100644 > > --- a/drivers/cpufreq/cpufreq_conservative.c > > +++ b/drivers/cpufreq/cpufreq_conservative.c > > @@ -242,7 +242,7 @@ static ssize_t store_ignore_nice_load(struct kobject > > *a, struct attribute *b, > > struct cs_cpu_dbs_info_s *dbs_info; > > dbs_info = _cpu(cs_cpu_dbs_info, j); > > dbs_info->cdbs.prev_cpu_idle = get_cpu_idle_time(j, > > - > > _info->cdbs.prev_cpu_wall); > > + _info->cdbs.prev_cpu_wall, 0); > > if (cs_tuners.ignore_nice) > > dbs_info->cdbs.prev_cpu_nice = > > kcpustat_cpu(j).cpustat[CPUTIME_NICE]; > > diff --git a/drivers/cpufreq/cpufreq_governor.c > > b/drivers/cpufreq/cpufreq_governor.c > > index 5a76086..a322bda 100644 > > --- a/drivers/cpufreq/cpufreq_governor.c > > +++ b/drivers/cpufreq/cpufreq_governor.c > > @@ -50,13 +50,13 @@ static inline u64 get_cpu_idle_time_jiffy(unsigned int > > cpu, u64 *wall) > > return cputime_to_usecs(idle_time); > > } > > > > -u64 get_cpu_idle_time(unsigned int cpu, u64 *wall) > > +u64 get_cpu_idle_time(unsigned int cpu, u64 *wall, int io_busy) > > { > > u64 idle_time = get_cpu_idle_time_us(cpu, NULL); > > > > if (idle_time == -1ULL) > > return get_cpu_idle_time_jiffy(cpu, wall); > > - else > > + else if (!io_busy) > > idle_time += get_cpu_iowait_time_us(cpu, wall); > > > > return idle_time; > > @@ -83,13 +83,22 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu) > > /* Get Absolute Load (in terms of freq for ondemand gov) */ > > for_each_cpu(j, policy->cpus) { > > struct cpu_dbs_common_info *j_cdbs; > > - u64 cur_wall_time, cur_idle_time, cur_iowait_time; > > - unsigned int idle_time, wall_time, iowait_time; > > + u64 cur_wall_time, cur_idle_time; > > + unsigned int idle_time, wall_time; > > unsigned int load; > > + int io_busy = 0; > > > > j_cdbs = dbs_data->get_cpu_cdbs(j); > > > > - cur_idle_time = get_cpu_idle_time(j, _wall_time); > > + /* > > +* For the purpose of ondemand, waiting for disk IO is > > +* an indication that you're performance critical, and > > +* not that the system is actually idle. So do not add > > +* the iowait time to the cpu idle time. > > +*/ > > + if (dbs_data->governor == GOV_ONDEMAND) > > + io_busy = od_tuners->io_is_busy; > > + cur_idle_time = get_cpu_idle_time(j, _wall_time, > > io_busy); > > > > wall_time = (unsigned int) > > (cur_wall_time - j_cdbs->prev_cpu_wall); > > @@ -117,29 +126,6 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu) > > idle_time += jiffies_to_usecs(cur_nice_jiffies); > > } > > > > - if (dbs_data->governor == GOV_ONDEMAND) { > > - struct od_cpu_dbs_info_s *od_j_dbs_info = > > - dbs_data->get_cpu_dbs_info_s(cpu); > > - > > - cur_iowait_time = get_cpu_iowait_time_us(j, > > - _wall_time); > > - if (cur_iowait_time == -1ULL) > > - cur_iowait_time = 0; > > - > > - iowait_time = (unsigned int) (cur_iowait_time - > > - od_j_dbs_info->prev_cpu_iowait); > > - od_j_dbs_info->prev_cpu_iowait = cur_iowait_time; > > - > > - /* > > -* For the purpose of ondemand,
Re: [PATCH v2 linux-next] cpufreq: governors: Calculate iowait time only when necessary
On Friday, March 01, 2013 09:18:08 AM Viresh Kumar wrote: On 28 February 2013 22:27, Stratos Karafotis strat...@semaphore.gr wrote: Currently we always calculate the CPU iowait time and add it to idle time. If we are in ondemand and we use io_is_busy, we re-calculate iowait time and we subtract it from idle time. With this patch iowait time is calculated only when necessary avoiding the double call to get_cpu_iowait_time_us. We use a parameter in function get_cpu_idle_time to distinguish when the iowait time will be added to idle time or not, without the need of keeping the prev_io_wait. Signed-off-by: Stratos Karafotis strat...@semaphore.gr --- drivers/cpufreq/cpufreq_conservative.c | 2 +- drivers/cpufreq/cpufreq_governor.c | 46 +- drivers/cpufreq/cpufreq_governor.h | 3 +-- drivers/cpufreq/cpufreq_ondemand.c | 11 +++- 4 files changed, 29 insertions(+), 33 deletions(-) diff --git a/drivers/cpufreq/cpufreq_conservative.c b/drivers/cpufreq/cpufreq_conservative.c index 4fd0006..dfe652c 100644 --- a/drivers/cpufreq/cpufreq_conservative.c +++ b/drivers/cpufreq/cpufreq_conservative.c @@ -242,7 +242,7 @@ static ssize_t store_ignore_nice_load(struct kobject *a, struct attribute *b, struct cs_cpu_dbs_info_s *dbs_info; dbs_info = per_cpu(cs_cpu_dbs_info, j); dbs_info-cdbs.prev_cpu_idle = get_cpu_idle_time(j, - dbs_info-cdbs.prev_cpu_wall); + dbs_info-cdbs.prev_cpu_wall, 0); if (cs_tuners.ignore_nice) dbs_info-cdbs.prev_cpu_nice = kcpustat_cpu(j).cpustat[CPUTIME_NICE]; diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c index 5a76086..a322bda 100644 --- a/drivers/cpufreq/cpufreq_governor.c +++ b/drivers/cpufreq/cpufreq_governor.c @@ -50,13 +50,13 @@ static inline u64 get_cpu_idle_time_jiffy(unsigned int cpu, u64 *wall) return cputime_to_usecs(idle_time); } -u64 get_cpu_idle_time(unsigned int cpu, u64 *wall) +u64 get_cpu_idle_time(unsigned int cpu, u64 *wall, int io_busy) { u64 idle_time = get_cpu_idle_time_us(cpu, NULL); if (idle_time == -1ULL) return get_cpu_idle_time_jiffy(cpu, wall); - else + else if (!io_busy) idle_time += get_cpu_iowait_time_us(cpu, wall); return idle_time; @@ -83,13 +83,22 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu) /* Get Absolute Load (in terms of freq for ondemand gov) */ for_each_cpu(j, policy-cpus) { struct cpu_dbs_common_info *j_cdbs; - u64 cur_wall_time, cur_idle_time, cur_iowait_time; - unsigned int idle_time, wall_time, iowait_time; + u64 cur_wall_time, cur_idle_time; + unsigned int idle_time, wall_time; unsigned int load; + int io_busy = 0; j_cdbs = dbs_data-get_cpu_cdbs(j); - cur_idle_time = get_cpu_idle_time(j, cur_wall_time); + /* +* For the purpose of ondemand, waiting for disk IO is +* an indication that you're performance critical, and +* not that the system is actually idle. So do not add +* the iowait time to the cpu idle time. +*/ + if (dbs_data-governor == GOV_ONDEMAND) + io_busy = od_tuners-io_is_busy; + cur_idle_time = get_cpu_idle_time(j, cur_wall_time, io_busy); wall_time = (unsigned int) (cur_wall_time - j_cdbs-prev_cpu_wall); @@ -117,29 +126,6 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu) idle_time += jiffies_to_usecs(cur_nice_jiffies); } - if (dbs_data-governor == GOV_ONDEMAND) { - struct od_cpu_dbs_info_s *od_j_dbs_info = - dbs_data-get_cpu_dbs_info_s(cpu); - - cur_iowait_time = get_cpu_iowait_time_us(j, - cur_wall_time); - if (cur_iowait_time == -1ULL) - cur_iowait_time = 0; - - iowait_time = (unsigned int) (cur_iowait_time - - od_j_dbs_info-prev_cpu_iowait); - od_j_dbs_info-prev_cpu_iowait = cur_iowait_time; - - /* -* For the purpose of ondemand, waiting for disk IO is -* an indication that you're performance critical, and -* not that the system
Re: [PATCH v2 linux-next] cpufreq: governors: Calculate iowait time only when necessary
On 28 February 2013 22:27, Stratos Karafotis wrote: > Currently we always calculate the CPU iowait time and add it to idle time. > If we are in ondemand and we use io_is_busy, we re-calculate iowait time > and we subtract it from idle time. > > With this patch iowait time is calculated only when necessary avoiding > the double call to get_cpu_iowait_time_us. We use a parameter in > function get_cpu_idle_time to distinguish when the iowait time will be > added to idle time or not, without the need of keeping the prev_io_wait. > > Signed-off-by: Stratos Karafotis > --- > drivers/cpufreq/cpufreq_conservative.c | 2 +- > drivers/cpufreq/cpufreq_governor.c | 46 > +- > drivers/cpufreq/cpufreq_governor.h | 3 +-- > drivers/cpufreq/cpufreq_ondemand.c | 11 +++- > 4 files changed, 29 insertions(+), 33 deletions(-) > > diff --git a/drivers/cpufreq/cpufreq_conservative.c > b/drivers/cpufreq/cpufreq_conservative.c > index 4fd0006..dfe652c 100644 > --- a/drivers/cpufreq/cpufreq_conservative.c > +++ b/drivers/cpufreq/cpufreq_conservative.c > @@ -242,7 +242,7 @@ static ssize_t store_ignore_nice_load(struct kobject *a, > struct attribute *b, > struct cs_cpu_dbs_info_s *dbs_info; > dbs_info = _cpu(cs_cpu_dbs_info, j); > dbs_info->cdbs.prev_cpu_idle = get_cpu_idle_time(j, > - > _info->cdbs.prev_cpu_wall); > + _info->cdbs.prev_cpu_wall, 0); > if (cs_tuners.ignore_nice) > dbs_info->cdbs.prev_cpu_nice = > kcpustat_cpu(j).cpustat[CPUTIME_NICE]; > diff --git a/drivers/cpufreq/cpufreq_governor.c > b/drivers/cpufreq/cpufreq_governor.c > index 5a76086..a322bda 100644 > --- a/drivers/cpufreq/cpufreq_governor.c > +++ b/drivers/cpufreq/cpufreq_governor.c > @@ -50,13 +50,13 @@ static inline u64 get_cpu_idle_time_jiffy(unsigned int > cpu, u64 *wall) > return cputime_to_usecs(idle_time); > } > > -u64 get_cpu_idle_time(unsigned int cpu, u64 *wall) > +u64 get_cpu_idle_time(unsigned int cpu, u64 *wall, int io_busy) > { > u64 idle_time = get_cpu_idle_time_us(cpu, NULL); > > if (idle_time == -1ULL) > return get_cpu_idle_time_jiffy(cpu, wall); > - else > + else if (!io_busy) > idle_time += get_cpu_iowait_time_us(cpu, wall); > > return idle_time; > @@ -83,13 +83,22 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu) > /* Get Absolute Load (in terms of freq for ondemand gov) */ > for_each_cpu(j, policy->cpus) { > struct cpu_dbs_common_info *j_cdbs; > - u64 cur_wall_time, cur_idle_time, cur_iowait_time; > - unsigned int idle_time, wall_time, iowait_time; > + u64 cur_wall_time, cur_idle_time; > + unsigned int idle_time, wall_time; > unsigned int load; > + int io_busy = 0; > > j_cdbs = dbs_data->get_cpu_cdbs(j); > > - cur_idle_time = get_cpu_idle_time(j, _wall_time); > + /* > +* For the purpose of ondemand, waiting for disk IO is > +* an indication that you're performance critical, and > +* not that the system is actually idle. So do not add > +* the iowait time to the cpu idle time. > +*/ > + if (dbs_data->governor == GOV_ONDEMAND) > + io_busy = od_tuners->io_is_busy; > + cur_idle_time = get_cpu_idle_time(j, _wall_time, io_busy); > > wall_time = (unsigned int) > (cur_wall_time - j_cdbs->prev_cpu_wall); > @@ -117,29 +126,6 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu) > idle_time += jiffies_to_usecs(cur_nice_jiffies); > } > > - if (dbs_data->governor == GOV_ONDEMAND) { > - struct od_cpu_dbs_info_s *od_j_dbs_info = > - dbs_data->get_cpu_dbs_info_s(cpu); > - > - cur_iowait_time = get_cpu_iowait_time_us(j, > - _wall_time); > - if (cur_iowait_time == -1ULL) > - cur_iowait_time = 0; > - > - iowait_time = (unsigned int) (cur_iowait_time - > - od_j_dbs_info->prev_cpu_iowait); > - od_j_dbs_info->prev_cpu_iowait = cur_iowait_time; > - > - /* > -* For the purpose of ondemand, waiting for disk IO is > -* an indication that you're performance critical, and > -* not that the system is actually idle. So subtract > the > -* iowait time from the cpu idle time. > -
Re: [PATCH v2 linux-next] cpufreq: governors: Calculate iowait time only when necessary
Hi Viresh, On 02/28/2013 08:58 AM, Viresh Kumar wrote: > I have really spent some 10-15 minutes reviewing this patch as initially it > looked to me like something is missing here and calculations are going wrong. > > But it was fine :) Thanks for you review. >> diff --git a/drivers/cpufreq/cpufreq_ondemand.c >> b/drivers/cpufreq/cpufreq_ondemand.c >> index f3eb26c..46be2c4 100644 >> --- a/drivers/cpufreq/cpufreq_ondemand.c >> +++ b/drivers/cpufreq/cpufreq_ondemand.c >> @@ -339,11 +339,20 @@ static ssize_t store_io_is_busy(struct kobject *a, >> struct attribute *b, >> { > >> + /* we need to re-evaluate prev_cpu_idle */ >> + for_each_online_cpu(j) { >> + struct od_cpu_dbs_info_s *dbs_info; >> + dbs_info = _cpu(od_cpu_dbs_info, j); > > Probably squash above two lines and give a blank line after it. > >> + dbs_info->cdbs.prev_cpu_idle = get_cpu_idle_time(j, >> + _info->cdbs.prev_cpu_wall, od_tuners.io_is_busy); >> + } >> return count; >> } > > And after that add my: > > Acked-by: Viresh Kumar > Bellow V2 with your suggestion included. Regards, Stratos 8<-- Currently we always calculate the CPU iowait time and add it to idle time. If we are in ondemand and we use io_is_busy, we re-calculate iowait time and we subtract it from idle time. With this patch iowait time is calculated only when necessary avoiding the double call to get_cpu_iowait_time_us. We use a parameter in function get_cpu_idle_time to distinguish when the iowait time will be added to idle time or not, without the need of keeping the prev_io_wait. Signed-off-by: Stratos Karafotis --- drivers/cpufreq/cpufreq_conservative.c | 2 +- drivers/cpufreq/cpufreq_governor.c | 46 +- drivers/cpufreq/cpufreq_governor.h | 3 +-- drivers/cpufreq/cpufreq_ondemand.c | 11 +++- 4 files changed, 29 insertions(+), 33 deletions(-) diff --git a/drivers/cpufreq/cpufreq_conservative.c b/drivers/cpufreq/cpufreq_conservative.c index 4fd0006..dfe652c 100644 --- a/drivers/cpufreq/cpufreq_conservative.c +++ b/drivers/cpufreq/cpufreq_conservative.c @@ -242,7 +242,7 @@ static ssize_t store_ignore_nice_load(struct kobject *a, struct attribute *b, struct cs_cpu_dbs_info_s *dbs_info; dbs_info = _cpu(cs_cpu_dbs_info, j); dbs_info->cdbs.prev_cpu_idle = get_cpu_idle_time(j, - _info->cdbs.prev_cpu_wall); + _info->cdbs.prev_cpu_wall, 0); if (cs_tuners.ignore_nice) dbs_info->cdbs.prev_cpu_nice = kcpustat_cpu(j).cpustat[CPUTIME_NICE]; diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c index 5a76086..a322bda 100644 --- a/drivers/cpufreq/cpufreq_governor.c +++ b/drivers/cpufreq/cpufreq_governor.c @@ -50,13 +50,13 @@ static inline u64 get_cpu_idle_time_jiffy(unsigned int cpu, u64 *wall) return cputime_to_usecs(idle_time); } -u64 get_cpu_idle_time(unsigned int cpu, u64 *wall) +u64 get_cpu_idle_time(unsigned int cpu, u64 *wall, int io_busy) { u64 idle_time = get_cpu_idle_time_us(cpu, NULL); if (idle_time == -1ULL) return get_cpu_idle_time_jiffy(cpu, wall); - else + else if (!io_busy) idle_time += get_cpu_iowait_time_us(cpu, wall); return idle_time; @@ -83,13 +83,22 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu) /* Get Absolute Load (in terms of freq for ondemand gov) */ for_each_cpu(j, policy->cpus) { struct cpu_dbs_common_info *j_cdbs; - u64 cur_wall_time, cur_idle_time, cur_iowait_time; - unsigned int idle_time, wall_time, iowait_time; + u64 cur_wall_time, cur_idle_time; + unsigned int idle_time, wall_time; unsigned int load; + int io_busy = 0; j_cdbs = dbs_data->get_cpu_cdbs(j); - cur_idle_time = get_cpu_idle_time(j, _wall_time); + /* +* For the purpose of ondemand, waiting for disk IO is +* an indication that you're performance critical, and +* not that the system is actually idle. So do not add +* the iowait time to the cpu idle time. +*/ + if (dbs_data->governor == GOV_ONDEMAND) + io_busy = od_tuners->io_is_busy; + cur_idle_time = get_cpu_idle_time(j, _wall_time, io_busy); wall_time = (unsigned int) (cur_wall_time - j_cdbs->prev_cpu_wall); @@ -117,29 +126,6 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu) idle_time += jiffies_to_usecs(cur_nice_jiffies); } -
Re: [PATCH v2 linux-next] cpufreq: governors: Calculate iowait time only when necessary
Hi Viresh, On 02/28/2013 08:58 AM, Viresh Kumar wrote: I have really spent some 10-15 minutes reviewing this patch as initially it looked to me like something is missing here and calculations are going wrong. But it was fine :) Thanks for you review. diff --git a/drivers/cpufreq/cpufreq_ondemand.c b/drivers/cpufreq/cpufreq_ondemand.c index f3eb26c..46be2c4 100644 --- a/drivers/cpufreq/cpufreq_ondemand.c +++ b/drivers/cpufreq/cpufreq_ondemand.c @@ -339,11 +339,20 @@ static ssize_t store_io_is_busy(struct kobject *a, struct attribute *b, { + /* we need to re-evaluate prev_cpu_idle */ + for_each_online_cpu(j) { + struct od_cpu_dbs_info_s *dbs_info; + dbs_info = per_cpu(od_cpu_dbs_info, j); Probably squash above two lines and give a blank line after it. + dbs_info-cdbs.prev_cpu_idle = get_cpu_idle_time(j, + dbs_info-cdbs.prev_cpu_wall, od_tuners.io_is_busy); + } return count; } And after that add my: Acked-by: Viresh Kumar viresh.ku...@linaro.org Bellow V2 with your suggestion included. Regards, Stratos 8-- Currently we always calculate the CPU iowait time and add it to idle time. If we are in ondemand and we use io_is_busy, we re-calculate iowait time and we subtract it from idle time. With this patch iowait time is calculated only when necessary avoiding the double call to get_cpu_iowait_time_us. We use a parameter in function get_cpu_idle_time to distinguish when the iowait time will be added to idle time or not, without the need of keeping the prev_io_wait. Signed-off-by: Stratos Karafotis strat...@semaphore.gr --- drivers/cpufreq/cpufreq_conservative.c | 2 +- drivers/cpufreq/cpufreq_governor.c | 46 +- drivers/cpufreq/cpufreq_governor.h | 3 +-- drivers/cpufreq/cpufreq_ondemand.c | 11 +++- 4 files changed, 29 insertions(+), 33 deletions(-) diff --git a/drivers/cpufreq/cpufreq_conservative.c b/drivers/cpufreq/cpufreq_conservative.c index 4fd0006..dfe652c 100644 --- a/drivers/cpufreq/cpufreq_conservative.c +++ b/drivers/cpufreq/cpufreq_conservative.c @@ -242,7 +242,7 @@ static ssize_t store_ignore_nice_load(struct kobject *a, struct attribute *b, struct cs_cpu_dbs_info_s *dbs_info; dbs_info = per_cpu(cs_cpu_dbs_info, j); dbs_info-cdbs.prev_cpu_idle = get_cpu_idle_time(j, - dbs_info-cdbs.prev_cpu_wall); + dbs_info-cdbs.prev_cpu_wall, 0); if (cs_tuners.ignore_nice) dbs_info-cdbs.prev_cpu_nice = kcpustat_cpu(j).cpustat[CPUTIME_NICE]; diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c index 5a76086..a322bda 100644 --- a/drivers/cpufreq/cpufreq_governor.c +++ b/drivers/cpufreq/cpufreq_governor.c @@ -50,13 +50,13 @@ static inline u64 get_cpu_idle_time_jiffy(unsigned int cpu, u64 *wall) return cputime_to_usecs(idle_time); } -u64 get_cpu_idle_time(unsigned int cpu, u64 *wall) +u64 get_cpu_idle_time(unsigned int cpu, u64 *wall, int io_busy) { u64 idle_time = get_cpu_idle_time_us(cpu, NULL); if (idle_time == -1ULL) return get_cpu_idle_time_jiffy(cpu, wall); - else + else if (!io_busy) idle_time += get_cpu_iowait_time_us(cpu, wall); return idle_time; @@ -83,13 +83,22 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu) /* Get Absolute Load (in terms of freq for ondemand gov) */ for_each_cpu(j, policy-cpus) { struct cpu_dbs_common_info *j_cdbs; - u64 cur_wall_time, cur_idle_time, cur_iowait_time; - unsigned int idle_time, wall_time, iowait_time; + u64 cur_wall_time, cur_idle_time; + unsigned int idle_time, wall_time; unsigned int load; + int io_busy = 0; j_cdbs = dbs_data-get_cpu_cdbs(j); - cur_idle_time = get_cpu_idle_time(j, cur_wall_time); + /* +* For the purpose of ondemand, waiting for disk IO is +* an indication that you're performance critical, and +* not that the system is actually idle. So do not add +* the iowait time to the cpu idle time. +*/ + if (dbs_data-governor == GOV_ONDEMAND) + io_busy = od_tuners-io_is_busy; + cur_idle_time = get_cpu_idle_time(j, cur_wall_time, io_busy); wall_time = (unsigned int) (cur_wall_time - j_cdbs-prev_cpu_wall); @@ -117,29 +126,6 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu) idle_time += jiffies_to_usecs(cur_nice_jiffies); } -
Re: [PATCH v2 linux-next] cpufreq: governors: Calculate iowait time only when necessary
On 28 February 2013 22:27, Stratos Karafotis strat...@semaphore.gr wrote: Currently we always calculate the CPU iowait time and add it to idle time. If we are in ondemand and we use io_is_busy, we re-calculate iowait time and we subtract it from idle time. With this patch iowait time is calculated only when necessary avoiding the double call to get_cpu_iowait_time_us. We use a parameter in function get_cpu_idle_time to distinguish when the iowait time will be added to idle time or not, without the need of keeping the prev_io_wait. Signed-off-by: Stratos Karafotis strat...@semaphore.gr --- drivers/cpufreq/cpufreq_conservative.c | 2 +- drivers/cpufreq/cpufreq_governor.c | 46 +- drivers/cpufreq/cpufreq_governor.h | 3 +-- drivers/cpufreq/cpufreq_ondemand.c | 11 +++- 4 files changed, 29 insertions(+), 33 deletions(-) diff --git a/drivers/cpufreq/cpufreq_conservative.c b/drivers/cpufreq/cpufreq_conservative.c index 4fd0006..dfe652c 100644 --- a/drivers/cpufreq/cpufreq_conservative.c +++ b/drivers/cpufreq/cpufreq_conservative.c @@ -242,7 +242,7 @@ static ssize_t store_ignore_nice_load(struct kobject *a, struct attribute *b, struct cs_cpu_dbs_info_s *dbs_info; dbs_info = per_cpu(cs_cpu_dbs_info, j); dbs_info-cdbs.prev_cpu_idle = get_cpu_idle_time(j, - dbs_info-cdbs.prev_cpu_wall); + dbs_info-cdbs.prev_cpu_wall, 0); if (cs_tuners.ignore_nice) dbs_info-cdbs.prev_cpu_nice = kcpustat_cpu(j).cpustat[CPUTIME_NICE]; diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c index 5a76086..a322bda 100644 --- a/drivers/cpufreq/cpufreq_governor.c +++ b/drivers/cpufreq/cpufreq_governor.c @@ -50,13 +50,13 @@ static inline u64 get_cpu_idle_time_jiffy(unsigned int cpu, u64 *wall) return cputime_to_usecs(idle_time); } -u64 get_cpu_idle_time(unsigned int cpu, u64 *wall) +u64 get_cpu_idle_time(unsigned int cpu, u64 *wall, int io_busy) { u64 idle_time = get_cpu_idle_time_us(cpu, NULL); if (idle_time == -1ULL) return get_cpu_idle_time_jiffy(cpu, wall); - else + else if (!io_busy) idle_time += get_cpu_iowait_time_us(cpu, wall); return idle_time; @@ -83,13 +83,22 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu) /* Get Absolute Load (in terms of freq for ondemand gov) */ for_each_cpu(j, policy-cpus) { struct cpu_dbs_common_info *j_cdbs; - u64 cur_wall_time, cur_idle_time, cur_iowait_time; - unsigned int idle_time, wall_time, iowait_time; + u64 cur_wall_time, cur_idle_time; + unsigned int idle_time, wall_time; unsigned int load; + int io_busy = 0; j_cdbs = dbs_data-get_cpu_cdbs(j); - cur_idle_time = get_cpu_idle_time(j, cur_wall_time); + /* +* For the purpose of ondemand, waiting for disk IO is +* an indication that you're performance critical, and +* not that the system is actually idle. So do not add +* the iowait time to the cpu idle time. +*/ + if (dbs_data-governor == GOV_ONDEMAND) + io_busy = od_tuners-io_is_busy; + cur_idle_time = get_cpu_idle_time(j, cur_wall_time, io_busy); wall_time = (unsigned int) (cur_wall_time - j_cdbs-prev_cpu_wall); @@ -117,29 +126,6 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu) idle_time += jiffies_to_usecs(cur_nice_jiffies); } - if (dbs_data-governor == GOV_ONDEMAND) { - struct od_cpu_dbs_info_s *od_j_dbs_info = - dbs_data-get_cpu_dbs_info_s(cpu); - - cur_iowait_time = get_cpu_iowait_time_us(j, - cur_wall_time); - if (cur_iowait_time == -1ULL) - cur_iowait_time = 0; - - iowait_time = (unsigned int) (cur_iowait_time - - od_j_dbs_info-prev_cpu_iowait); - od_j_dbs_info-prev_cpu_iowait = cur_iowait_time; - - /* -* For the purpose of ondemand, waiting for disk IO is -* an indication that you're performance critical, and -* not that the system is actually idle. So subtract the -* iowait time from the cpu idle time. -*/ - if