linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] cpuidle: record state entry failed statistics
@ 2020-09-02 21:01 Lina Iyer
  2020-09-22 15:57 ` Rafael J. Wysocki
  0 siblings, 1 reply; 3+ messages in thread
From: Lina Iyer @ 2020-09-02 21:01 UTC (permalink / raw)
  To: rjw, daniel.lezcano; +Cc: linux-pm, linux-arm-msm, Lina Iyer

When CPUs fail to enter the chosen idle state it's mostly because of a
pending interrupt. Let's record that and show along with other
statistics for the idle state. This could prove useful in understanding
behavior of the governor and the system during usecases that involve
multiple CPUs.

Signed-off-by: Lina Iyer <ilina@codeaurora.org>
---
 drivers/cpuidle/cpuidle.c | 1 +
 drivers/cpuidle/sysfs.c   | 3 +++
 include/linux/cpuidle.h   | 1 +
 3 files changed, 5 insertions(+)

diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index 04becd70cc41..8dbf71f6138d 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -302,6 +302,7 @@ int cpuidle_enter_state(struct cpuidle_device *dev, struct cpuidle_driver *drv,
 		}
 	} else {
 		dev->last_residency_ns = 0;
+		dev->states_usage[index].failed++;
 	}
 
 	return entered_state;
diff --git a/drivers/cpuidle/sysfs.c b/drivers/cpuidle/sysfs.c
index 091d1caceb41..f166687b3bcd 100644
--- a/drivers/cpuidle/sysfs.c
+++ b/drivers/cpuidle/sysfs.c
@@ -256,6 +256,7 @@ define_show_state_time_function(exit_latency)
 define_show_state_time_function(target_residency)
 define_show_state_function(power_usage)
 define_show_state_ull_function(usage)
+define_show_state_ull_function(failed)
 define_show_state_str_function(name)
 define_show_state_str_function(desc)
 define_show_state_ull_function(above)
@@ -312,6 +313,7 @@ define_one_state_ro(latency, show_state_exit_latency);
 define_one_state_ro(residency, show_state_target_residency);
 define_one_state_ro(power, show_state_power_usage);
 define_one_state_ro(usage, show_state_usage);
+define_one_state_ro(failed, show_state_failed);
 define_one_state_ro(time, show_state_time);
 define_one_state_rw(disable, show_state_disable, store_state_disable);
 define_one_state_ro(above, show_state_above);
@@ -325,6 +327,7 @@ static struct attribute *cpuidle_state_default_attrs[] = {
 	&attr_residency.attr,
 	&attr_power.attr,
 	&attr_usage.attr,
+	&attr_failed.attr,
 	&attr_time.attr,
 	&attr_disable.attr,
 	&attr_above.attr,
diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
index 75895e6363b8..911b99273eba 100644
--- a/include/linux/cpuidle.h
+++ b/include/linux/cpuidle.h
@@ -38,6 +38,7 @@ struct cpuidle_state_usage {
 	u64			time_ns;
 	unsigned long long	above; /* Number of times it's been too deep */
 	unsigned long long	below; /* Number of times it's been too shallow */
+	unsigned long long	failed; /* Number of times it failed to enter */
 #ifdef CONFIG_SUSPEND
 	unsigned long long	s2idle_usage;
 	unsigned long long	s2idle_time; /* in US */
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


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

* Re: [PATCH] cpuidle: record state entry failed statistics
  2020-09-02 21:01 [PATCH] cpuidle: record state entry failed statistics Lina Iyer
@ 2020-09-22 15:57 ` Rafael J. Wysocki
  2020-09-22 16:00   ` Lina Iyer
  0 siblings, 1 reply; 3+ messages in thread
From: Rafael J. Wysocki @ 2020-09-22 15:57 UTC (permalink / raw)
  To: Lina Iyer; +Cc: Rafael J. Wysocki, Daniel Lezcano, Linux PM, linux-arm-msm

Sorry for the delay.

On Wed, Sep 2, 2020 at 11:01 PM Lina Iyer <ilina@codeaurora.org> wrote:
>
> When CPUs fail to enter the chosen idle state it's mostly because of a
> pending interrupt. Let's record that and show along with other
> statistics for the idle state. This could prove useful in understanding
> behavior of the governor and the system during usecases that involve
> multiple CPUs.
>
> Signed-off-by: Lina Iyer <ilina@codeaurora.org>
> ---
>  drivers/cpuidle/cpuidle.c | 1 +
>  drivers/cpuidle/sysfs.c   | 3 +++
>  include/linux/cpuidle.h   | 1 +

The documentation needs to be updated too to cover the new state attribute.

>  3 files changed, 5 insertions(+)
>
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index 04becd70cc41..8dbf71f6138d 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -302,6 +302,7 @@ int cpuidle_enter_state(struct cpuidle_device *dev, struct cpuidle_driver *drv,
>                 }
>         } else {
>                 dev->last_residency_ns = 0;
> +               dev->states_usage[index].failed++;
>         }
>
>         return entered_state;
> diff --git a/drivers/cpuidle/sysfs.c b/drivers/cpuidle/sysfs.c
> index 091d1caceb41..f166687b3bcd 100644
> --- a/drivers/cpuidle/sysfs.c
> +++ b/drivers/cpuidle/sysfs.c
> @@ -256,6 +256,7 @@ define_show_state_time_function(exit_latency)
>  define_show_state_time_function(target_residency)
>  define_show_state_function(power_usage)
>  define_show_state_ull_function(usage)
> +define_show_state_ull_function(failed)

And what about calling it "rejected" instead of "failed"?

>  define_show_state_str_function(name)
>  define_show_state_str_function(desc)
>  define_show_state_ull_function(above)
> @@ -312,6 +313,7 @@ define_one_state_ro(latency, show_state_exit_latency);
>  define_one_state_ro(residency, show_state_target_residency);
>  define_one_state_ro(power, show_state_power_usage);
>  define_one_state_ro(usage, show_state_usage);
> +define_one_state_ro(failed, show_state_failed);
>  define_one_state_ro(time, show_state_time);
>  define_one_state_rw(disable, show_state_disable, store_state_disable);
>  define_one_state_ro(above, show_state_above);
> @@ -325,6 +327,7 @@ static struct attribute *cpuidle_state_default_attrs[] = {
>         &attr_residency.attr,
>         &attr_power.attr,
>         &attr_usage.attr,
> +       &attr_failed.attr,
>         &attr_time.attr,
>         &attr_disable.attr,
>         &attr_above.attr,
> diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
> index 75895e6363b8..911b99273eba 100644
> --- a/include/linux/cpuidle.h
> +++ b/include/linux/cpuidle.h
> @@ -38,6 +38,7 @@ struct cpuidle_state_usage {
>         u64                     time_ns;
>         unsigned long long      above; /* Number of times it's been too deep */
>         unsigned long long      below; /* Number of times it's been too shallow */
> +       unsigned long long      failed; /* Number of times it failed to enter */
>  #ifdef CONFIG_SUSPEND
>         unsigned long long      s2idle_usage;
>         unsigned long long      s2idle_time; /* in US */
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>

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

* Re: [PATCH] cpuidle: record state entry failed statistics
  2020-09-22 15:57 ` Rafael J. Wysocki
@ 2020-09-22 16:00   ` Lina Iyer
  0 siblings, 0 replies; 3+ messages in thread
From: Lina Iyer @ 2020-09-22 16:00 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Rafael J. Wysocki, Daniel Lezcano, Linux PM, linux-arm-msm

On Tue, Sep 22 2020 at 09:57 -0600, Rafael J. Wysocki wrote:
>Sorry for the delay.
>
Thanks for the review.

>On Wed, Sep 2, 2020 at 11:01 PM Lina Iyer <ilina@codeaurora.org> wrote:
>>
>> When CPUs fail to enter the chosen idle state it's mostly because of a
>> pending interrupt. Let's record that and show along with other
>> statistics for the idle state. This could prove useful in understanding
>> behavior of the governor and the system during usecases that involve
>> multiple CPUs.
>>
>> Signed-off-by: Lina Iyer <ilina@codeaurora.org>
>> ---
>>  drivers/cpuidle/cpuidle.c | 1 +
>>  drivers/cpuidle/sysfs.c   | 3 +++
>>  include/linux/cpuidle.h   | 1 +
>
>The documentation needs to be updated too to cover the new state attribute.
>
Sure, will add that in the next spin.
>>  3 files changed, 5 insertions(+)
>>
>> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
>> index 04becd70cc41..8dbf71f6138d 100644
>> --- a/drivers/cpuidle/cpuidle.c
>> +++ b/drivers/cpuidle/cpuidle.c
>> @@ -302,6 +302,7 @@ int cpuidle_enter_state(struct cpuidle_device *dev, struct cpuidle_driver *drv,
>>                 }
>>         } else {
>>                 dev->last_residency_ns = 0;
>> +               dev->states_usage[index].failed++;
>>         }
>>
>>         return entered_state;
>> diff --git a/drivers/cpuidle/sysfs.c b/drivers/cpuidle/sysfs.c
>> index 091d1caceb41..f166687b3bcd 100644
>> --- a/drivers/cpuidle/sysfs.c
>> +++ b/drivers/cpuidle/sysfs.c
>> @@ -256,6 +256,7 @@ define_show_state_time_function(exit_latency)
>>  define_show_state_time_function(target_residency)
>>  define_show_state_function(power_usage)
>>  define_show_state_ull_function(usage)
>> +define_show_state_ull_function(failed)
>
>And what about calling it "rejected" instead of "failed"?
>
Works for me.

Thanks,
Lina

>>  define_show_state_str_function(name)
>>  define_show_state_str_function(desc)
>>  define_show_state_ull_function(above)
>> @@ -312,6 +313,7 @@ define_one_state_ro(latency, show_state_exit_latency);
>>  define_one_state_ro(residency, show_state_target_residency);
>>  define_one_state_ro(power, show_state_power_usage);
>>  define_one_state_ro(usage, show_state_usage);
>> +define_one_state_ro(failed, show_state_failed);
>>  define_one_state_ro(time, show_state_time);
>>  define_one_state_rw(disable, show_state_disable, store_state_disable);
>>  define_one_state_ro(above, show_state_above);
>> @@ -325,6 +327,7 @@ static struct attribute *cpuidle_state_default_attrs[] = {
>>         &attr_residency.attr,
>>         &attr_power.attr,
>>         &attr_usage.attr,
>> +       &attr_failed.attr,
>>         &attr_time.attr,
>>         &attr_disable.attr,
>>         &attr_above.attr,
>> diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
>> index 75895e6363b8..911b99273eba 100644
>> --- a/include/linux/cpuidle.h
>> +++ b/include/linux/cpuidle.h
>> @@ -38,6 +38,7 @@ struct cpuidle_state_usage {
>>         u64                     time_ns;
>>         unsigned long long      above; /* Number of times it's been too deep */
>>         unsigned long long      below; /* Number of times it's been too shallow */
>> +       unsigned long long      failed; /* Number of times it failed to enter */
>>  #ifdef CONFIG_SUSPEND
>>         unsigned long long      s2idle_usage;
>>         unsigned long long      s2idle_time; /* in US */
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
>> a Linux Foundation Collaborative Project
>>

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

end of thread, other threads:[~2020-09-22 16:01 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-02 21:01 [PATCH] cpuidle: record state entry failed statistics Lina Iyer
2020-09-22 15:57 ` Rafael J. Wysocki
2020-09-22 16:00   ` Lina Iyer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).