From ead18c23c263374ed098a7d955b29b4a466d4573 Mon Sep 17 00:00:00 2001 From: Lukas Wunner Date: Sat, 10 Feb 2018 19:27:12 +0100 Subject: driver core: Introduce device links reference counting If device_link_add() is invoked multiple times with the same supplier and consumer combo, it will create the link on first addition and return a pointer to the already existing link on all subsequent additions. The semantics for device_link_del() are quite different, it deletes the link unconditionally, so multiple invocations are not allowed. In other words, this snippet ... struct device *dev1, *dev2; struct device_link *link1, *link2; link1 = device_link_add(dev1, dev2, 0); link2 = device_link_add(dev1, dev2, 0); device_link_del(link1); device_link_del(link2); ... causes the following crash: WARNING: CPU: 4 PID: 2686 at drivers/base/power/runtime.c:1611 pm_runtime_drop_link+0x40/0x50 [...] list_del corruption, 0000000039b800a4->prev is LIST_POISON2 (00000000ecf79852) kernel BUG at lib/list_debug.c:50! The issue isn't as arbitrary as it may seem: Imagine a device link which is added in both the supplier's and the consumer's ->probe hook. The two drivers can't just call device_link_del() in their ->remove hook without coordination. Fix by counting multiple additions and dropping the device link only when the last addition is unwound. Signed-off-by: Lukas Wunner [ rjw: Subject ] Signed-off-by: Rafael J. Wysocki --- include/linux/device.h | 2 ++ 1 file changed, 2 insertions(+) (limited to 'include/linux') diff --git a/include/linux/device.h b/include/linux/device.h index b093405ed525..abf952c82c6d 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -769,6 +769,7 @@ enum device_link_state { * @status: The state of the link (with respect to the presence of drivers). * @flags: Link flags. * @rpm_active: Whether or not the consumer device is runtime-PM-active. + * @kref: Count repeated addition of the same link. * @rcu_head: An RCU head to use for deferred execution of SRCU callbacks. */ struct device_link { @@ -779,6 +780,7 @@ struct device_link { enum device_link_state status; u32 flags; bool rpm_active; + struct kref kref; #ifdef CONFIG_SRCU struct rcu_head rcu_head; #endif -- cgit v1.2.3 From d417e0691ac00d35c4e6b90fc3fc85631a7865ad Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 22 Feb 2018 11:29:44 +0530 Subject: cpufreq: Validate frequency table in the core By design, cpufreq drivers are responsible for calling cpufreq_frequency_table_cpuinfo() from their ->init() callbacks to validate the frequency table. However, if a cpufreq driver is buggy and fails to do so properly, it lead to unexpected behavior of the driver or the cpufreq core at a later point in time. It would be better if the core could validate the frequency table during driver initialization. To that end, introduce cpufreq_table_validate_and_sort() and make the cpufreq core call it right after invoking the ->init() callback of the driver and destroy the cpufreq policy if the table is invalid. For the time being the validation of the table happens twice, once from the driver and then from the core. The individual drivers will be updated separately to drop table validation if they don't need it for other reasons. The frequency table is marked "sorted" or "unsorted" by the new helper now instead of in cpufreq_table_validate_and_show(), as it should only be done after validating the table (which the drivers won't do going forward). Signed-off-by: Viresh Kumar [ rjw: Subject/changelog ] Signed-off-by: Rafael J. Wysocki --- drivers/cpufreq/cpufreq.c | 13 +++++++++---- drivers/cpufreq/freq_table.c | 16 +++++++++++++++- include/linux/cpufreq.h | 1 + 3 files changed, 25 insertions(+), 5 deletions(-) (limited to 'include/linux') diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 8814c572e263..239063fb6afc 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1219,6 +1219,10 @@ static int cpufreq_online(unsigned int cpu) goto out_free_policy; } + ret = cpufreq_table_validate_and_sort(policy); + if (ret) + goto out_exit_policy; + down_write(&policy->rwsem); if (new_policy) { @@ -1249,7 +1253,7 @@ static int cpufreq_online(unsigned int cpu) policy->cur = cpufreq_driver->get(policy->cpu); if (!policy->cur) { pr_err("%s: ->get() failed\n", __func__); - goto out_exit_policy; + goto out_destroy_policy; } } @@ -1296,7 +1300,7 @@ static int cpufreq_online(unsigned int cpu) if (new_policy) { ret = cpufreq_add_dev_interface(policy); if (ret) - goto out_exit_policy; + goto out_destroy_policy; cpufreq_stats_create_table(policy); @@ -1311,7 +1315,7 @@ static int cpufreq_online(unsigned int cpu) __func__, cpu, ret); /* cpufreq_policy_free() will notify based on this */ new_policy = false; - goto out_exit_policy; + goto out_destroy_policy; } up_write(&policy->rwsem); @@ -1326,12 +1330,13 @@ static int cpufreq_online(unsigned int cpu) return 0; -out_exit_policy: +out_destroy_policy: for_each_cpu(j, policy->real_cpus) remove_cpu_dev_symlink(policy, get_cpu_device(j)); up_write(&policy->rwsem); +out_exit_policy: if (cpufreq_driver->exit) cpufreq_driver->exit(policy); diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c index 6d007f824ca7..10e119ae66dd 100644 --- a/drivers/cpufreq/freq_table.c +++ b/drivers/cpufreq/freq_table.c @@ -362,10 +362,24 @@ int cpufreq_table_validate_and_show(struct cpufreq_policy *policy, return ret; policy->freq_table = table; - return set_freq_table_sorted(policy); + return 0; } EXPORT_SYMBOL_GPL(cpufreq_table_validate_and_show); +int cpufreq_table_validate_and_sort(struct cpufreq_policy *policy) +{ + int ret; + + if (!policy->freq_table) + return 0; + + ret = cpufreq_frequency_table_cpuinfo(policy, policy->freq_table); + if (ret) + return ret; + + return set_freq_table_sorted(policy); +} + MODULE_AUTHOR("Dominik Brodowski "); MODULE_DESCRIPTION("CPUfreq frequency table helpers"); MODULE_LICENSE("GPL"); diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h index 21e8d248d956..1fe49724da9e 100644 --- a/include/linux/cpufreq.h +++ b/include/linux/cpufreq.h @@ -962,6 +962,7 @@ extern struct freq_attr cpufreq_freq_attr_scaling_boost_freqs; extern struct freq_attr *cpufreq_generic_attr[]; int cpufreq_table_validate_and_show(struct cpufreq_policy *policy, struct cpufreq_frequency_table *table); +int cpufreq_table_validate_and_sort(struct cpufreq_policy *policy); unsigned int cpufreq_generic_get(unsigned int cpu); int cpufreq_generic_init(struct cpufreq_policy *policy, -- cgit v1.2.3 From 64bdff698092aa6be28c3b248f887022eec77902 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Wed, 14 Mar 2018 12:27:21 +0100 Subject: PM: cpuidle/suspend: Add s2idle usage and time state attributes Add a new attribute group called "s2idle" under the sysfs directory of each cpuidle state that supports the ->enter_s2idle callback and put two new attributes, "usage" and "time", into that group to represent the number of times the given state was requested for suspend-to-idle and the total time spent in suspend-to-idle after requesting that state, respectively. That will allow diagnostic information related to suspend-to-idle to be collected without enabling advanced debug features and analyzing dmesg output. Signed-off-by: Rafael J. Wysocki --- Documentation/ABI/testing/sysfs-devices-system-cpu | 25 ++++++++++ drivers/cpuidle/cpuidle.c | 9 ++++ drivers/cpuidle/sysfs.c | 54 ++++++++++++++++++++++ include/linux/cpuidle.h | 4 ++ 4 files changed, 92 insertions(+) (limited to 'include/linux') diff --git a/Documentation/ABI/testing/sysfs-devices-system-cpu b/Documentation/ABI/testing/sysfs-devices-system-cpu index 4ed63b6cfb15..025b7cf3768d 100644 --- a/Documentation/ABI/testing/sysfs-devices-system-cpu +++ b/Documentation/ABI/testing/sysfs-devices-system-cpu @@ -198,6 +198,31 @@ Description: time (in microseconds) this cpu should spend in this idle state to make the transition worth the effort. +What: /sys/devices/system/cpu/cpuX/cpuidle/stateN/s2idle/ +Date: March 2018 +KernelVersion: v4.17 +Contact: Linux power management list +Description: + Idle state usage statistics related to suspend-to-idle. + + This attribute group is only present for states that can be + used in suspend-to-idle with suspended timekeeping. + +What: /sys/devices/system/cpu/cpuX/cpuidle/stateN/s2idle/time +Date: March 2018 +KernelVersion: v4.17 +Contact: Linux power management list +Description: + Total time spent by the CPU in suspend-to-idle (with scheduler + tick suspended) after requesting this state. + +What: /sys/devices/system/cpu/cpuX/cpuidle/stateN/s2idle/usage +Date: March 2018 +KernelVersion: v4.17 +Contact: Linux power management list +Description: + Total number of times this state has been requested by the CPU + while entering suspend-to-idle. What: /sys/devices/system/cpu/cpu#/cpufreq/* Date: pre-git history diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c index 68a16827f45f..0003e9a02637 100644 --- a/drivers/cpuidle/cpuidle.c +++ b/drivers/cpuidle/cpuidle.c @@ -131,6 +131,10 @@ int cpuidle_find_deepest_state(struct cpuidle_driver *drv, static void enter_s2idle_proper(struct cpuidle_driver *drv, struct cpuidle_device *dev, int index) { + ktime_t time_start, time_end; + + time_start = ns_to_ktime(local_clock()); + /* * trace_suspend_resume() called by tick_freeze() for the last CPU * executing it contains RCU usage regarded as invalid in the idle @@ -152,6 +156,11 @@ static void enter_s2idle_proper(struct cpuidle_driver *drv, */ RCU_NONIDLE(tick_unfreeze()); start_critical_timings(); + + time_end = ns_to_ktime(local_clock()); + + dev->states_usage[index].s2idle_time += ktime_us_delta(time_end, time_start); + dev->states_usage[index].s2idle_usage++; } /** diff --git a/drivers/cpuidle/sysfs.c b/drivers/cpuidle/sysfs.c index ae948b1da93a..e754c7aae7f7 100644 --- a/drivers/cpuidle/sysfs.c +++ b/drivers/cpuidle/sysfs.c @@ -330,6 +330,58 @@ struct cpuidle_state_kobj { struct kobject kobj; }; +#ifdef CONFIG_SUSPEND +#define define_show_state_s2idle_ull_function(_name) \ +static ssize_t show_state_s2idle_##_name(struct cpuidle_state *state, \ + struct cpuidle_state_usage *state_usage, \ + char *buf) \ +{ \ + return sprintf(buf, "%llu\n", state_usage->s2idle_##_name);\ +} + +define_show_state_s2idle_ull_function(usage); +define_show_state_s2idle_ull_function(time); + +#define define_one_state_s2idle_ro(_name, show) \ +static struct cpuidle_state_attr attr_s2idle_##_name = \ + __ATTR(_name, 0444, show, NULL) + +define_one_state_s2idle_ro(usage, show_state_s2idle_usage); +define_one_state_s2idle_ro(time, show_state_s2idle_time); + +static struct attribute *cpuidle_state_s2idle_attrs[] = { + &attr_s2idle_usage.attr, + &attr_s2idle_time.attr, + NULL +}; + +static const struct attribute_group cpuidle_state_s2idle_group = { + .name = "s2idle", + .attrs = cpuidle_state_s2idle_attrs, +}; + +static void cpuidle_add_s2idle_attr_group(struct cpuidle_state_kobj *kobj) +{ + int ret; + + if (!kobj->state->enter_s2idle) + return; + + ret = sysfs_create_group(&kobj->kobj, &cpuidle_state_s2idle_group); + if (ret) + pr_debug("%s: sysfs attribute group not created\n", __func__); +} + +static void cpuidle_remove_s2idle_attr_group(struct cpuidle_state_kobj *kobj) +{ + if (kobj->state->enter_s2idle) + sysfs_remove_group(&kobj->kobj, &cpuidle_state_s2idle_group); +} +#else +static inline void cpuidle_add_s2idle_attr_group(struct cpuidle_state_kobj *kobj) { } +static inline void cpuidle_remove_s2idle_attr_group(struct cpuidle_state_kobj *kobj) { } +#endif /* CONFIG_SUSPEND */ + #define kobj_to_state_obj(k) container_of(k, struct cpuidle_state_kobj, kobj) #define kobj_to_state(k) (kobj_to_state_obj(k)->state) #define kobj_to_state_usage(k) (kobj_to_state_obj(k)->state_usage) @@ -383,6 +435,7 @@ static struct kobj_type ktype_state_cpuidle = { static inline void cpuidle_free_state_kobj(struct cpuidle_device *device, int i) { + cpuidle_remove_s2idle_attr_group(device->kobjs[i]); kobject_put(&device->kobjs[i]->kobj); wait_for_completion(&device->kobjs[i]->kobj_unregister); kfree(device->kobjs[i]); @@ -417,6 +470,7 @@ static int cpuidle_add_state_sysfs(struct cpuidle_device *device) kfree(kobj); goto error_state; } + cpuidle_add_s2idle_attr_group(kobj); kobject_uevent(&kobj->kobj, KOBJ_ADD); device->kobjs[i] = kobj; } diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h index 0b3fc229086c..a806e94c482f 100644 --- a/include/linux/cpuidle.h +++ b/include/linux/cpuidle.h @@ -33,6 +33,10 @@ struct cpuidle_state_usage { unsigned long long disable; unsigned long long usage; unsigned long long time; /* in US */ +#ifdef CONFIG_SUSPEND + unsigned long long s2idle_usage; + unsigned long long s2idle_time; /* in US */ +#endif }; struct cpuidle_state { -- cgit v1.2.3