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
>
