On Wed, Sep 12, 2018 at 01:58:39PM +0530, Viresh Kumar wrote:
Hello,
Niklas Cassle recently reported some regressions with his Qcom cpufreq driver where he was getting some errors while creating the OPPs tables.
After looking into it I realized that the OPP core incorrectly creates multiple OPP tables for the devices even if they are sharing the OPP table in DT. This happens when the request comes using different CPU devices. For example, dev_pm_opp_set_supported_hw() getting called using CPU0 and dev_pm_opp_of_add_table() getting called using CPU1.
This series redesigns the internals of the OPP core to fix that. The redesign has simplified the core itself though.
@Niklas: Can you please confirm that this series fixes the issues you have reported ? I have already tested it on Hikey960.
Hello Viresh,
This fixes the OPP error messages that I previously reported.
However, I also tested to add a duplicate OPP to the opp-table.
Before this series, I got the first two error prints. After this series, I get the first two error prints + the use after free splat.
[ 5.693273] cpu cpu0: _opp_is_duplicate: duplicate OPPs detected. Existing: freq: 307200000, volt: 905000, enabled: 1. New: freq: 307200000, volt: 904000, enabled: 1 [ 5.695602] cpu cpu0: _of_add_opp_table_v2: Failed to add OPP, -17 [ 5.713673] ------------[ cut here ]------------ [ 5.715124] refcount_t: underflow; use-after-free. [ 5.720047] WARNING: CPU: 3 PID: 35 at lib/refcount.c:280 refcount_dec_not_one+0x9c/0xc0 [ 5.723463] Modules linked in: [ 5.731461] CPU: 3 PID: 35 Comm: kworker/3:1 Tainted: G W 4.19.0-rc3-00219-g 3f2e8f8e1fc5-dirty #63 [ 5.734688] Hardware name: Qualcomm Technologies, Inc. DB820c (DT) [ 5.744940] Workqueue: events deferred_probe_work_func [ 5.750810] pstate: 40000005 (nZcv daif -PAN -UAO) [ 5.755973] pc : refcount_dec_not_one+0x9c/0xc0 [ 5.760710] lr : refcount_dec_not_one+0x9c/0xc0 [ 5.765018] sp : ffff000009f8b3a0 [ 5.769469] x29: ffff000009f8b3a0 x28: 0000000000000000 [ 5.773052] x27: 0000000000000000 x26: 0000000000000001 [ 5.778428] x25: ffff8000d5347200 x24: ffff0000092f00e0 [ 5.783722] x23: ffff0000092efcf8 x22: ffff000008f5d250 [ 5.789023] x21: ffff0000094f9000 x20: ffff8000d5347264 [ 5.794313] x19: ffff000009637960 x18: ffffffffffffffff [ 5.799605] x17: 0000000000000000 x16: 0000000000000000 [ 5.804900] x15: ffff0000094f96c8 x14: 0720072007200720 [ 5.810199] x13: 0720072007200720 x12: 0720072007200720 [ 5.815491] x11: 0720072007200720 x10: 0720072007200720 [ 5.820799] x9 : 0720072007200720 x8 : 072007200720072e [ 5.826081] x7 : 0765076507720766 x6 : ffff8000da028f00 [ 5.831377] x5 : 0000000000000000 x4 : 0000000000000000 [ 5.836666] x3 : ffffffffffffffff x2 : ffff000009512480 [ 5.841971] x1 : a4c264660aaf4100 x0 : 0000000000000000 [ 5.847274] Call trace: [ 5.852544] refcount_dec_not_one+0x9c/0xc0 [ 5.854792] refcount_dec_and_mutex_lock+0x18/0x70 [ 5.859055] _put_opp_list_kref+0x28/0x50 [ 5.863822] _dev_pm_opp_find_and_remove_table+0x24/0x88 [ 5.867996] _dev_pm_opp_cpumask_remove_table+0x4c/0x98 [ 5.873369] dev_pm_opp_of_cpumask_add_table+0x98/0x100 [ 5.878315] cpufreq_init+0xe4/0x3a8 [ 5.883376] cpufreq_online+0xc4/0x6d0 [ 5.887186] cpufreq_add_dev+0xa8/0xb8 [ 5.890835] subsys_interface_register+0x9c/0x100 [ 5.894540] cpufreq_register_driver+0x190/0x278 [ 5.899344] dt_cpufreq_probe+0xa0/0x1e0 [ 5.903969] platform_drv_probe+0x50/0xa0 [ 5.907840] really_probe+0x260/0x3b8 [ 5.911720] driver_probe_device+0x5c/0x148 [ 5.915398] __device_attach_driver+0xa8/0x160 [ 5.919456] bus_for_each_drv+0x64/0xc8 [ 5.923875] __device_attach+0xd8/0x158 [ 5.927625] device_initial_probe+0x10/0x18 [ 5.931450] bus_probe_device+0x90/0x98 [ 5.935650] device_add+0x440/0x668 [ 5.939416] platform_device_add+0x124/0x2d0 [ 5.942977] platform_device_register_full+0xf8/0x118 [ 5.947571] qcom_cpufreq_kryo_probe+0x27c/0x320 [ 5.952445] platform_drv_probe+0x50/0xa0 [ 5.957066] really_probe+0x260/0x3b8 [ 5.960939] driver_probe_device+0x5c/0x148 [ 5.964612] __device_attach_driver+0xa8/0x160 [ 5.968687] bus_for_each_drv+0x64/0xc8 [ 5.973086] __device_attach+0xd8/0x158 [ 5.976831] device_initial_probe+0x10/0x18 [ 5.980657] bus_probe_device+0x90/0x98 [ 5.984824] deferred_probe_work_func+0x88/0xe0 [ 5.988801] process_one_work+0x1e0/0x318 [ 5.993243] worker_thread+0x228/0x450 [ 5.997345] kthread+0x128/0x130 [ 6.000973] ret_from_fork+0x10/0x18 [ 6.004313] ---[ end trace 5715d70f8f823685 ]---
Kind regards, Niklas
-- viresh
Viresh Kumar (11): OPP: Free OPP table properly on performance state irregularities OPP: Protect dev_list with opp_table lock OPP: Pass index to _of_init_opp_table() OPP: Parse OPP table's DT properties from _of_init_opp_table() OPP: Don't take OPP table's kref for static OPPs OPP: Create separate kref for static OPPs list cpufreq: mvebu: Remove OPPs using dev_pm_opp_remove() OPP: Don't remove dynamic OPPs from _dev_pm_opp_remove_table() OPP: Use a single mechanism to free the OPP table OPP: Prevent creating multiple OPP tables for devices sharing OPP nodes OPP: Pass OPP table to _of_add_opp_table_v{1|2}()
drivers/cpufreq/mvebu-cpufreq.c | 9 +- drivers/opp/core.c | 147 ++++++++++++++++--------- drivers/opp/cpu.c | 11 +- drivers/opp/of.c | 186 +++++++++++++++++--------------- drivers/opp/opp.h | 19 ++-- include/linux/pm_opp.h | 6 ++ 6 files changed, 221 insertions(+), 157 deletions(-)
-- 2.18.0.rc1.242.g61856ae69a2c