Re: [PATCH] cpufreq: governor: Be friendly towards latency-sensitive bursty workloads

[Srivatsa S. Bhat]

On 06/07/2014 03:25 PM, Pavel Machek wrote:
> Hi!
> 
>> We just want to avoid the stupidity of dropping down the frequency to a 
>> minimum
>> and then enduring a needless (and long) delay before ramping it up back 
>> again.
>> So, let us simply carry forward the previous load - that is, let us just 
>> pretend
>> that the 'load' for the current time-window is the same as the load for the
>> previous window. That way, the frequency and voltage will continue to be set
>> to whatever values they were set at previously. This means that bursty 
>> workloads
>> will get a chance to influence the CPU frequency at which they wake up from
>> cpu-idle, based on their past execution history. Thus, they might be able to
>> avoid suffering from slow wakeups and long response-times.
>>
>> [ The right way to solve this problem is to teach the CPU frequency governors
>> to track load on a per-task basis, not a per-CPU basis, and set the 
>> appropriate
>> frequency on whichever CPU the task executes. But that involves redesigning
>> the cpufreq subsystem, so this patch should make the situation bearable until
>> then. ]
> 
> Are you sure? For example "./configure" load consists of a lot of
> short-lived tasks. Per-task basis may not be option for that.
> 

True, but then there is no guarantee that all the tasks spawned by ./configure
run on a single CPU. So per-CPU tracking may or may not help, depending on the
scheduling pattern.

For very-short-lived tasks, per-task tracking won't give much benefit, but it
will for medium to long-lived tasks. So I guess we might need some sort of a
combination of both methods of tracking, to handle all the scenarios well.

>> A rudimentary and somewhat approximately latency-sensitive workload such as
>> sleeping-ebizzy itself showed a consistent, noticeable performance 
>> improvement
>> with this patch. Hence, workloads that are truly latency-sensitive will 
>> benefit
>> quite a bit from this change. Moreover, this is an overall win-win since this
>> patch does not hurt power-savings at all (because, this patch does not reduce
>> the idle time or idle residency; and the high frequency of the CPU when it 
>> goes
>> to cpu-idle does not affect/hurt the power-savings of deep idle
>> states).
> 
> Are you sure about win-win?
> 
> AFAICT, your patch helps
> 
> ##.#.###...##
> 
> case (not surprising, that's why you wrote the patch :-), but what happens in
> 
> ##.#.#.#.
> 
> case? (That is idle system, with some tasks taking very small ammounts
> of CPU).
> 
> AFAICT, it will remember the (high) prev_load over the idle period,
> and use too high frequency for mostly idle system, no?
> 

Wow, great catch! I think the problem is that the prev-load is getting copied
over too many times; we need to restrict it to just one-copy and leave the
next sampling interval to the cpufreq governor's regular ramp-up, even if it
was an idle interval.

Below is an untested patch that implements that logic (and it also fixes the
64 bit division problem reported by Fengguang's build robot in the v2 of the
patch).

I'll test this patch and then send out the v3 later today. Please let me
know your thoughts!

Regards,
Srivatsa S. Bhat

---

diff --git a/drivers/cpufreq/cpufreq_governor.c 
b/drivers/cpufreq/cpufreq_governor.c
index e1c6433..3856a6b 100644
--- a/drivers/cpufreq/cpufreq_governor.c
+++ b/drivers/cpufreq/cpufreq_governor.c
@@ -36,14 +36,29 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu)
struct od_dbs_tuners *od_tuners = dbs_data->tuners;
struct cs_dbs_tuners *cs_tuners = dbs_data->tuners;
struct cpufreq_policy *policy;
+   unsigned int sampling_rate;
unsigned int max_load = 0;
unsigned int ignore_nice;
unsigned int j;
 
-   if (dbs_data->cdata->governor == GOV_ONDEMAND)
+   if (dbs_data->cdata->governor == GOV_ONDEMAND) {
+   struct od_cpu_dbs_info_s *od_dbs_info =
+   dbs_data->cdata->get_cpu_dbs_info_s(cpu);
+
+   /*
+* Sometimes, the ondemand governor uses an additional
+* multiplier to give long delays. So apply this multiplier to
+* the 'sampling_rate', so as to keep the wake-up-from-idle
+* detection logic a bit conservative.
+*/
+   sampling_rate = od_tuners->sampling_rate;
+   sampling_rate *= od_dbs_info->rate_mult;
+
ignore_nice = od_tuners->ignore_nice_load;
-   else
+   } else {
+   sampling_rate = cs_tuners->sampling_rate;
ignore_nice = cs_tuners->ignore_nice_load;
+   }
 
policy = cdbs->cur_policy;
 
@@ -96,7 +111,36 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu)

Re: [PATCH] cpufreq: governor: Be friendly towards latency-sensitive bursty workloads

[Pavel Machek]

Hi!

> We just want to avoid the stupidity of dropping down the frequency to a 
> minimum
> and then enduring a needless (and long) delay before ramping it up back again.
> So, let us simply carry forward the previous load - that is, let us just 
> pretend
> that the 'load' for the current time-window is the same as the load for the
> previous window. That way, the frequency and voltage will continue to be set
> to whatever values they were set at previously. This means that bursty 
> workloads
> will get a chance to influence the CPU frequency at which they wake up from
> cpu-idle, based on their past execution history. Thus, they might be able to
> avoid suffering from slow wakeups and long response-times.
> 
> [ The right way to solve this problem is to teach the CPU frequency governors
> to track load on a per-task basis, not a per-CPU basis, and set the 
> appropriate
> frequency on whichever CPU the task executes. But that involves redesigning
> the cpufreq subsystem, so this patch should make the situation bearable until
> then. ]

Are you sure? For example "./configure" load consists of a lot of
short-lived tasks. Per-task basis may not be option for that.

> A rudimentary and somewhat approximately latency-sensitive workload such as
> sleeping-ebizzy itself showed a consistent, noticeable performance improvement
> with this patch. Hence, workloads that are truly latency-sensitive will 
> benefit
> quite a bit from this change. Moreover, this is an overall win-win since this
> patch does not hurt power-savings at all (because, this patch does not reduce
> the idle time or idle residency; and the high frequency of the CPU when it 
> goes
> to cpu-idle does not affect/hurt the power-savings of deep idle
> states).

Are you sure about win-win?

AFAICT, your patch helps

##.#.###...##

case (not surprising, that's why you wrote the patch :-), but what happens in

##.#.#.#.

case? (That is idle system, with some tasks taking very small ammounts
of CPU).

AFAICT, it will remember the (high) prev_load over the idle period,
and use too high frequency for mostly idle system, no?

Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
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] cpufreq: governor: Be friendly towards latency-sensitive bursty workloads

[Pavel Machek]

Hi!

 We just want to avoid the stupidity of dropping down the frequency to a 
 minimum
 and then enduring a needless (and long) delay before ramping it up back again.
 So, let us simply carry forward the previous load - that is, let us just 
 pretend
 that the 'load' for the current time-window is the same as the load for the
 previous window. That way, the frequency and voltage will continue to be set
 to whatever values they were set at previously. This means that bursty 
 workloads
 will get a chance to influence the CPU frequency at which they wake up from
 cpu-idle, based on their past execution history. Thus, they might be able to
 avoid suffering from slow wakeups and long response-times.
 
 [ The right way to solve this problem is to teach the CPU frequency governors
 to track load on a per-task basis, not a per-CPU basis, and set the 
 appropriate
 frequency on whichever CPU the task executes. But that involves redesigning
 the cpufreq subsystem, so this patch should make the situation bearable until
 then. ]

Are you sure? For example ./configure load consists of a lot of
short-lived tasks. Per-task basis may not be option for that.

 A rudimentary and somewhat approximately latency-sensitive workload such as
 sleeping-ebizzy itself showed a consistent, noticeable performance improvement
 with this patch. Hence, workloads that are truly latency-sensitive will 
 benefit
 quite a bit from this change. Moreover, this is an overall win-win since this
 patch does not hurt power-savings at all (because, this patch does not reduce
 the idle time or idle residency; and the high frequency of the CPU when it 
 goes
 to cpu-idle does not affect/hurt the power-savings of deep idle
 states).

Are you sure about win-win?

AFAICT, your patch helps

##.#.###...##

case (not surprising, that's why you wrote the patch :-), but what happens in

##.#.#.#.

case? (That is idle system, with some tasks taking very small ammounts
of CPU).

AFAICT, it will remember the (high) prev_load over the idle period,
and use too high frequency for mostly idle system, no?

Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
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] cpufreq: governor: Be friendly towards latency-sensitive bursty workloads

[Srivatsa S. Bhat]

On 06/07/2014 03:25 PM, Pavel Machek wrote:
 Hi!
 
 We just want to avoid the stupidity of dropping down the frequency to a 
 minimum
 and then enduring a needless (and long) delay before ramping it up back 
 again.
 So, let us simply carry forward the previous load - that is, let us just 
 pretend
 that the 'load' for the current time-window is the same as the load for the
 previous window. That way, the frequency and voltage will continue to be set
 to whatever values they were set at previously. This means that bursty 
 workloads
 will get a chance to influence the CPU frequency at which they wake up from
 cpu-idle, based on their past execution history. Thus, they might be able to
 avoid suffering from slow wakeups and long response-times.

 [ The right way to solve this problem is to teach the CPU frequency governors
 to track load on a per-task basis, not a per-CPU basis, and set the 
 appropriate
 frequency on whichever CPU the task executes. But that involves redesigning
 the cpufreq subsystem, so this patch should make the situation bearable until
 then. ]
 
 Are you sure? For example ./configure load consists of a lot of
 short-lived tasks. Per-task basis may not be option for that.
 

True, but then there is no guarantee that all the tasks spawned by ./configure
run on a single CPU. So per-CPU tracking may or may not help, depending on the
scheduling pattern.

For very-short-lived tasks, per-task tracking won't give much benefit, but it
will for medium to long-lived tasks. So I guess we might need some sort of a
combination of both methods of tracking, to handle all the scenarios well.

 A rudimentary and somewhat approximately latency-sensitive workload such as
 sleeping-ebizzy itself showed a consistent, noticeable performance 
 improvement
 with this patch. Hence, workloads that are truly latency-sensitive will 
 benefit
 quite a bit from this change. Moreover, this is an overall win-win since this
 patch does not hurt power-savings at all (because, this patch does not reduce
 the idle time or idle residency; and the high frequency of the CPU when it 
 goes
 to cpu-idle does not affect/hurt the power-savings of deep idle
 states).
 
 Are you sure about win-win?
 
 AFAICT, your patch helps
 
 ##.#.###...##
 
 case (not surprising, that's why you wrote the patch :-), but what happens in
 
 ##.#.#.#.
 
 case? (That is idle system, with some tasks taking very small ammounts
 of CPU).
 
 AFAICT, it will remember the (high) prev_load over the idle period,
 and use too high frequency for mostly idle system, no?
 

Wow, great catch! I think the problem is that the prev-load is getting copied
over too many times; we need to restrict it to just one-copy and leave the
next sampling interval to the cpufreq governor's regular ramp-up, even if it
was an idle interval.

Below is an untested patch that implements that logic (and it also fixes the
64 bit division problem reported by Fengguang's build robot in the v2 of the
patch).

I'll test this patch and then send out the v3 later today. Please let me
know your thoughts!

Regards,
Srivatsa S. Bhat

---

diff --git a/drivers/cpufreq/cpufreq_governor.c 
b/drivers/cpufreq/cpufreq_governor.c
index e1c6433..3856a6b 100644
--- a/drivers/cpufreq/cpufreq_governor.c
+++ b/drivers/cpufreq/cpufreq_governor.c
@@ -36,14 +36,29 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu)
struct od_dbs_tuners *od_tuners = dbs_data-tuners;
struct cs_dbs_tuners *cs_tuners = dbs_data-tuners;
struct cpufreq_policy *policy;
+   unsigned int sampling_rate;
unsigned int max_load = 0;
unsigned int ignore_nice;
unsigned int j;
 
-   if (dbs_data-cdata-governor == GOV_ONDEMAND)
+   if (dbs_data-cdata-governor == GOV_ONDEMAND) {
+   struct od_cpu_dbs_info_s *od_dbs_info =
+   dbs_data-cdata-get_cpu_dbs_info_s(cpu);
+
+   /*
+* Sometimes, the ondemand governor uses an additional
+* multiplier to give long delays. So apply this multiplier to
+* the 'sampling_rate', so as to keep the wake-up-from-idle
+* detection logic a bit conservative.
+*/
+   sampling_rate = od_tuners-sampling_rate;
+   sampling_rate *= od_dbs_info-rate_mult;
+
ignore_nice = od_tuners-ignore_nice_load;
-   else
+   } else {
+   sampling_rate = cs_tuners-sampling_rate;
ignore_nice = cs_tuners-ignore_nice_load;
+   }
 
policy = cdbs-cur_policy;
 
@@ -96,7 +111,36 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu)
if (unlikely(!wall_time || wall_time  idle_time))
continue;
 
-   

Re: [PATCH] cpufreq: governor: Be friendly towards latency-sensitive bursty workloads

[Srivatsa S. Bhat]

On 06/03/2014 03:46 PM, Viresh Kumar wrote:
> On 3 June 2014 15:43, Srivatsa S. Bhat  
> wrote:
>> diff --git a/drivers/cpufreq/cpufreq_governor.c 
>> b/drivers/cpufreq/cpufreq_governor.c
>> index e1c6433..2597bbe 100644
>> --- a/drivers/cpufreq/cpufreq_governor.c
>> +++ b/drivers/cpufreq/cpufreq_governor.c
>> @@ -36,14 +36,29 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu)
>> struct od_dbs_tuners *od_tuners = dbs_data->tuners;
>> struct cs_dbs_tuners *cs_tuners = dbs_data->tuners;
>> struct cpufreq_policy *policy;
>> +   unsigned int sampling_rate;
>> unsigned int max_load = 0;
>> unsigned int ignore_nice;
>> unsigned int j;
[...]
>> diff --git a/drivers/cpufreq/cpufreq_governor.h 
>> b/drivers/cpufreq/cpufreq_governor.h
>> index bfb9ae1..b56552b 100644
>> --- a/drivers/cpufreq/cpufreq_governor.h
>> +++ b/drivers/cpufreq/cpufreq_governor.h
>> @@ -134,6 +134,7 @@ struct cpu_dbs_common_info {
>> u64 prev_cpu_idle;
>> u64 prev_cpu_wall;
>> u64 prev_cpu_nice;
>> +   unsigned int prev_load;
>> struct cpufreq_policy *cur_policy;
>> struct delayed_work work;
>> /*
> 
> Acked-by: Viresh Kumar 
> 

Thanks a lot!
 
Regards,
Srivatsa S. Bhat

--
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] cpufreq: governor: Be friendly towards latency-sensitive bursty workloads

[Viresh Kumar]

On 3 June 2014 15:43, Srivatsa S. Bhat  wrote:
> diff --git a/drivers/cpufreq/cpufreq_governor.c 
> b/drivers/cpufreq/cpufreq_governor.c
> index e1c6433..2597bbe 100644
> --- a/drivers/cpufreq/cpufreq_governor.c
> +++ b/drivers/cpufreq/cpufreq_governor.c
> @@ -36,14 +36,29 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu)
> struct od_dbs_tuners *od_tuners = dbs_data->tuners;
> struct cs_dbs_tuners *cs_tuners = dbs_data->tuners;
> struct cpufreq_policy *policy;
> +   unsigned int sampling_rate;
> unsigned int max_load = 0;
> unsigned int ignore_nice;
> unsigned int j;
>
> -   if (dbs_data->cdata->governor == GOV_ONDEMAND)
> +   if (dbs_data->cdata->governor == GOV_ONDEMAND) {
> +   struct od_cpu_dbs_info_s *od_dbs_info =
> +   dbs_data->cdata->get_cpu_dbs_info_s(cpu);
> +
> +   /*
> +* Sometimes, the ondemand governor uses an additional
> +* multiplier to give long delays. So apply this multiplier to
> +* the 'sampling_rate', so as to keep the wake-up-from-idle
> +* detection logic a bit conservative.
> +*/
> +   sampling_rate = od_tuners->sampling_rate;
> +   sampling_rate *= od_dbs_info->rate_mult;
> +
> ignore_nice = od_tuners->ignore_nice_load;
> -   else
> +   } else {
> +   sampling_rate = cs_tuners->sampling_rate;
> ignore_nice = cs_tuners->ignore_nice_load;
> +   }
>
> policy = cdbs->cur_policy;
>
> @@ -96,7 +111,29 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu)
> if (unlikely(!wall_time || wall_time < idle_time))
> continue;
>
> -   load = 100 * (wall_time - idle_time) / wall_time;
> +   /*
> +* If the CPU had gone completely idle, and a task just woke 
> up
> +* on this CPU now, it would be unfair to calculate 'load' the
> +* usual way for this elapsed time-window, because it will 
> show
> +* near-zero load, irrespective of how CPU intensive the new
> +* task is. This is undesirable for latency-sensitive bursty
> +* workloads.
> +*
> +* To avoid this, we reuse the 'load' from the previous
> +* time-window and give this task a chance to start with a
> +* reasonably high CPU frequency.
> +*
> +* Detecting this situation is easy: the governor's deferrable
> +* timer would not have fired during CPU-idle periods. Hence
> +* an unusually large 'wall_time' (as compared to the sampling
> +* rate) indicates this scenario.
> +*/
> +   if (unlikely(wall_time > (2 * sampling_rate))) {
> +   load = j_cdbs->prev_load;
> +   } else {
> +   load = 100 * (wall_time - idle_time) / wall_time;
> +   j_cdbs->prev_load = load;
> +   }
>
> if (load > max_load)
> max_load = load;
> @@ -323,6 +360,10 @@ int cpufreq_governor_dbs(struct cpufreq_policy *policy,
> j_cdbs->cur_policy = policy;
> j_cdbs->prev_cpu_idle = get_cpu_idle_time(j,
>_cdbs->prev_cpu_wall, 
> io_busy);
> +   j_cdbs->prev_load = 100 * (j_cdbs->prev_cpu_wall -
> +  j_cdbs->prev_cpu_idle) /
> +  j_cdbs->prev_cpu_wall;
> +
> if (ignore_nice)
> j_cdbs->prev_cpu_nice =
> kcpustat_cpu(j).cpustat[CPUTIME_NICE];
> diff --git a/drivers/cpufreq/cpufreq_governor.h 
> b/drivers/cpufreq/cpufreq_governor.h
> index bfb9ae1..b56552b 100644
> --- a/drivers/cpufreq/cpufreq_governor.h
> +++ b/drivers/cpufreq/cpufreq_governor.h
> @@ -134,6 +134,7 @@ struct cpu_dbs_common_info {
> u64 prev_cpu_idle;
> u64 prev_cpu_wall;
> u64 prev_cpu_nice;
> +   unsigned int prev_load;
> struct cpufreq_policy *cur_policy;
> struct delayed_work work;
> /*

Acked-by: Viresh Kumar 
--
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] cpufreq: governor: Be friendly towards latency-sensitive bursty workloads

[Srivatsa S. Bhat]

On 06/03/2014 03:38 PM, Viresh Kumar wrote:
> On 3 June 2014 15:34, Srivatsa S. Bhat  
> wrote:
>> Well, the method I used keeps the organization such that the code following
>> the comment does precisely what the comment says (i.e, get the sampling_rate,
>> fetch the multiplier, and then multiply). So I feel it makes it easier to
>> understand.
> 
> It looked like the comment is there only for this special statement:
> 
 +   sampling_rate *= od_dbs_info->rate_mult;
> 
> And so suggested that :)
> 
> Anyway move this up as it doesn't belong to comment for sure.
>>> +   od_dbs_info = dbs_data->cdata->get_cpu_dbs_info_s(cpu);
> 

Fair enough :) Here it is:


diff --git a/drivers/cpufreq/cpufreq_governor.c 
b/drivers/cpufreq/cpufreq_governor.c
index e1c6433..2597bbe 100644
--- a/drivers/cpufreq/cpufreq_governor.c
+++ b/drivers/cpufreq/cpufreq_governor.c
@@ -36,14 +36,29 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu)
struct od_dbs_tuners *od_tuners = dbs_data->tuners;
struct cs_dbs_tuners *cs_tuners = dbs_data->tuners;
struct cpufreq_policy *policy;
+   unsigned int sampling_rate;
unsigned int max_load = 0;
unsigned int ignore_nice;
unsigned int j;
 
-   if (dbs_data->cdata->governor == GOV_ONDEMAND)
+   if (dbs_data->cdata->governor == GOV_ONDEMAND) {
+   struct od_cpu_dbs_info_s *od_dbs_info =
+   dbs_data->cdata->get_cpu_dbs_info_s(cpu);
+
+   /*
+* Sometimes, the ondemand governor uses an additional
+* multiplier to give long delays. So apply this multiplier to
+* the 'sampling_rate', so as to keep the wake-up-from-idle
+* detection logic a bit conservative.
+*/
+   sampling_rate = od_tuners->sampling_rate;
+   sampling_rate *= od_dbs_info->rate_mult;
+
ignore_nice = od_tuners->ignore_nice_load;
-   else
+   } else {
+   sampling_rate = cs_tuners->sampling_rate;
ignore_nice = cs_tuners->ignore_nice_load;
+   }
 
policy = cdbs->cur_policy;
 
@@ -96,7 +111,29 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu)
if (unlikely(!wall_time || wall_time < idle_time))
continue;
 
-   load = 100 * (wall_time - idle_time) / wall_time;
+   /*
+* If the CPU had gone completely idle, and a task just woke up
+* on this CPU now, it would be unfair to calculate 'load' the
+* usual way for this elapsed time-window, because it will show
+* near-zero load, irrespective of how CPU intensive the new
+* task is. This is undesirable for latency-sensitive bursty
+* workloads.
+*
+* To avoid this, we reuse the 'load' from the previous
+* time-window and give this task a chance to start with a
+* reasonably high CPU frequency.
+*
+* Detecting this situation is easy: the governor's deferrable
+* timer would not have fired during CPU-idle periods. Hence
+* an unusually large 'wall_time' (as compared to the sampling
+* rate) indicates this scenario.
+*/
+   if (unlikely(wall_time > (2 * sampling_rate))) {
+   load = j_cdbs->prev_load;
+   } else {
+   load = 100 * (wall_time - idle_time) / wall_time;
+   j_cdbs->prev_load = load;
+   }
 
if (load > max_load)
max_load = load;
@@ -323,6 +360,10 @@ int cpufreq_governor_dbs(struct cpufreq_policy *policy,
j_cdbs->cur_policy = policy;
j_cdbs->prev_cpu_idle = get_cpu_idle_time(j,
   _cdbs->prev_cpu_wall, io_busy);
+   j_cdbs->prev_load = 100 * (j_cdbs->prev_cpu_wall -
+  j_cdbs->prev_cpu_idle) /
+  j_cdbs->prev_cpu_wall;
+
if (ignore_nice)
j_cdbs->prev_cpu_nice =
kcpustat_cpu(j).cpustat[CPUTIME_NICE];
diff --git a/drivers/cpufreq/cpufreq_governor.h 
b/drivers/cpufreq/cpufreq_governor.h
index bfb9ae1..b56552b 100644
--- a/drivers/cpufreq/cpufreq_governor.h
+++ b/drivers/cpufreq/cpufreq_governor.h
@@ -134,6 +134,7 @@ struct cpu_dbs_common_info {
u64 prev_cpu_idle;
u64 prev_cpu_wall;
u64 prev_cpu_nice;
+   unsigned int prev_load;
struct cpufreq_policy *cur_policy;
struct delayed_work work;
/*

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body 

Re: [PATCH] cpufreq: governor: Be friendly towards latency-sensitive bursty workloads

[Viresh Kumar]

On 3 June 2014 15:34, Srivatsa S. Bhat  wrote:
> Well, the method I used keeps the organization such that the code following
> the comment does precisely what the comment says (i.e, get the sampling_rate,
> fetch the multiplier, and then multiply). So I feel it makes it easier to
> understand.

It looked like the comment is there only for this special statement:

>>> +   sampling_rate *= od_dbs_info->rate_mult;

And so suggested that :)

Anyway move this up as it doesn't belong to comment for sure.
>> +   od_dbs_info = dbs_data->cdata->get_cpu_dbs_info_s(cpu);
--
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] cpufreq: governor: Be friendly towards latency-sensitive bursty workloads

[Srivatsa S. Bhat]

On 06/03/2014 03:09 PM, Viresh Kumar wrote:
> On 3 June 2014 15:02, Srivatsa S. Bhat  
> wrote:
>> diff --git a/drivers/cpufreq/cpufreq_governor.c 
>> b/drivers/cpufreq/cpufreq_governor.c
>> index e1c6433..3e8588f 100644
>> --- a/drivers/cpufreq/cpufreq_governor.c
>> +++ b/drivers/cpufreq/cpufreq_governor.c
>> @@ -36,14 +36,29 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu)
>> struct od_dbs_tuners *od_tuners = dbs_data->tuners;
>> struct cs_dbs_tuners *cs_tuners = dbs_data->tuners;
>> struct cpufreq_policy *policy;
>> +   unsigned int sampling_rate;
>> unsigned int max_load = 0;
>> unsigned int ignore_nice;
>> unsigned int j;
>>
>> -   if (dbs_data->cdata->governor == GOV_ONDEMAND)
>> +   if (dbs_data->cdata->governor == GOV_ONDEMAND) {
>> +   struct od_cpu_dbs_info_s *od_dbs_info;
>> +
>> +   /*
>> +* Sometimes, the ondemand governor uses an additional
>> +* multiplier to give long delays. So apply this multiplier 
>> to
>> +* the 'sampling_rate', so as to keep the wake-up-from-idle
>> +* detection logic a bit conservative.
>> +*/
>> +   sampling_rate = od_tuners->sampling_rate;
>> +   od_dbs_info = dbs_data->cdata->get_cpu_dbs_info_s(cpu);
> 
> Probably do both above right after definition of od_dbs_info or merge
> above with it. and just keep below after the comment ?
> 
>> +   sampling_rate *= od_dbs_info->rate_mult;
> 

Well, the method I used keeps the organization such that the code following
the comment does precisely what the comment says (i.e, get the sampling_rate,
fetch the multiplier, and then multiply). So I feel it makes it easier to
understand.

Regards,
Srivatsa S. Bhat

--
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] cpufreq: governor: Be friendly towards latency-sensitive bursty workloads

[Viresh Kumar]

On 3 June 2014 15:02, Srivatsa S. Bhat  wrote:
> diff --git a/drivers/cpufreq/cpufreq_governor.c 
> b/drivers/cpufreq/cpufreq_governor.c
> index e1c6433..3e8588f 100644
> --- a/drivers/cpufreq/cpufreq_governor.c
> +++ b/drivers/cpufreq/cpufreq_governor.c
> @@ -36,14 +36,29 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu)
> struct od_dbs_tuners *od_tuners = dbs_data->tuners;
> struct cs_dbs_tuners *cs_tuners = dbs_data->tuners;
> struct cpufreq_policy *policy;
> +   unsigned int sampling_rate;
> unsigned int max_load = 0;
> unsigned int ignore_nice;
> unsigned int j;
>
> -   if (dbs_data->cdata->governor == GOV_ONDEMAND)
> +   if (dbs_data->cdata->governor == GOV_ONDEMAND) {
> +   struct od_cpu_dbs_info_s *od_dbs_info;
> +
> +   /*
> +* Sometimes, the ondemand governor uses an additional
> +* multiplier to give long delays. So apply this multiplier to
> +* the 'sampling_rate', so as to keep the wake-up-from-idle
> +* detection logic a bit conservative.
> +*/
> +   sampling_rate = od_tuners->sampling_rate;
> +   od_dbs_info = dbs_data->cdata->get_cpu_dbs_info_s(cpu);

Probably do both above right after definition of od_dbs_info or merge
above with it. and just keep below after the comment ?

> +   sampling_rate *= od_dbs_info->rate_mult;
--
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] cpufreq: governor: Be friendly towards latency-sensitive bursty workloads

[Srivatsa S. Bhat]

On 06/03/2014 01:48 PM, Viresh Kumar wrote:
> On 27 May 2014 02:23, Srivatsa S. Bhat  
> wrote:
> 
> Looks fine, some nits..
> 
>> diff --git a/drivers/cpufreq/cpufreq_governor.c 
>> b/drivers/cpufreq/cpufreq_governor.c
>> -void dbs_check_cpu(struct dbs_data *dbs_data, int cpu)
>> +void dbs_check_cpu(struct dbs_data *dbs_data, int cpu,
>> +  unsigned int sampling_rate)
> 
> We don't need to pass a new argument, we can get all the information from
> dbs_data alone. Its already done for multiple routines. Let me know if you
> find it difficult to figure out..
> 

Sure, that would be a good improvement. Does something like the patch below
look good? I have only compile-tested it. I'll send out the patch with changelog
once I finish testing it.

Thank you!

Regards,
Srivatsa S. Bhat



diff --git a/drivers/cpufreq/cpufreq_governor.c 
b/drivers/cpufreq/cpufreq_governor.c
index e1c6433..3e8588f 100644
--- a/drivers/cpufreq/cpufreq_governor.c
+++ b/drivers/cpufreq/cpufreq_governor.c
@@ -36,14 +36,29 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu)
struct od_dbs_tuners *od_tuners = dbs_data->tuners;
struct cs_dbs_tuners *cs_tuners = dbs_data->tuners;
struct cpufreq_policy *policy;
+   unsigned int sampling_rate;
unsigned int max_load = 0;
unsigned int ignore_nice;
unsigned int j;
 
-   if (dbs_data->cdata->governor == GOV_ONDEMAND)
+   if (dbs_data->cdata->governor == GOV_ONDEMAND) {
+   struct od_cpu_dbs_info_s *od_dbs_info;
+
+   /*
+* Sometimes, the ondemand governor uses an additional
+* multiplier to give long delays. So apply this multiplier to
+* the 'sampling_rate', so as to keep the wake-up-from-idle
+* detection logic a bit conservative.
+*/
+   sampling_rate = od_tuners->sampling_rate;
+   od_dbs_info = dbs_data->cdata->get_cpu_dbs_info_s(cpu);
+   sampling_rate *= od_dbs_info->rate_mult;
+
ignore_nice = od_tuners->ignore_nice_load;
-   else
+   } else {
+   sampling_rate = cs_tuners->sampling_rate;
ignore_nice = cs_tuners->ignore_nice_load;
+   }
 
policy = cdbs->cur_policy;
 
@@ -96,7 +111,29 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu)
if (unlikely(!wall_time || wall_time < idle_time))
continue;
 
-   load = 100 * (wall_time - idle_time) / wall_time;
+   /*
+* If the CPU had gone completely idle, and a task just woke up
+* on this CPU now, it would be unfair to calculate 'load' the
+* usual way for this elapsed time-window, because it will show
+* near-zero load, irrespective of how CPU intensive the new
+* task is. This is undesirable for latency-sensitive bursty
+* workloads.
+*
+* To avoid this, we reuse the 'load' from the previous
+* time-window and give this task a chance to start with a
+* reasonably high CPU frequency.
+*
+* Detecting this situation is easy: the governor's deferrable
+* timer would not have fired during CPU-idle periods. Hence
+* an unusually large 'wall_time' (as compared to the sampling
+* rate) indicates this scenario.
+*/
+   if (unlikely(wall_time > (2 * sampling_rate))) {
+   load = j_cdbs->prev_load;
+   } else {
+   load = 100 * (wall_time - idle_time) / wall_time;
+   j_cdbs->prev_load = load;
+   }
 
if (load > max_load)
max_load = load;
@@ -323,6 +360,10 @@ int cpufreq_governor_dbs(struct cpufreq_policy *policy,
j_cdbs->cur_policy = policy;
j_cdbs->prev_cpu_idle = get_cpu_idle_time(j,
   _cdbs->prev_cpu_wall, io_busy);
+   j_cdbs->prev_load = 100 * (j_cdbs->prev_cpu_wall -
+  j_cdbs->prev_cpu_idle) /
+  j_cdbs->prev_cpu_wall;
+
if (ignore_nice)
j_cdbs->prev_cpu_nice =
kcpustat_cpu(j).cpustat[CPUTIME_NICE];
diff --git a/drivers/cpufreq/cpufreq_governor.h 
b/drivers/cpufreq/cpufreq_governor.h
index bfb9ae1..b56552b 100644
--- a/drivers/cpufreq/cpufreq_governor.h
+++ b/drivers/cpufreq/cpufreq_governor.h
@@ -134,6 +134,7 @@ struct cpu_dbs_common_info {
u64 prev_cpu_idle;
u64 prev_cpu_wall;
u64 prev_cpu_nice;
+   unsigned int 

Re: [PATCH] cpufreq: governor: Be friendly towards latency-sensitive bursty workloads

[Viresh Kumar]

On 27 May 2014 02:23, Srivatsa S. Bhat  wrote:

Looks fine, some nits..

> diff --git a/drivers/cpufreq/cpufreq_governor.c 
> b/drivers/cpufreq/cpufreq_governor.c
> -void dbs_check_cpu(struct dbs_data *dbs_data, int cpu)
> +void dbs_check_cpu(struct dbs_data *dbs_data, int cpu,
> +  unsigned int sampling_rate)

We don't need to pass a new argument, we can get all the information from
dbs_data alone. Its already done for multiple routines. Let me know if you
find it difficult to figure out..
--
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] cpufreq: governor: Be friendly towards latency-sensitive bursty workloads

[Viresh Kumar]

On 27 May 2014 02:23, Srivatsa S. Bhat srivatsa.b...@linux.vnet.ibm.com wrote:

Looks fine, some nits..

 diff --git a/drivers/cpufreq/cpufreq_governor.c 
 b/drivers/cpufreq/cpufreq_governor.c
 -void dbs_check_cpu(struct dbs_data *dbs_data, int cpu)
 +void dbs_check_cpu(struct dbs_data *dbs_data, int cpu,
 +  unsigned int sampling_rate)

We don't need to pass a new argument, we can get all the information from
dbs_data alone. Its already done for multiple routines. Let me know if you
find it difficult to figure out..
--
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] cpufreq: governor: Be friendly towards latency-sensitive bursty workloads

[Srivatsa S. Bhat]

On 06/03/2014 01:48 PM, Viresh Kumar wrote:
 On 27 May 2014 02:23, Srivatsa S. Bhat srivatsa.b...@linux.vnet.ibm.com 
 wrote:
 
 Looks fine, some nits..
 
 diff --git a/drivers/cpufreq/cpufreq_governor.c 
 b/drivers/cpufreq/cpufreq_governor.c
 -void dbs_check_cpu(struct dbs_data *dbs_data, int cpu)
 +void dbs_check_cpu(struct dbs_data *dbs_data, int cpu,
 +  unsigned int sampling_rate)
 
 We don't need to pass a new argument, we can get all the information from
 dbs_data alone. Its already done for multiple routines. Let me know if you
 find it difficult to figure out..
 

Sure, that would be a good improvement. Does something like the patch below
look good? I have only compile-tested it. I'll send out the patch with changelog
once I finish testing it.

Thank you!

Regards,
Srivatsa S. Bhat



diff --git a/drivers/cpufreq/cpufreq_governor.c 
b/drivers/cpufreq/cpufreq_governor.c
index e1c6433..3e8588f 100644
--- a/drivers/cpufreq/cpufreq_governor.c
+++ b/drivers/cpufreq/cpufreq_governor.c
@@ -36,14 +36,29 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu)
struct od_dbs_tuners *od_tuners = dbs_data-tuners;
struct cs_dbs_tuners *cs_tuners = dbs_data-tuners;
struct cpufreq_policy *policy;
+   unsigned int sampling_rate;
unsigned int max_load = 0;
unsigned int ignore_nice;
unsigned int j;
 
-   if (dbs_data-cdata-governor == GOV_ONDEMAND)
+   if (dbs_data-cdata-governor == GOV_ONDEMAND) {
+   struct od_cpu_dbs_info_s *od_dbs_info;
+
+   /*
+* Sometimes, the ondemand governor uses an additional
+* multiplier to give long delays. So apply this multiplier to
+* the 'sampling_rate', so as to keep the wake-up-from-idle
+* detection logic a bit conservative.
+*/
+   sampling_rate = od_tuners-sampling_rate;
+   od_dbs_info = dbs_data-cdata-get_cpu_dbs_info_s(cpu);
+   sampling_rate *= od_dbs_info-rate_mult;
+
ignore_nice = od_tuners-ignore_nice_load;
-   else
+   } else {
+   sampling_rate = cs_tuners-sampling_rate;
ignore_nice = cs_tuners-ignore_nice_load;
+   }
 
policy = cdbs-cur_policy;
 
@@ -96,7 +111,29 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu)
if (unlikely(!wall_time || wall_time  idle_time))
continue;
 
-   load = 100 * (wall_time - idle_time) / wall_time;
+   /*
+* If the CPU had gone completely idle, and a task just woke up
+* on this CPU now, it would be unfair to calculate 'load' the
+* usual way for this elapsed time-window, because it will show
+* near-zero load, irrespective of how CPU intensive the new
+* task is. This is undesirable for latency-sensitive bursty
+* workloads.
+*
+* To avoid this, we reuse the 'load' from the previous
+* time-window and give this task a chance to start with a
+* reasonably high CPU frequency.
+*
+* Detecting this situation is easy: the governor's deferrable
+* timer would not have fired during CPU-idle periods. Hence
+* an unusually large 'wall_time' (as compared to the sampling
+* rate) indicates this scenario.
+*/
+   if (unlikely(wall_time  (2 * sampling_rate))) {
+   load = j_cdbs-prev_load;
+   } else {
+   load = 100 * (wall_time - idle_time) / wall_time;
+   j_cdbs-prev_load = load;
+   }
 
if (load  max_load)
max_load = load;
@@ -323,6 +360,10 @@ int cpufreq_governor_dbs(struct cpufreq_policy *policy,
j_cdbs-cur_policy = policy;
j_cdbs-prev_cpu_idle = get_cpu_idle_time(j,
   j_cdbs-prev_cpu_wall, io_busy);
+   j_cdbs-prev_load = 100 * (j_cdbs-prev_cpu_wall -
+  j_cdbs-prev_cpu_idle) /
+  j_cdbs-prev_cpu_wall;
+
if (ignore_nice)
j_cdbs-prev_cpu_nice =
kcpustat_cpu(j).cpustat[CPUTIME_NICE];
diff --git a/drivers/cpufreq/cpufreq_governor.h 
b/drivers/cpufreq/cpufreq_governor.h
index bfb9ae1..b56552b 100644
--- a/drivers/cpufreq/cpufreq_governor.h
+++ b/drivers/cpufreq/cpufreq_governor.h
@@ -134,6 +134,7 @@ struct cpu_dbs_common_info {
u64 prev_cpu_idle;
u64 prev_cpu_wall;
u64 prev_cpu_nice;
+   unsigned int prev_load;
   

Re: [PATCH] cpufreq: governor: Be friendly towards latency-sensitive bursty workloads

[Viresh Kumar]

On 3 June 2014 15:02, Srivatsa S. Bhat srivatsa.b...@linux.vnet.ibm.com wrote:
 diff --git a/drivers/cpufreq/cpufreq_governor.c 
 b/drivers/cpufreq/cpufreq_governor.c
 index e1c6433..3e8588f 100644
 --- a/drivers/cpufreq/cpufreq_governor.c
 +++ b/drivers/cpufreq/cpufreq_governor.c
 @@ -36,14 +36,29 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu)
 struct od_dbs_tuners *od_tuners = dbs_data-tuners;
 struct cs_dbs_tuners *cs_tuners = dbs_data-tuners;
 struct cpufreq_policy *policy;
 +   unsigned int sampling_rate;
 unsigned int max_load = 0;
 unsigned int ignore_nice;
 unsigned int j;

 -   if (dbs_data-cdata-governor == GOV_ONDEMAND)
 +   if (dbs_data-cdata-governor == GOV_ONDEMAND) {
 +   struct od_cpu_dbs_info_s *od_dbs_info;
 +
 +   /*
 +* Sometimes, the ondemand governor uses an additional
 +* multiplier to give long delays. So apply this multiplier to
 +* the 'sampling_rate', so as to keep the wake-up-from-idle
 +* detection logic a bit conservative.
 +*/
 +   sampling_rate = od_tuners-sampling_rate;
 +   od_dbs_info = dbs_data-cdata-get_cpu_dbs_info_s(cpu);

Probably do both above right after definition of od_dbs_info or merge
above with it. and just keep below after the comment ?

 +   sampling_rate *= od_dbs_info-rate_mult;
--
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] cpufreq: governor: Be friendly towards latency-sensitive bursty workloads

[Srivatsa S. Bhat]

On 06/03/2014 03:09 PM, Viresh Kumar wrote:
 On 3 June 2014 15:02, Srivatsa S. Bhat srivatsa.b...@linux.vnet.ibm.com 
 wrote:
 diff --git a/drivers/cpufreq/cpufreq_governor.c 
 b/drivers/cpufreq/cpufreq_governor.c
 index e1c6433..3e8588f 100644
 --- a/drivers/cpufreq/cpufreq_governor.c
 +++ b/drivers/cpufreq/cpufreq_governor.c
 @@ -36,14 +36,29 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu)
 struct od_dbs_tuners *od_tuners = dbs_data-tuners;
 struct cs_dbs_tuners *cs_tuners = dbs_data-tuners;
 struct cpufreq_policy *policy;
 +   unsigned int sampling_rate;
 unsigned int max_load = 0;
 unsigned int ignore_nice;
 unsigned int j;

 -   if (dbs_data-cdata-governor == GOV_ONDEMAND)
 +   if (dbs_data-cdata-governor == GOV_ONDEMAND) {
 +   struct od_cpu_dbs_info_s *od_dbs_info;
 +
 +   /*
 +* Sometimes, the ondemand governor uses an additional
 +* multiplier to give long delays. So apply this multiplier 
 to
 +* the 'sampling_rate', so as to keep the wake-up-from-idle
 +* detection logic a bit conservative.
 +*/
 +   sampling_rate = od_tuners-sampling_rate;
 +   od_dbs_info = dbs_data-cdata-get_cpu_dbs_info_s(cpu);
 
 Probably do both above right after definition of od_dbs_info or merge
 above with it. and just keep below after the comment ?
 
 +   sampling_rate *= od_dbs_info-rate_mult;
 

Well, the method I used keeps the organization such that the code following
the comment does precisely what the comment says (i.e, get the sampling_rate,
fetch the multiplier, and then multiply). So I feel it makes it easier to
understand.

Regards,
Srivatsa S. Bhat

--
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] cpufreq: governor: Be friendly towards latency-sensitive bursty workloads

[Viresh Kumar]

On 3 June 2014 15:34, Srivatsa S. Bhat srivatsa.b...@linux.vnet.ibm.com wrote:
 Well, the method I used keeps the organization such that the code following
 the comment does precisely what the comment says (i.e, get the sampling_rate,
 fetch the multiplier, and then multiply). So I feel it makes it easier to
 understand.

It looked like the comment is there only for this special statement:

 +   sampling_rate *= od_dbs_info-rate_mult;

And so suggested that :)

Anyway move this up as it doesn't belong to comment for sure.
 +   od_dbs_info = dbs_data-cdata-get_cpu_dbs_info_s(cpu);
--
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] cpufreq: governor: Be friendly towards latency-sensitive bursty workloads

[Srivatsa S. Bhat]

On 06/03/2014 03:38 PM, Viresh Kumar wrote:
 On 3 June 2014 15:34, Srivatsa S. Bhat srivatsa.b...@linux.vnet.ibm.com 
 wrote:
 Well, the method I used keeps the organization such that the code following
 the comment does precisely what the comment says (i.e, get the sampling_rate,
 fetch the multiplier, and then multiply). So I feel it makes it easier to
 understand.
 
 It looked like the comment is there only for this special statement:
 
 +   sampling_rate *= od_dbs_info-rate_mult;
 
 And so suggested that :)
 
 Anyway move this up as it doesn't belong to comment for sure.
 +   od_dbs_info = dbs_data-cdata-get_cpu_dbs_info_s(cpu);
 

Fair enough :) Here it is:


diff --git a/drivers/cpufreq/cpufreq_governor.c 
b/drivers/cpufreq/cpufreq_governor.c
index e1c6433..2597bbe 100644
--- a/drivers/cpufreq/cpufreq_governor.c
+++ b/drivers/cpufreq/cpufreq_governor.c
@@ -36,14 +36,29 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu)
struct od_dbs_tuners *od_tuners = dbs_data-tuners;
struct cs_dbs_tuners *cs_tuners = dbs_data-tuners;
struct cpufreq_policy *policy;
+   unsigned int sampling_rate;
unsigned int max_load = 0;
unsigned int ignore_nice;
unsigned int j;
 
-   if (dbs_data-cdata-governor == GOV_ONDEMAND)
+   if (dbs_data-cdata-governor == GOV_ONDEMAND) {
+   struct od_cpu_dbs_info_s *od_dbs_info =
+   dbs_data-cdata-get_cpu_dbs_info_s(cpu);
+
+   /*
+* Sometimes, the ondemand governor uses an additional
+* multiplier to give long delays. So apply this multiplier to
+* the 'sampling_rate', so as to keep the wake-up-from-idle
+* detection logic a bit conservative.
+*/
+   sampling_rate = od_tuners-sampling_rate;
+   sampling_rate *= od_dbs_info-rate_mult;
+
ignore_nice = od_tuners-ignore_nice_load;
-   else
+   } else {
+   sampling_rate = cs_tuners-sampling_rate;
ignore_nice = cs_tuners-ignore_nice_load;
+   }
 
policy = cdbs-cur_policy;
 
@@ -96,7 +111,29 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu)
if (unlikely(!wall_time || wall_time  idle_time))
continue;
 
-   load = 100 * (wall_time - idle_time) / wall_time;
+   /*
+* If the CPU had gone completely idle, and a task just woke up
+* on this CPU now, it would be unfair to calculate 'load' the
+* usual way for this elapsed time-window, because it will show
+* near-zero load, irrespective of how CPU intensive the new
+* task is. This is undesirable for latency-sensitive bursty
+* workloads.
+*
+* To avoid this, we reuse the 'load' from the previous
+* time-window and give this task a chance to start with a
+* reasonably high CPU frequency.
+*
+* Detecting this situation is easy: the governor's deferrable
+* timer would not have fired during CPU-idle periods. Hence
+* an unusually large 'wall_time' (as compared to the sampling
+* rate) indicates this scenario.
+*/
+   if (unlikely(wall_time  (2 * sampling_rate))) {
+   load = j_cdbs-prev_load;
+   } else {
+   load = 100 * (wall_time - idle_time) / wall_time;
+   j_cdbs-prev_load = load;
+   }
 
if (load  max_load)
max_load = load;
@@ -323,6 +360,10 @@ int cpufreq_governor_dbs(struct cpufreq_policy *policy,
j_cdbs-cur_policy = policy;
j_cdbs-prev_cpu_idle = get_cpu_idle_time(j,
   j_cdbs-prev_cpu_wall, io_busy);
+   j_cdbs-prev_load = 100 * (j_cdbs-prev_cpu_wall -
+  j_cdbs-prev_cpu_idle) /
+  j_cdbs-prev_cpu_wall;
+
if (ignore_nice)
j_cdbs-prev_cpu_nice =
kcpustat_cpu(j).cpustat[CPUTIME_NICE];
diff --git a/drivers/cpufreq/cpufreq_governor.h 
b/drivers/cpufreq/cpufreq_governor.h
index bfb9ae1..b56552b 100644
--- a/drivers/cpufreq/cpufreq_governor.h
+++ b/drivers/cpufreq/cpufreq_governor.h
@@ -134,6 +134,7 @@ struct cpu_dbs_common_info {
u64 prev_cpu_idle;
u64 prev_cpu_wall;
u64 prev_cpu_nice;
+   unsigned int prev_load;
struct cpufreq_policy *cur_policy;
struct delayed_work work;
/*

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to 

Re: [PATCH] cpufreq: governor: Be friendly towards latency-sensitive bursty workloads

[Viresh Kumar]

On 3 June 2014 15:43, Srivatsa S. Bhat srivatsa.b...@linux.vnet.ibm.com wrote:
 diff --git a/drivers/cpufreq/cpufreq_governor.c 
 b/drivers/cpufreq/cpufreq_governor.c
 index e1c6433..2597bbe 100644
 --- a/drivers/cpufreq/cpufreq_governor.c
 +++ b/drivers/cpufreq/cpufreq_governor.c
 @@ -36,14 +36,29 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu)
 struct od_dbs_tuners *od_tuners = dbs_data-tuners;
 struct cs_dbs_tuners *cs_tuners = dbs_data-tuners;
 struct cpufreq_policy *policy;
 +   unsigned int sampling_rate;
 unsigned int max_load = 0;
 unsigned int ignore_nice;
 unsigned int j;

 -   if (dbs_data-cdata-governor == GOV_ONDEMAND)
 +   if (dbs_data-cdata-governor == GOV_ONDEMAND) {
 +   struct od_cpu_dbs_info_s *od_dbs_info =
 +   dbs_data-cdata-get_cpu_dbs_info_s(cpu);
 +
 +   /*
 +* Sometimes, the ondemand governor uses an additional
 +* multiplier to give long delays. So apply this multiplier to
 +* the 'sampling_rate', so as to keep the wake-up-from-idle
 +* detection logic a bit conservative.
 +*/
 +   sampling_rate = od_tuners-sampling_rate;
 +   sampling_rate *= od_dbs_info-rate_mult;
 +
 ignore_nice = od_tuners-ignore_nice_load;
 -   else
 +   } else {
 +   sampling_rate = cs_tuners-sampling_rate;
 ignore_nice = cs_tuners-ignore_nice_load;
 +   }

 policy = cdbs-cur_policy;

 @@ -96,7 +111,29 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu)
 if (unlikely(!wall_time || wall_time  idle_time))
 continue;

 -   load = 100 * (wall_time - idle_time) / wall_time;
 +   /*
 +* If the CPU had gone completely idle, and a task just woke 
 up
 +* on this CPU now, it would be unfair to calculate 'load' the
 +* usual way for this elapsed time-window, because it will 
 show
 +* near-zero load, irrespective of how CPU intensive the new
 +* task is. This is undesirable for latency-sensitive bursty
 +* workloads.
 +*
 +* To avoid this, we reuse the 'load' from the previous
 +* time-window and give this task a chance to start with a
 +* reasonably high CPU frequency.
 +*
 +* Detecting this situation is easy: the governor's deferrable
 +* timer would not have fired during CPU-idle periods. Hence
 +* an unusually large 'wall_time' (as compared to the sampling
 +* rate) indicates this scenario.
 +*/
 +   if (unlikely(wall_time  (2 * sampling_rate))) {
 +   load = j_cdbs-prev_load;
 +   } else {
 +   load = 100 * (wall_time - idle_time) / wall_time;
 +   j_cdbs-prev_load = load;
 +   }

 if (load  max_load)
 max_load = load;
 @@ -323,6 +360,10 @@ int cpufreq_governor_dbs(struct cpufreq_policy *policy,
 j_cdbs-cur_policy = policy;
 j_cdbs-prev_cpu_idle = get_cpu_idle_time(j,
j_cdbs-prev_cpu_wall, 
 io_busy);
 +   j_cdbs-prev_load = 100 * (j_cdbs-prev_cpu_wall -
 +  j_cdbs-prev_cpu_idle) /
 +  j_cdbs-prev_cpu_wall;
 +
 if (ignore_nice)
 j_cdbs-prev_cpu_nice =
 kcpustat_cpu(j).cpustat[CPUTIME_NICE];
 diff --git a/drivers/cpufreq/cpufreq_governor.h 
 b/drivers/cpufreq/cpufreq_governor.h
 index bfb9ae1..b56552b 100644
 --- a/drivers/cpufreq/cpufreq_governor.h
 +++ b/drivers/cpufreq/cpufreq_governor.h
 @@ -134,6 +134,7 @@ struct cpu_dbs_common_info {
 u64 prev_cpu_idle;
 u64 prev_cpu_wall;
 u64 prev_cpu_nice;
 +   unsigned int prev_load;
 struct cpufreq_policy *cur_policy;
 struct delayed_work work;
 /*

Acked-by: Viresh Kumar viresh.ku...@linaro.org
--
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] cpufreq: governor: Be friendly towards latency-sensitive bursty workloads

[Srivatsa S. Bhat]

On 06/03/2014 03:46 PM, Viresh Kumar wrote:
 On 3 June 2014 15:43, Srivatsa S. Bhat srivatsa.b...@linux.vnet.ibm.com 
 wrote:
 diff --git a/drivers/cpufreq/cpufreq_governor.c 
 b/drivers/cpufreq/cpufreq_governor.c
 index e1c6433..2597bbe 100644
 --- a/drivers/cpufreq/cpufreq_governor.c
 +++ b/drivers/cpufreq/cpufreq_governor.c
 @@ -36,14 +36,29 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu)
 struct od_dbs_tuners *od_tuners = dbs_data-tuners;
 struct cs_dbs_tuners *cs_tuners = dbs_data-tuners;
 struct cpufreq_policy *policy;
 +   unsigned int sampling_rate;
 unsigned int max_load = 0;
 unsigned int ignore_nice;
 unsigned int j;
[...]
 diff --git a/drivers/cpufreq/cpufreq_governor.h 
 b/drivers/cpufreq/cpufreq_governor.h
 index bfb9ae1..b56552b 100644
 --- a/drivers/cpufreq/cpufreq_governor.h
 +++ b/drivers/cpufreq/cpufreq_governor.h
 @@ -134,6 +134,7 @@ struct cpu_dbs_common_info {
 u64 prev_cpu_idle;
 u64 prev_cpu_wall;
 u64 prev_cpu_nice;
 +   unsigned int prev_load;
 struct cpufreq_policy *cur_policy;
 struct delayed_work work;
 /*
 
 Acked-by: Viresh Kumar viresh.ku...@linaro.org
 

Thanks a lot!
 
Regards,
Srivatsa S. Bhat

--
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] cpufreq: governor: Be friendly towards latency-sensitive bursty workloads

[Srivatsa S. Bhat]

On 06/03/2014 10:46 AM, Gautham R Shenoy wrote:
> On Mon, Jun 02, 2014 at 01:45:38PM +0530, Srivatsa S. Bhat wrote:
>> On 06/02/2014 01:03 PM, Gautham R Shenoy wrote:
>>> Hi,
>>>
>>> On Tue, May 27, 2014 at 02:23:38AM +0530, Srivatsa S. Bhat wrote:
>>>
>>> [..snip..]

 Experimental results:
 

 I ran a modified version of ebizzy (called 'sleeping-ebizzy') that sleeps 
 in
 between its execution such that its total utilization can be a user-defined
 value, say 10% or 20% (higher the utilization specified, lesser the amount 
 of
 sleeps injected). This ebizzy was run with a single-thread, tied to CPU 8.

 Behavior observed with tracing (sample taken from 40% utilization runs):
 

 Without patch:
 ~~
 kworker/8:2-12137  416.335742: cpu_frequency: state=2061000 cpu_id=8
 kworker/8:2-12137  416.335744: sched_switch: prev_comm=kworker/8:2 ==> 
 next_comm=ebizzy
   <...>-40753  416.345741: sched_switch: prev_comm=ebizzy ==> 
 next_comm=kworker/8:2
 kworker/8:2-12137  416.345744: cpu_frequency: state=4123000 cpu_id=8
 kworker/8:2-12137  416.345746: sched_switch: prev_comm=kworker/8:2 ==> 
 next_comm=ebizzy
   <...>-40753  416.355738: sched_switch: prev_comm=ebizzy ==> 
 next_comm=kworker/8:2
   
 -  
 
   <...>-40753  416.402202: sched_switch: prev_comm=ebizzy ==> 
 next_comm=swapper/8
  -0  416.502130: sched_switch: prev_comm=swapper/8 ==> 
 next_comm=ebizzy
   <...>-40753  416.505738: sched_switch: prev_comm=ebizzy ==> 
 next_comm=kworker/8:2
 kworker/8:2-12137  416.505739: cpu_frequency: state=2061000 cpu_id=8
 kworker/8:2-12137  416.505741: sched_switch: prev_comm=kworker/8:2 ==> 
 next_comm=ebizzy
   <...>-40753  416.515739: sched_switch: prev_comm=ebizzy ==> 
 next_comm=kworker/8:2
 kworker/8:2-12137  416.515742: cpu_frequency: state=4123000 cpu_id=8
 kworker/8:2-12137  416.515744: sched_switch: prev_comm=kworker/8:2 ==> 
 next_comm=ebizzy

 Observation: Ebizzy went idle at 416.402202, and started running again at
 416.502130. But cpufreq noticed the long idle period, and dropped the 
 frequency
 at 416.505739, only to increase it back again at 416.515742, realizing 
 that the
 workload is in-fact CPU bound. Thus ebizzy needlessly ran at the lowest 
 frequency
 for almost 13 milliseconds (almost 1 full sample period), and this pattern
 repeats on every sleep-wakeup. This could hurt latency-sensitive workloads 
 quite
 a lot.

 With patch:
 ~~~

 kworker/8:2-29802  464.832535: cpu_frequency: state=2061000 cpu_id=8
   
 -  
 
 kworker/8:2-29802  464.962538: sched_switch: prev_comm=kworker/8:2 ==> 
 next_comm=ebizzy
   <...>-40738  464.972533: sched_switch: prev_comm=ebizzy ==> 
 next_comm=kworker/8:2
 kworker/8:2-29802  464.972536: cpu_frequency: state=4123000 cpu_id=8
 kworker/8:2-29802  464.972538: sched_switch: prev_comm=kworker/8:2 ==> 
 next_comm=ebizzy
   <...>-40738  464.982531: sched_switch: prev_comm=ebizzy ==> 
 next_comm=kworker/8:2
   
 -  
 
 kworker/8:2-29802  465.022533: sched_switch: prev_comm=kworker/8:2 ==> 
 next_comm=ebizzy
   <...>-40738  465.032531: sched_switch: prev_comm=ebizzy ==> 
 next_comm=kworker/8:2
 kworker/8:2-29802  465.032532: sched_switch: prev_comm=kworker/8:2 ==> 
 next_comm=ebizzy
   <...>-40738  465.035797: sched_switch: prev_comm=ebizzy ==> 
 next_comm=swapper/8
  -0  465.240178: sched_switch: prev_comm=swapper/8 ==> 
 next_comm=ebizzy
   <...>-40738  465.242533: sched_switch: prev_comm=ebizzy ==> 
 next_comm=kworker/8:2
 kworker/8:2-29802  465.242535: sched_switch: prev_comm=kworker/8:2 ==> 
 next_comm=ebizzy
   <...>-40738  465.252531: sched_switch: prev_comm=ebizzy ==> 
 next_comm=kworker/8:2

>>>
>>> Have the log entries emmitted by kworker/8 to report about the
>>> cpu_frequency states been snipped out in the entries post the
>>> "465.032531" mark ?
>>>
>>
>> No, why? Anything looks odd at that point?
> 
> I was expecting to see log messages of the following kind after a
> kworker thread is scheduled in.
> 
>   "kworker/8:2-12137  416.505739: cpu_frequency: state=2061000 cpu_id=8"
>

But this gets printed only if the frequency is changed. If the frequency is 
left at the
same value as it was previously set at (that's the point of this patch), then 
we won't
get this print. [Note that these logs are with the patch applied.]
 
>>
>> Note 

Re: [PATCH] cpufreq: governor: Be friendly towards latency-sensitive bursty workloads

[Gautham R Shenoy]

On Mon, Jun 02, 2014 at 01:45:38PM +0530, Srivatsa S. Bhat wrote:
> On 06/02/2014 01:03 PM, Gautham R Shenoy wrote:
> > Hi,
> > 
> > On Tue, May 27, 2014 at 02:23:38AM +0530, Srivatsa S. Bhat wrote:
> > 
> > [..snip..]
> >>
> >> Experimental results:
> >> 
> >>
> >> I ran a modified version of ebizzy (called 'sleeping-ebizzy') that sleeps 
> >> in
> >> between its execution such that its total utilization can be a user-defined
> >> value, say 10% or 20% (higher the utilization specified, lesser the amount 
> >> of
> >> sleeps injected). This ebizzy was run with a single-thread, tied to CPU 8.
> >>
> >> Behavior observed with tracing (sample taken from 40% utilization runs):
> >> 
> >>
> >> Without patch:
> >> ~~
> >> kworker/8:2-12137  416.335742: cpu_frequency: state=2061000 cpu_id=8
> >> kworker/8:2-12137  416.335744: sched_switch: prev_comm=kworker/8:2 ==> 
> >> next_comm=ebizzy
> >>   <...>-40753  416.345741: sched_switch: prev_comm=ebizzy ==> 
> >> next_comm=kworker/8:2
> >> kworker/8:2-12137  416.345744: cpu_frequency: state=4123000 cpu_id=8
> >> kworker/8:2-12137  416.345746: sched_switch: prev_comm=kworker/8:2 ==> 
> >> next_comm=ebizzy
> >>   <...>-40753  416.355738: sched_switch: prev_comm=ebizzy ==> 
> >> next_comm=kworker/8:2
> >>   
> >> -  
> >> 
> >>   <...>-40753  416.402202: sched_switch: prev_comm=ebizzy ==> 
> >> next_comm=swapper/8
> >>  -0  416.502130: sched_switch: prev_comm=swapper/8 ==> 
> >> next_comm=ebizzy
> >>   <...>-40753  416.505738: sched_switch: prev_comm=ebizzy ==> 
> >> next_comm=kworker/8:2
> >> kworker/8:2-12137  416.505739: cpu_frequency: state=2061000 cpu_id=8
> >> kworker/8:2-12137  416.505741: sched_switch: prev_comm=kworker/8:2 ==> 
> >> next_comm=ebizzy
> >>   <...>-40753  416.515739: sched_switch: prev_comm=ebizzy ==> 
> >> next_comm=kworker/8:2
> >> kworker/8:2-12137  416.515742: cpu_frequency: state=4123000 cpu_id=8
> >> kworker/8:2-12137  416.515744: sched_switch: prev_comm=kworker/8:2 ==> 
> >> next_comm=ebizzy
> >>
> >> Observation: Ebizzy went idle at 416.402202, and started running again at
> >> 416.502130. But cpufreq noticed the long idle period, and dropped the 
> >> frequency
> >> at 416.505739, only to increase it back again at 416.515742, realizing 
> >> that the
> >> workload is in-fact CPU bound. Thus ebizzy needlessly ran at the lowest 
> >> frequency
> >> for almost 13 milliseconds (almost 1 full sample period), and this pattern
> >> repeats on every sleep-wakeup. This could hurt latency-sensitive workloads 
> >> quite
> >> a lot.
> >>
> >> With patch:
> >> ~~~
> >>
> >> kworker/8:2-29802  464.832535: cpu_frequency: state=2061000 cpu_id=8
> >>   
> >> -  
> >> 
> >> kworker/8:2-29802  464.962538: sched_switch: prev_comm=kworker/8:2 ==> 
> >> next_comm=ebizzy
> >>   <...>-40738  464.972533: sched_switch: prev_comm=ebizzy ==> 
> >> next_comm=kworker/8:2
> >> kworker/8:2-29802  464.972536: cpu_frequency: state=4123000 cpu_id=8
> >> kworker/8:2-29802  464.972538: sched_switch: prev_comm=kworker/8:2 ==> 
> >> next_comm=ebizzy
> >>   <...>-40738  464.982531: sched_switch: prev_comm=ebizzy ==> 
> >> next_comm=kworker/8:2
> >>   
> >> -  
> >> 
> >> kworker/8:2-29802  465.022533: sched_switch: prev_comm=kworker/8:2 ==> 
> >> next_comm=ebizzy
> >>   <...>-40738  465.032531: sched_switch: prev_comm=ebizzy ==> 
> >> next_comm=kworker/8:2
> >> kworker/8:2-29802  465.032532: sched_switch: prev_comm=kworker/8:2 ==> 
> >> next_comm=ebizzy
> >>   <...>-40738  465.035797: sched_switch: prev_comm=ebizzy ==> 
> >> next_comm=swapper/8
> >>  -0  465.240178: sched_switch: prev_comm=swapper/8 ==> 
> >> next_comm=ebizzy
> >>   <...>-40738  465.242533: sched_switch: prev_comm=ebizzy ==> 
> >> next_comm=kworker/8:2
> >> kworker/8:2-29802  465.242535: sched_switch: prev_comm=kworker/8:2 ==> 
> >> next_comm=ebizzy
> >>   <...>-40738  465.252531: sched_switch: prev_comm=ebizzy ==> 
> >> next_comm=kworker/8:2
> >>
> > 
> > Have the log entries emmitted by kworker/8 to report about the
> > cpu_frequency states been snipped out in the entries post the
> > "465.032531" mark ?
> > 
> 
> No, why? Anything looks odd at that point?

I was expecting to see log messages of the following kind after a
kworker thread is scheduled in.

  "kworker/8:2-12137  416.505739: cpu_frequency: state=2061000 cpu_id=8"

> 
> Note that the CPU went idle from 465.035797 to 465.240178, and hence cpufreq's
> deferrable timer didn't fire (and hence kworker didn't run). But once the CPU
> became busy again at 465.240178, the kworker got scheduled on the CPU within
> 2 ms (at 465.242533).

Yes, but the logs don't show the frequency that 

Re: [PATCH] cpufreq: governor: Be friendly towards latency-sensitive bursty workloads

[Srivatsa S. Bhat]

On 06/02/2014 01:03 PM, Gautham R Shenoy wrote:
> Hi,
> 
> On Tue, May 27, 2014 at 02:23:38AM +0530, Srivatsa S. Bhat wrote:
> 
> [..snip..]
>>
>> Experimental results:
>> 
>>
>> I ran a modified version of ebizzy (called 'sleeping-ebizzy') that sleeps in
>> between its execution such that its total utilization can be a user-defined
>> value, say 10% or 20% (higher the utilization specified, lesser the amount of
>> sleeps injected). This ebizzy was run with a single-thread, tied to CPU 8.
>>
>> Behavior observed with tracing (sample taken from 40% utilization runs):
>> 
>>
>> Without patch:
>> ~~
>> kworker/8:2-12137  416.335742: cpu_frequency: state=2061000 cpu_id=8
>> kworker/8:2-12137  416.335744: sched_switch: prev_comm=kworker/8:2 ==> 
>> next_comm=ebizzy
>>   <...>-40753  416.345741: sched_switch: prev_comm=ebizzy ==> 
>> next_comm=kworker/8:2
>> kworker/8:2-12137  416.345744: cpu_frequency: state=4123000 cpu_id=8
>> kworker/8:2-12137  416.345746: sched_switch: prev_comm=kworker/8:2 ==> 
>> next_comm=ebizzy
>>   <...>-40753  416.355738: sched_switch: prev_comm=ebizzy ==> 
>> next_comm=kworker/8:2
>>   
>> -  
>>   <...>-40753  416.402202: sched_switch: prev_comm=ebizzy ==> 
>> next_comm=swapper/8
>>  -0  416.502130: sched_switch: prev_comm=swapper/8 ==> 
>> next_comm=ebizzy
>>   <...>-40753  416.505738: sched_switch: prev_comm=ebizzy ==> 
>> next_comm=kworker/8:2
>> kworker/8:2-12137  416.505739: cpu_frequency: state=2061000 cpu_id=8
>> kworker/8:2-12137  416.505741: sched_switch: prev_comm=kworker/8:2 ==> 
>> next_comm=ebizzy
>>   <...>-40753  416.515739: sched_switch: prev_comm=ebizzy ==> 
>> next_comm=kworker/8:2
>> kworker/8:2-12137  416.515742: cpu_frequency: state=4123000 cpu_id=8
>> kworker/8:2-12137  416.515744: sched_switch: prev_comm=kworker/8:2 ==> 
>> next_comm=ebizzy
>>
>> Observation: Ebizzy went idle at 416.402202, and started running again at
>> 416.502130. But cpufreq noticed the long idle period, and dropped the 
>> frequency
>> at 416.505739, only to increase it back again at 416.515742, realizing that 
>> the
>> workload is in-fact CPU bound. Thus ebizzy needlessly ran at the lowest 
>> frequency
>> for almost 13 milliseconds (almost 1 full sample period), and this pattern
>> repeats on every sleep-wakeup. This could hurt latency-sensitive workloads 
>> quite
>> a lot.
>>
>> With patch:
>> ~~~
>>
>> kworker/8:2-29802  464.832535: cpu_frequency: state=2061000 cpu_id=8
>>   
>> -  
>> kworker/8:2-29802  464.962538: sched_switch: prev_comm=kworker/8:2 ==> 
>> next_comm=ebizzy
>>   <...>-40738  464.972533: sched_switch: prev_comm=ebizzy ==> 
>> next_comm=kworker/8:2
>> kworker/8:2-29802  464.972536: cpu_frequency: state=4123000 cpu_id=8
>> kworker/8:2-29802  464.972538: sched_switch: prev_comm=kworker/8:2 ==> 
>> next_comm=ebizzy
>>   <...>-40738  464.982531: sched_switch: prev_comm=ebizzy ==> 
>> next_comm=kworker/8:2
>>   
>> -  
>> kworker/8:2-29802  465.022533: sched_switch: prev_comm=kworker/8:2 ==> 
>> next_comm=ebizzy
>>   <...>-40738  465.032531: sched_switch: prev_comm=ebizzy ==> 
>> next_comm=kworker/8:2
>> kworker/8:2-29802  465.032532: sched_switch: prev_comm=kworker/8:2 ==> 
>> next_comm=ebizzy
>>   <...>-40738  465.035797: sched_switch: prev_comm=ebizzy ==> 
>> next_comm=swapper/8
>>  -0  465.240178: sched_switch: prev_comm=swapper/8 ==> 
>> next_comm=ebizzy
>>   <...>-40738  465.242533: sched_switch: prev_comm=ebizzy ==> 
>> next_comm=kworker/8:2
>> kworker/8:2-29802  465.242535: sched_switch: prev_comm=kworker/8:2 ==> 
>> next_comm=ebizzy
>>   <...>-40738  465.252531: sched_switch: prev_comm=ebizzy ==> 
>> next_comm=kworker/8:2
>>
> 
> Have the log entries emmitted by kworker/8 to report about the
> cpu_frequency states been snipped out in the entries post the
> "465.032531" mark ?
> 

No, why? Anything looks odd at that point?

Note that the CPU went idle from 465.035797 to 465.240178, and hence cpufreq's
deferrable timer didn't fire (and hence kworker didn't run). But once the CPU
became busy again at 465.240178, the kworker got scheduled on the CPU within
2 ms (at 465.242533).

> 
>> Observation: Ebizzy went idle at 465.035797, and started running again at
>> 465.240178. Since ebizzy was the only real workload running on this CPU,
>> cpufreq retained the frequency at 4.1Ghz throughout the run of ebizzy, no
>> matter how many times ebizzy slept and woke-up in-between. Thus, ebizzy
>> got the 10ms worth of 4.1 Ghz benefit during every sleep-wakeup (as compared
>> to the run without the patch) and this boost gave a modest improvement in 
>> total
>> throughput, as shown below.
>>
>> Sleeping-ebizzy 

Re: [PATCH] cpufreq: governor: Be friendly towards latency-sensitive bursty workloads

[Gautham R Shenoy]

Hi,

On Tue, May 27, 2014 at 02:23:38AM +0530, Srivatsa S. Bhat wrote:

[..snip..]
> 
> Experimental results:
> 
> 
> I ran a modified version of ebizzy (called 'sleeping-ebizzy') that sleeps in
> between its execution such that its total utilization can be a user-defined
> value, say 10% or 20% (higher the utilization specified, lesser the amount of
> sleeps injected). This ebizzy was run with a single-thread, tied to CPU 8.
> 
> Behavior observed with tracing (sample taken from 40% utilization runs):
> 
> 
> Without patch:
> ~~
> kworker/8:2-12137  416.335742: cpu_frequency: state=2061000 cpu_id=8
> kworker/8:2-12137  416.335744: sched_switch: prev_comm=kworker/8:2 ==> 
> next_comm=ebizzy
>   <...>-40753  416.345741: sched_switch: prev_comm=ebizzy ==> 
> next_comm=kworker/8:2
> kworker/8:2-12137  416.345744: cpu_frequency: state=4123000 cpu_id=8
> kworker/8:2-12137  416.345746: sched_switch: prev_comm=kworker/8:2 ==> 
> next_comm=ebizzy
>   <...>-40753  416.355738: sched_switch: prev_comm=ebizzy ==> 
> next_comm=kworker/8:2
>   - 
>  
>   <...>-40753  416.402202: sched_switch: prev_comm=ebizzy ==> 
> next_comm=swapper/8
>  -0  416.502130: sched_switch: prev_comm=swapper/8 ==> 
> next_comm=ebizzy
>   <...>-40753  416.505738: sched_switch: prev_comm=ebizzy ==> 
> next_comm=kworker/8:2
> kworker/8:2-12137  416.505739: cpu_frequency: state=2061000 cpu_id=8
> kworker/8:2-12137  416.505741: sched_switch: prev_comm=kworker/8:2 ==> 
> next_comm=ebizzy
>   <...>-40753  416.515739: sched_switch: prev_comm=ebizzy ==> 
> next_comm=kworker/8:2
> kworker/8:2-12137  416.515742: cpu_frequency: state=4123000 cpu_id=8
> kworker/8:2-12137  416.515744: sched_switch: prev_comm=kworker/8:2 ==> 
> next_comm=ebizzy
> 
> Observation: Ebizzy went idle at 416.402202, and started running again at
> 416.502130. But cpufreq noticed the long idle period, and dropped the 
> frequency
> at 416.505739, only to increase it back again at 416.515742, realizing that 
> the
> workload is in-fact CPU bound. Thus ebizzy needlessly ran at the lowest 
> frequency
> for almost 13 milliseconds (almost 1 full sample period), and this pattern
> repeats on every sleep-wakeup. This could hurt latency-sensitive workloads 
> quite
> a lot.
> 
> With patch:
> ~~~
> 
> kworker/8:2-29802  464.832535: cpu_frequency: state=2061000 cpu_id=8
>   - 
>  
> kworker/8:2-29802  464.962538: sched_switch: prev_comm=kworker/8:2 ==> 
> next_comm=ebizzy
>   <...>-40738  464.972533: sched_switch: prev_comm=ebizzy ==> 
> next_comm=kworker/8:2
> kworker/8:2-29802  464.972536: cpu_frequency: state=4123000 cpu_id=8
> kworker/8:2-29802  464.972538: sched_switch: prev_comm=kworker/8:2 ==> 
> next_comm=ebizzy
>   <...>-40738  464.982531: sched_switch: prev_comm=ebizzy ==> 
> next_comm=kworker/8:2
>   - 
>  
> kworker/8:2-29802  465.022533: sched_switch: prev_comm=kworker/8:2 ==> 
> next_comm=ebizzy
>   <...>-40738  465.032531: sched_switch: prev_comm=ebizzy ==> 
> next_comm=kworker/8:2
> kworker/8:2-29802  465.032532: sched_switch: prev_comm=kworker/8:2 ==> 
> next_comm=ebizzy
>   <...>-40738  465.035797: sched_switch: prev_comm=ebizzy ==> 
> next_comm=swapper/8
>  -0  465.240178: sched_switch: prev_comm=swapper/8 ==> 
> next_comm=ebizzy
>   <...>-40738  465.242533: sched_switch: prev_comm=ebizzy ==> 
> next_comm=kworker/8:2
> kworker/8:2-29802  465.242535: sched_switch: prev_comm=kworker/8:2 ==> 
> next_comm=ebizzy
>   <...>-40738  465.252531: sched_switch: prev_comm=ebizzy ==> 
> next_comm=kworker/8:2
> 

Have the log entries emmitted by kworker/8 to report about the
cpu_frequency states been snipped out in the entries post the
"465.032531" mark ?


> Observation: Ebizzy went idle at 465.035797, and started running again at
> 465.240178. Since ebizzy was the only real workload running on this CPU,
> cpufreq retained the frequency at 4.1Ghz throughout the run of ebizzy, no
> matter how many times ebizzy slept and woke-up in-between. Thus, ebizzy
> got the 10ms worth of 4.1 Ghz benefit during every sleep-wakeup (as compared
> to the run without the patch) and this boost gave a modest improvement in 
> total
> throughput, as shown below.
> 
> Sleeping-ebizzy records-per-second:
> ---
> 
> Utilization  Without patch  With patch  Difference (Absolute and % values)
> 10% 274767277046+  2279 (+0.829%)
> 20% 543429553484+ 10055 (+1.850%)
> 40%1090744   1107959+ 17215 (+1.578%)
> 60%1634908   1662018+ 27110 (+1.658%)
> 
> A rudimentary and somewhat approximately latency-sensitive 

Re: [PATCH] cpufreq: governor: Be friendly towards latency-sensitive bursty workloads

[Gautham R Shenoy]

Hi,

On Tue, May 27, 2014 at 02:23:38AM +0530, Srivatsa S. Bhat wrote:

[..snip..]
 
 Experimental results:
 
 
 I ran a modified version of ebizzy (called 'sleeping-ebizzy') that sleeps in
 between its execution such that its total utilization can be a user-defined
 value, say 10% or 20% (higher the utilization specified, lesser the amount of
 sleeps injected). This ebizzy was run with a single-thread, tied to CPU 8.
 
 Behavior observed with tracing (sample taken from 40% utilization runs):
 
 
 Without patch:
 ~~
 kworker/8:2-12137  416.335742: cpu_frequency: state=2061000 cpu_id=8
 kworker/8:2-12137  416.335744: sched_switch: prev_comm=kworker/8:2 == 
 next_comm=ebizzy
   ...-40753  416.345741: sched_switch: prev_comm=ebizzy == 
 next_comm=kworker/8:2
 kworker/8:2-12137  416.345744: cpu_frequency: state=4123000 cpu_id=8
 kworker/8:2-12137  416.345746: sched_switch: prev_comm=kworker/8:2 == 
 next_comm=ebizzy
   ...-40753  416.355738: sched_switch: prev_comm=ebizzy == 
 next_comm=kworker/8:2
 snip  - 
  snip
   ...-40753  416.402202: sched_switch: prev_comm=ebizzy == 
 next_comm=swapper/8
  idle-0  416.502130: sched_switch: prev_comm=swapper/8 == 
 next_comm=ebizzy
   ...-40753  416.505738: sched_switch: prev_comm=ebizzy == 
 next_comm=kworker/8:2
 kworker/8:2-12137  416.505739: cpu_frequency: state=2061000 cpu_id=8
 kworker/8:2-12137  416.505741: sched_switch: prev_comm=kworker/8:2 == 
 next_comm=ebizzy
   ...-40753  416.515739: sched_switch: prev_comm=ebizzy == 
 next_comm=kworker/8:2
 kworker/8:2-12137  416.515742: cpu_frequency: state=4123000 cpu_id=8
 kworker/8:2-12137  416.515744: sched_switch: prev_comm=kworker/8:2 == 
 next_comm=ebizzy
 
 Observation: Ebizzy went idle at 416.402202, and started running again at
 416.502130. But cpufreq noticed the long idle period, and dropped the 
 frequency
 at 416.505739, only to increase it back again at 416.515742, realizing that 
 the
 workload is in-fact CPU bound. Thus ebizzy needlessly ran at the lowest 
 frequency
 for almost 13 milliseconds (almost 1 full sample period), and this pattern
 repeats on every sleep-wakeup. This could hurt latency-sensitive workloads 
 quite
 a lot.
 
 With patch:
 ~~~
 
 kworker/8:2-29802  464.832535: cpu_frequency: state=2061000 cpu_id=8
 snip  - 
  snip
 kworker/8:2-29802  464.962538: sched_switch: prev_comm=kworker/8:2 == 
 next_comm=ebizzy
   ...-40738  464.972533: sched_switch: prev_comm=ebizzy == 
 next_comm=kworker/8:2
 kworker/8:2-29802  464.972536: cpu_frequency: state=4123000 cpu_id=8
 kworker/8:2-29802  464.972538: sched_switch: prev_comm=kworker/8:2 == 
 next_comm=ebizzy
   ...-40738  464.982531: sched_switch: prev_comm=ebizzy == 
 next_comm=kworker/8:2
 snip  - 
  snip
 kworker/8:2-29802  465.022533: sched_switch: prev_comm=kworker/8:2 == 
 next_comm=ebizzy
   ...-40738  465.032531: sched_switch: prev_comm=ebizzy == 
 next_comm=kworker/8:2
 kworker/8:2-29802  465.032532: sched_switch: prev_comm=kworker/8:2 == 
 next_comm=ebizzy
   ...-40738  465.035797: sched_switch: prev_comm=ebizzy == 
 next_comm=swapper/8
  idle-0  465.240178: sched_switch: prev_comm=swapper/8 == 
 next_comm=ebizzy
   ...-40738  465.242533: sched_switch: prev_comm=ebizzy == 
 next_comm=kworker/8:2
 kworker/8:2-29802  465.242535: sched_switch: prev_comm=kworker/8:2 == 
 next_comm=ebizzy
   ...-40738  465.252531: sched_switch: prev_comm=ebizzy == 
 next_comm=kworker/8:2
 

Have the log entries emmitted by kworker/8 to report about the
cpu_frequency states been snipped out in the entries post the
465.032531 mark ?


 Observation: Ebizzy went idle at 465.035797, and started running again at
 465.240178. Since ebizzy was the only real workload running on this CPU,
 cpufreq retained the frequency at 4.1Ghz throughout the run of ebizzy, no
 matter how many times ebizzy slept and woke-up in-between. Thus, ebizzy
 got the 10ms worth of 4.1 Ghz benefit during every sleep-wakeup (as compared
 to the run without the patch) and this boost gave a modest improvement in 
 total
 throughput, as shown below.
 
 Sleeping-ebizzy records-per-second:
 ---
 
 Utilization  Without patch  With patch  Difference (Absolute and % values)
 10% 274767277046+  2279 (+0.829%)
 20% 543429553484+ 10055 (+1.850%)
 40%1090744   1107959+ 17215 (+1.578%)
 60%1634908   1662018+ 27110 (+1.658%)
 
 A rudimentary and somewhat approximately latency-sensitive workload such as
 sleeping-ebizzy itself showed a consistent, noticeable performance improvement
 with this patch. Hence, 

Re: [PATCH] cpufreq: governor: Be friendly towards latency-sensitive bursty workloads

[Srivatsa S. Bhat]

On 06/02/2014 01:03 PM, Gautham R Shenoy wrote:
 Hi,
 
 On Tue, May 27, 2014 at 02:23:38AM +0530, Srivatsa S. Bhat wrote:
 
 [..snip..]

 Experimental results:
 

 I ran a modified version of ebizzy (called 'sleeping-ebizzy') that sleeps in
 between its execution such that its total utilization can be a user-defined
 value, say 10% or 20% (higher the utilization specified, lesser the amount of
 sleeps injected). This ebizzy was run with a single-thread, tied to CPU 8.

 Behavior observed with tracing (sample taken from 40% utilization runs):
 

 Without patch:
 ~~
 kworker/8:2-12137  416.335742: cpu_frequency: state=2061000 cpu_id=8
 kworker/8:2-12137  416.335744: sched_switch: prev_comm=kworker/8:2 == 
 next_comm=ebizzy
   ...-40753  416.345741: sched_switch: prev_comm=ebizzy == 
 next_comm=kworker/8:2
 kworker/8:2-12137  416.345744: cpu_frequency: state=4123000 cpu_id=8
 kworker/8:2-12137  416.345746: sched_switch: prev_comm=kworker/8:2 == 
 next_comm=ebizzy
   ...-40753  416.355738: sched_switch: prev_comm=ebizzy == 
 next_comm=kworker/8:2
 snip  
 -  snip
   ...-40753  416.402202: sched_switch: prev_comm=ebizzy == 
 next_comm=swapper/8
  idle-0  416.502130: sched_switch: prev_comm=swapper/8 == 
 next_comm=ebizzy
   ...-40753  416.505738: sched_switch: prev_comm=ebizzy == 
 next_comm=kworker/8:2
 kworker/8:2-12137  416.505739: cpu_frequency: state=2061000 cpu_id=8
 kworker/8:2-12137  416.505741: sched_switch: prev_comm=kworker/8:2 == 
 next_comm=ebizzy
   ...-40753  416.515739: sched_switch: prev_comm=ebizzy == 
 next_comm=kworker/8:2
 kworker/8:2-12137  416.515742: cpu_frequency: state=4123000 cpu_id=8
 kworker/8:2-12137  416.515744: sched_switch: prev_comm=kworker/8:2 == 
 next_comm=ebizzy

 Observation: Ebizzy went idle at 416.402202, and started running again at
 416.502130. But cpufreq noticed the long idle period, and dropped the 
 frequency
 at 416.505739, only to increase it back again at 416.515742, realizing that 
 the
 workload is in-fact CPU bound. Thus ebizzy needlessly ran at the lowest 
 frequency
 for almost 13 milliseconds (almost 1 full sample period), and this pattern
 repeats on every sleep-wakeup. This could hurt latency-sensitive workloads 
 quite
 a lot.

 With patch:
 ~~~

 kworker/8:2-29802  464.832535: cpu_frequency: state=2061000 cpu_id=8
 snip  
 -  snip
 kworker/8:2-29802  464.962538: sched_switch: prev_comm=kworker/8:2 == 
 next_comm=ebizzy
   ...-40738  464.972533: sched_switch: prev_comm=ebizzy == 
 next_comm=kworker/8:2
 kworker/8:2-29802  464.972536: cpu_frequency: state=4123000 cpu_id=8
 kworker/8:2-29802  464.972538: sched_switch: prev_comm=kworker/8:2 == 
 next_comm=ebizzy
   ...-40738  464.982531: sched_switch: prev_comm=ebizzy == 
 next_comm=kworker/8:2
 snip  
 -  snip
 kworker/8:2-29802  465.022533: sched_switch: prev_comm=kworker/8:2 == 
 next_comm=ebizzy
   ...-40738  465.032531: sched_switch: prev_comm=ebizzy == 
 next_comm=kworker/8:2
 kworker/8:2-29802  465.032532: sched_switch: prev_comm=kworker/8:2 == 
 next_comm=ebizzy
   ...-40738  465.035797: sched_switch: prev_comm=ebizzy == 
 next_comm=swapper/8
  idle-0  465.240178: sched_switch: prev_comm=swapper/8 == 
 next_comm=ebizzy
   ...-40738  465.242533: sched_switch: prev_comm=ebizzy == 
 next_comm=kworker/8:2
 kworker/8:2-29802  465.242535: sched_switch: prev_comm=kworker/8:2 == 
 next_comm=ebizzy
   ...-40738  465.252531: sched_switch: prev_comm=ebizzy == 
 next_comm=kworker/8:2

 
 Have the log entries emmitted by kworker/8 to report about the
 cpu_frequency states been snipped out in the entries post the
 465.032531 mark ?
 

No, why? Anything looks odd at that point?

Note that the CPU went idle from 465.035797 to 465.240178, and hence cpufreq's
deferrable timer didn't fire (and hence kworker didn't run). But once the CPU
became busy again at 465.240178, the kworker got scheduled on the CPU within
2 ms (at 465.242533).

 
 Observation: Ebizzy went idle at 465.035797, and started running again at
 465.240178. Since ebizzy was the only real workload running on this CPU,
 cpufreq retained the frequency at 4.1Ghz throughout the run of ebizzy, no
 matter how many times ebizzy slept and woke-up in-between. Thus, ebizzy
 got the 10ms worth of 4.1 Ghz benefit during every sleep-wakeup (as compared
 to the run without the patch) and this boost gave a modest improvement in 
 total
 throughput, as shown below.

 Sleeping-ebizzy records-per-second:
 ---

 Utilization  Without patch  With patch  Difference (Absolute and % values)
 10% 274767277046+  2279 (+0.829%)
 20% 

Re: [PATCH] cpufreq: governor: Be friendly towards latency-sensitive bursty workloads

[Gautham R Shenoy]

On Mon, Jun 02, 2014 at 01:45:38PM +0530, Srivatsa S. Bhat wrote:
 On 06/02/2014 01:03 PM, Gautham R Shenoy wrote:
  Hi,
  
  On Tue, May 27, 2014 at 02:23:38AM +0530, Srivatsa S. Bhat wrote:
  
  [..snip..]
 
  Experimental results:
  
 
  I ran a modified version of ebizzy (called 'sleeping-ebizzy') that sleeps 
  in
  between its execution such that its total utilization can be a user-defined
  value, say 10% or 20% (higher the utilization specified, lesser the amount 
  of
  sleeps injected). This ebizzy was run with a single-thread, tied to CPU 8.
 
  Behavior observed with tracing (sample taken from 40% utilization runs):
  
 
  Without patch:
  ~~
  kworker/8:2-12137  416.335742: cpu_frequency: state=2061000 cpu_id=8
  kworker/8:2-12137  416.335744: sched_switch: prev_comm=kworker/8:2 == 
  next_comm=ebizzy
...-40753  416.345741: sched_switch: prev_comm=ebizzy == 
  next_comm=kworker/8:2
  kworker/8:2-12137  416.345744: cpu_frequency: state=4123000 cpu_id=8
  kworker/8:2-12137  416.345746: sched_switch: prev_comm=kworker/8:2 == 
  next_comm=ebizzy
...-40753  416.355738: sched_switch: prev_comm=ebizzy == 
  next_comm=kworker/8:2
  snip  
  -  
  snip
...-40753  416.402202: sched_switch: prev_comm=ebizzy == 
  next_comm=swapper/8
   idle-0  416.502130: sched_switch: prev_comm=swapper/8 == 
  next_comm=ebizzy
...-40753  416.505738: sched_switch: prev_comm=ebizzy == 
  next_comm=kworker/8:2
  kworker/8:2-12137  416.505739: cpu_frequency: state=2061000 cpu_id=8
  kworker/8:2-12137  416.505741: sched_switch: prev_comm=kworker/8:2 == 
  next_comm=ebizzy
...-40753  416.515739: sched_switch: prev_comm=ebizzy == 
  next_comm=kworker/8:2
  kworker/8:2-12137  416.515742: cpu_frequency: state=4123000 cpu_id=8
  kworker/8:2-12137  416.515744: sched_switch: prev_comm=kworker/8:2 == 
  next_comm=ebizzy
 
  Observation: Ebizzy went idle at 416.402202, and started running again at
  416.502130. But cpufreq noticed the long idle period, and dropped the 
  frequency
  at 416.505739, only to increase it back again at 416.515742, realizing 
  that the
  workload is in-fact CPU bound. Thus ebizzy needlessly ran at the lowest 
  frequency
  for almost 13 milliseconds (almost 1 full sample period), and this pattern
  repeats on every sleep-wakeup. This could hurt latency-sensitive workloads 
  quite
  a lot.
 
  With patch:
  ~~~
 
  kworker/8:2-29802  464.832535: cpu_frequency: state=2061000 cpu_id=8
  snip  
  -  
  snip
  kworker/8:2-29802  464.962538: sched_switch: prev_comm=kworker/8:2 == 
  next_comm=ebizzy
...-40738  464.972533: sched_switch: prev_comm=ebizzy == 
  next_comm=kworker/8:2
  kworker/8:2-29802  464.972536: cpu_frequency: state=4123000 cpu_id=8
  kworker/8:2-29802  464.972538: sched_switch: prev_comm=kworker/8:2 == 
  next_comm=ebizzy
...-40738  464.982531: sched_switch: prev_comm=ebizzy == 
  next_comm=kworker/8:2
  snip  
  -  
  snip
  kworker/8:2-29802  465.022533: sched_switch: prev_comm=kworker/8:2 == 
  next_comm=ebizzy
...-40738  465.032531: sched_switch: prev_comm=ebizzy == 
  next_comm=kworker/8:2
  kworker/8:2-29802  465.032532: sched_switch: prev_comm=kworker/8:2 == 
  next_comm=ebizzy
...-40738  465.035797: sched_switch: prev_comm=ebizzy == 
  next_comm=swapper/8
   idle-0  465.240178: sched_switch: prev_comm=swapper/8 == 
  next_comm=ebizzy
...-40738  465.242533: sched_switch: prev_comm=ebizzy == 
  next_comm=kworker/8:2
  kworker/8:2-29802  465.242535: sched_switch: prev_comm=kworker/8:2 == 
  next_comm=ebizzy
...-40738  465.252531: sched_switch: prev_comm=ebizzy == 
  next_comm=kworker/8:2
 
  
  Have the log entries emmitted by kworker/8 to report about the
  cpu_frequency states been snipped out in the entries post the
  465.032531 mark ?
  
 
 No, why? Anything looks odd at that point?

I was expecting to see log messages of the following kind after a
kworker thread is scheduled in.

  kworker/8:2-12137  416.505739: cpu_frequency: state=2061000 cpu_id=8

 
 Note that the CPU went idle from 465.035797 to 465.240178, and hence cpufreq's
 deferrable timer didn't fire (and hence kworker didn't run). But once the CPU
 became busy again at 465.240178, the kworker got scheduled on the CPU within
 2 ms (at 465.242533).

Yes, but the logs don't show the frequency that the kworker thread
would have set on that cpu.

 [..snip...]
 
  -  load = 100 * (wall_time - idle_time) / wall_time;
  +  /*
  +   * If the CPU had gone completely idle, and a task just woke up
  +   * on this CPU now, it would be unfair to calculate 'load' the
  +   * usual way 

Re: [PATCH] cpufreq: governor: Be friendly towards latency-sensitive bursty workloads

[Srivatsa S. Bhat]

On 06/03/2014 10:46 AM, Gautham R Shenoy wrote:
 On Mon, Jun 02, 2014 at 01:45:38PM +0530, Srivatsa S. Bhat wrote:
 On 06/02/2014 01:03 PM, Gautham R Shenoy wrote:
 Hi,

 On Tue, May 27, 2014 at 02:23:38AM +0530, Srivatsa S. Bhat wrote:

 [..snip..]

 Experimental results:
 

 I ran a modified version of ebizzy (called 'sleeping-ebizzy') that sleeps 
 in
 between its execution such that its total utilization can be a user-defined
 value, say 10% or 20% (higher the utilization specified, lesser the amount 
 of
 sleeps injected). This ebizzy was run with a single-thread, tied to CPU 8.

 Behavior observed with tracing (sample taken from 40% utilization runs):
 

 Without patch:
 ~~
 kworker/8:2-12137  416.335742: cpu_frequency: state=2061000 cpu_id=8
 kworker/8:2-12137  416.335744: sched_switch: prev_comm=kworker/8:2 == 
 next_comm=ebizzy
   ...-40753  416.345741: sched_switch: prev_comm=ebizzy == 
 next_comm=kworker/8:2
 kworker/8:2-12137  416.345744: cpu_frequency: state=4123000 cpu_id=8
 kworker/8:2-12137  416.345746: sched_switch: prev_comm=kworker/8:2 == 
 next_comm=ebizzy
   ...-40753  416.355738: sched_switch: prev_comm=ebizzy == 
 next_comm=kworker/8:2
 snip  
 -  
 snip
   ...-40753  416.402202: sched_switch: prev_comm=ebizzy == 
 next_comm=swapper/8
  idle-0  416.502130: sched_switch: prev_comm=swapper/8 == 
 next_comm=ebizzy
   ...-40753  416.505738: sched_switch: prev_comm=ebizzy == 
 next_comm=kworker/8:2
 kworker/8:2-12137  416.505739: cpu_frequency: state=2061000 cpu_id=8
 kworker/8:2-12137  416.505741: sched_switch: prev_comm=kworker/8:2 == 
 next_comm=ebizzy
   ...-40753  416.515739: sched_switch: prev_comm=ebizzy == 
 next_comm=kworker/8:2
 kworker/8:2-12137  416.515742: cpu_frequency: state=4123000 cpu_id=8
 kworker/8:2-12137  416.515744: sched_switch: prev_comm=kworker/8:2 == 
 next_comm=ebizzy

 Observation: Ebizzy went idle at 416.402202, and started running again at
 416.502130. But cpufreq noticed the long idle period, and dropped the 
 frequency
 at 416.505739, only to increase it back again at 416.515742, realizing 
 that the
 workload is in-fact CPU bound. Thus ebizzy needlessly ran at the lowest 
 frequency
 for almost 13 milliseconds (almost 1 full sample period), and this pattern
 repeats on every sleep-wakeup. This could hurt latency-sensitive workloads 
 quite
 a lot.

 With patch:
 ~~~

 kworker/8:2-29802  464.832535: cpu_frequency: state=2061000 cpu_id=8
 snip  
 -  
 snip
 kworker/8:2-29802  464.962538: sched_switch: prev_comm=kworker/8:2 == 
 next_comm=ebizzy
   ...-40738  464.972533: sched_switch: prev_comm=ebizzy == 
 next_comm=kworker/8:2
 kworker/8:2-29802  464.972536: cpu_frequency: state=4123000 cpu_id=8
 kworker/8:2-29802  464.972538: sched_switch: prev_comm=kworker/8:2 == 
 next_comm=ebizzy
   ...-40738  464.982531: sched_switch: prev_comm=ebizzy == 
 next_comm=kworker/8:2
 snip  
 -  
 snip
 kworker/8:2-29802  465.022533: sched_switch: prev_comm=kworker/8:2 == 
 next_comm=ebizzy
   ...-40738  465.032531: sched_switch: prev_comm=ebizzy == 
 next_comm=kworker/8:2
 kworker/8:2-29802  465.032532: sched_switch: prev_comm=kworker/8:2 == 
 next_comm=ebizzy
   ...-40738  465.035797: sched_switch: prev_comm=ebizzy == 
 next_comm=swapper/8
  idle-0  465.240178: sched_switch: prev_comm=swapper/8 == 
 next_comm=ebizzy
   ...-40738  465.242533: sched_switch: prev_comm=ebizzy == 
 next_comm=kworker/8:2
 kworker/8:2-29802  465.242535: sched_switch: prev_comm=kworker/8:2 == 
 next_comm=ebizzy
   ...-40738  465.252531: sched_switch: prev_comm=ebizzy == 
 next_comm=kworker/8:2


 Have the log entries emmitted by kworker/8 to report about the
 cpu_frequency states been snipped out in the entries post the
 465.032531 mark ?


 No, why? Anything looks odd at that point?
 
 I was expecting to see log messages of the following kind after a
 kworker thread is scheduled in.
 
   kworker/8:2-12137  416.505739: cpu_frequency: state=2061000 cpu_id=8


But this gets printed only if the frequency is changed. If the frequency is 
left at the
same value as it was previously set at (that's the point of this patch), then 
we won't
get this print. [Note that these logs are with the patch applied.]
 

 Note that the CPU went idle from 465.035797 to 465.240178, and hence 
 cpufreq's
 deferrable timer didn't fire (and hence kworker didn't run). But once the CPU
 became busy again at 465.240178, the kworker got scheduled on the CPU within
 2 ms (at 465.242533).
 
 Yes, but the logs don't show the frequency that the kworker thread
 would have set on that cpu.
 

Yes, and that's expected, because we copy the previous load to the 

Re: [PATCH] cpufreq: governor: Be friendly towards latency-sensitive bursty workloads

[Srivatsa S. Bhat]

On 05/27/2014 04:57 AM, Rafael J. Wysocki wrote:
> Hi Srivatsa,
> 
> On Tuesday, May 27, 2014 02:23:38 AM Srivatsa S. Bhat wrote:
>> Cpufreq governors like the ondemand governor calculate the load on the CPU
>> periodically by employing deferrable timers. A deferrable timer won't fire
>> if the CPU is completely idle (and there are no other timers to be run), in
>> order to avoid unnecessary wakeups and thus save CPU power.
>>
>> However, the load calculation logic is agnostic to all this, and this can
>> lead to the problem described below.
> 
> This is subtle enough that I need some more time to chew on it, but since the
> merge window is coming, I'm not sure when that's going to happen honestly.
>

Sure, I completely understand. Please take your time, no hurry!

Thank you very much!

Regards,
Srivatsa S. Bhat

--
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] cpufreq: governor: Be friendly towards latency-sensitive bursty workloads

[Rafael J. Wysocki]

Hi Srivatsa,

On Tuesday, May 27, 2014 02:23:38 AM Srivatsa S. Bhat wrote:
> Cpufreq governors like the ondemand governor calculate the load on the CPU
> periodically by employing deferrable timers. A deferrable timer won't fire
> if the CPU is completely idle (and there are no other timers to be run), in
> order to avoid unnecessary wakeups and thus save CPU power.
> 
> However, the load calculation logic is agnostic to all this, and this can
> lead to the problem described below.

This is subtle enough that I need some more time to chew on it, but since the
merge window is coming, I'm not sure when that's going to happen honestly.

Rafael

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

[PATCH] cpufreq: governor: Be friendly towards latency-sensitive bursty workloads

[Srivatsa S. Bhat]

Cpufreq governors like the ondemand governor calculate the load on the CPU
periodically by employing deferrable timers. A deferrable timer won't fire
if the CPU is completely idle (and there are no other timers to be run), in
order to avoid unnecessary wakeups and thus save CPU power.

However, the load calculation logic is agnostic to all this, and this can
lead to the problem described below.


Time (ms)   CPU 1

100Task-A running

110Governor's timer fires, finds load as 100% in the last
   10ms interval and increases the CPU frequency.

110.5  Task-A running

120Governor's timer fires, finds load as 100% in the last
   10ms interval and increases the CPU frequency.

125Task-A went to sleep. With nothing else to do, CPU 1
   went completely idle.

200Task-A woke up and started running again.

200.5  Governor's deferred timer (which was originally programmed
   to fire at time 130) fires now. It calculates load for the
   time period 120 to 200.5, and finds the load is almost zero.
   Hence it decreases the CPU frequency to the minimum.

210Governor's timer fires, finds load as 100% in the last
   10ms interval and increases the CPU frequency.


So, after the workload woke up and started running, the frequency was suddenly
dropped to absolute minimum, and after that, there was an unnecessary delay of
10ms (sampling period) to increase the CPU frequency back to a reasonable value.
And this pattern repeats for every wake-up-from-cpu-idle for that workload.
This can be quite undesirable for latency- or response-time sensitive bursty
workloads. So we need to fix the governor's logic to detect such wake-up-from-
cpu-idle scenarios and start the workload at a reasonably high CPU frequency.

One extreme solution would be to fake a load of 100% in such scenarios. But
that might lead to undesirable side-effects such as frequency spikes (which
might also need voltage changes) especially if the previous frequency happened
to be very low.

We just want to avoid the stupidity of dropping down the frequency to a minimum
and then enduring a needless (and long) delay before ramping it up back again.
So, let us simply carry forward the previous load - that is, let us just pretend
that the 'load' for the current time-window is the same as the load for the
previous window. That way, the frequency and voltage will continue to be set
to whatever values they were set at previously. This means that bursty workloads
will get a chance to influence the CPU frequency at which they wake up from
cpu-idle, based on their past execution history. Thus, they might be able to
avoid suffering from slow wakeups and long response-times.

[ The right way to solve this problem is to teach the CPU frequency governors
to track load on a per-task basis, not a per-CPU basis, and set the appropriate
frequency on whichever CPU the task executes. But that involves redesigning
the cpufreq subsystem, so this patch should make the situation bearable until
then. ]

Experimental results:


I ran a modified version of ebizzy (called 'sleeping-ebizzy') that sleeps in
between its execution such that its total utilization can be a user-defined
value, say 10% or 20% (higher the utilization specified, lesser the amount of
sleeps injected). This ebizzy was run with a single-thread, tied to CPU 8.

Behavior observed with tracing (sample taken from 40% utilization runs):


Without patch:
~~
kworker/8:2-12137  416.335742: cpu_frequency: state=2061000 cpu_id=8
kworker/8:2-12137  416.335744: sched_switch: prev_comm=kworker/8:2 ==> 
next_comm=ebizzy
  <...>-40753  416.345741: sched_switch: prev_comm=ebizzy ==> 
next_comm=kworker/8:2
kworker/8:2-12137  416.345744: cpu_frequency: state=4123000 cpu_id=8
kworker/8:2-12137  416.345746: sched_switch: prev_comm=kworker/8:2 ==> 
next_comm=ebizzy
  <...>-40753  416.355738: sched_switch: prev_comm=ebizzy ==> 
next_comm=kworker/8:2
  -  

  <...>-40753  416.402202: sched_switch: prev_comm=ebizzy ==> 
next_comm=swapper/8
 -0  416.502130: sched_switch: prev_comm=swapper/8 ==> 
next_comm=ebizzy
  <...>-40753  416.505738: sched_switch: prev_comm=ebizzy ==> 
next_comm=kworker/8:2
kworker/8:2-12137  416.505739: cpu_frequency: state=2061000 cpu_id=8
kworker/8:2-12137  416.505741: sched_switch: prev_comm=kworker/8:2 ==> 
next_comm=ebizzy
  <...>-40753  416.515739: sched_switch: prev_comm=ebizzy ==> 
next_comm=kworker/8:2
kworker/8:2-12137  416.515742: cpu_frequency: state=4123000 cpu_id=8
kworker/8:2-12137  416.515744: sched_switch: prev_comm=kworker/8:2 ==> 
next_comm=ebizzy

Observation: Ebizzy 

[PATCH] cpufreq: governor: Be friendly towards latency-sensitive bursty workloads

[Srivatsa S. Bhat]

Cpufreq governors like the ondemand governor calculate the load on the CPU
periodically by employing deferrable timers. A deferrable timer won't fire
if the CPU is completely idle (and there are no other timers to be run), in
order to avoid unnecessary wakeups and thus save CPU power.

However, the load calculation logic is agnostic to all this, and this can
lead to the problem described below.


Time (ms)   CPU 1

100Task-A running

110Governor's timer fires, finds load as 100% in the last
   10ms interval and increases the CPU frequency.

110.5  Task-A running

120Governor's timer fires, finds load as 100% in the last
   10ms interval and increases the CPU frequency.

125Task-A went to sleep. With nothing else to do, CPU 1
   went completely idle.

200Task-A woke up and started running again.

200.5  Governor's deferred timer (which was originally programmed
   to fire at time 130) fires now. It calculates load for the
   time period 120 to 200.5, and finds the load is almost zero.
   Hence it decreases the CPU frequency to the minimum.

210Governor's timer fires, finds load as 100% in the last
   10ms interval and increases the CPU frequency.


So, after the workload woke up and started running, the frequency was suddenly
dropped to absolute minimum, and after that, there was an unnecessary delay of
10ms (sampling period) to increase the CPU frequency back to a reasonable value.
And this pattern repeats for every wake-up-from-cpu-idle for that workload.
This can be quite undesirable for latency- or response-time sensitive bursty
workloads. So we need to fix the governor's logic to detect such wake-up-from-
cpu-idle scenarios and start the workload at a reasonably high CPU frequency.

One extreme solution would be to fake a load of 100% in such scenarios. But
that might lead to undesirable side-effects such as frequency spikes (which
might also need voltage changes) especially if the previous frequency happened
to be very low.

We just want to avoid the stupidity of dropping down the frequency to a minimum
and then enduring a needless (and long) delay before ramping it up back again.
So, let us simply carry forward the previous load - that is, let us just pretend
that the 'load' for the current time-window is the same as the load for the
previous window. That way, the frequency and voltage will continue to be set
to whatever values they were set at previously. This means that bursty workloads
will get a chance to influence the CPU frequency at which they wake up from
cpu-idle, based on their past execution history. Thus, they might be able to
avoid suffering from slow wakeups and long response-times.

[ The right way to solve this problem is to teach the CPU frequency governors
to track load on a per-task basis, not a per-CPU basis, and set the appropriate
frequency on whichever CPU the task executes. But that involves redesigning
the cpufreq subsystem, so this patch should make the situation bearable until
then. ]

Experimental results:


I ran a modified version of ebizzy (called 'sleeping-ebizzy') that sleeps in
between its execution such that its total utilization can be a user-defined
value, say 10% or 20% (higher the utilization specified, lesser the amount of
sleeps injected). This ebizzy was run with a single-thread, tied to CPU 8.

Behavior observed with tracing (sample taken from 40% utilization runs):


Without patch:
~~
kworker/8:2-12137  416.335742: cpu_frequency: state=2061000 cpu_id=8
kworker/8:2-12137  416.335744: sched_switch: prev_comm=kworker/8:2 == 
next_comm=ebizzy
  ...-40753  416.345741: sched_switch: prev_comm=ebizzy == 
next_comm=kworker/8:2
kworker/8:2-12137  416.345744: cpu_frequency: state=4123000 cpu_id=8
kworker/8:2-12137  416.345746: sched_switch: prev_comm=kworker/8:2 == 
next_comm=ebizzy
  ...-40753  416.355738: sched_switch: prev_comm=ebizzy == 
next_comm=kworker/8:2
snip  -  
snip
  ...-40753  416.402202: sched_switch: prev_comm=ebizzy == 
next_comm=swapper/8
 idle-0  416.502130: sched_switch: prev_comm=swapper/8 == 
next_comm=ebizzy
  ...-40753  416.505738: sched_switch: prev_comm=ebizzy == 
next_comm=kworker/8:2
kworker/8:2-12137  416.505739: cpu_frequency: state=2061000 cpu_id=8
kworker/8:2-12137  416.505741: sched_switch: prev_comm=kworker/8:2 == 
next_comm=ebizzy
  ...-40753  416.515739: sched_switch: prev_comm=ebizzy == 
next_comm=kworker/8:2
kworker/8:2-12137  416.515742: cpu_frequency: state=4123000 cpu_id=8
kworker/8:2-12137  416.515744: sched_switch: prev_comm=kworker/8:2 == 
next_comm=ebizzy

Observation: Ebizzy went idle 

Re: [PATCH] cpufreq: governor: Be friendly towards latency-sensitive bursty workloads

[Rafael J. Wysocki]

Hi Srivatsa,

On Tuesday, May 27, 2014 02:23:38 AM Srivatsa S. Bhat wrote:
 Cpufreq governors like the ondemand governor calculate the load on the CPU
 periodically by employing deferrable timers. A deferrable timer won't fire
 if the CPU is completely idle (and there are no other timers to be run), in
 order to avoid unnecessary wakeups and thus save CPU power.
 
 However, the load calculation logic is agnostic to all this, and this can
 lead to the problem described below.

This is subtle enough that I need some more time to chew on it, but since the
merge window is coming, I'm not sure when that's going to happen honestly.

Rafael

--
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] cpufreq: governor: Be friendly towards latency-sensitive bursty workloads

[Srivatsa S. Bhat]

On 05/27/2014 04:57 AM, Rafael J. Wysocki wrote:
 Hi Srivatsa,
 
 On Tuesday, May 27, 2014 02:23:38 AM Srivatsa S. Bhat wrote:
 Cpufreq governors like the ondemand governor calculate the load on the CPU
 periodically by employing deferrable timers. A deferrable timer won't fire
 if the CPU is completely idle (and there are no other timers to be run), in
 order to avoid unnecessary wakeups and thus save CPU power.

 However, the load calculation logic is agnostic to all this, and this can
 lead to the problem described below.
 
 This is subtle enough that I need some more time to chew on it, but since the
 merge window is coming, I'm not sure when that's going to happen honestly.


Sure, I completely understand. Please take your time, no hurry!

Thank you very much!

Regards,
Srivatsa S. Bhat

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