[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7ecd0d6a-cbf2-448c-92ef-2adb5184342c@samsung.com>
Date: Fri, 13 May 2022 00:34:50 +0200
From: Marek Szyprowski <m.szyprowski@...sung.com>
To: Chanwoo Choi <cwchoi00@...il.com>, linux-pm@...r.kernel.org,
linux-kernel@...r.kernel.org
Cc: johnson.wang@...iatek.com, mka@...omium.org, wenst@...omium.org,
jia-wei.chang@...iatek.com, andrew-sh.cheng@...iatek.com,
hsinyi@...omium.org, saravanak@...gle.com, cw00.choi@...sung.com,
myungjoo.ham@...sung.com, kyungmin.park@...sung.com,
Saravana Kannan <skannan@...eaurora.org>,
Sibi Sankar <sibis@...eaurora.org>,
Krzysztof Kozlowski <krzk@...nel.org>,
'Linux Samsung SOC' <linux-samsung-soc@...r.kernel.org>
Subject: Re: [PATCH v4 2/4] PM / devfreq: Add cpu based scaling support to
passive governor
Hi Chanwoo,
On 11.05.2022 11:35, Chanwoo Choi wrote:
> From: Saravana Kannan <skannan@...eaurora.org>
>
> Many CPU architectures have caches that can scale independent of the
> CPUs. Frequency scaling of the caches is necessary to make sure that the
> cache is not a performance bottleneck that leads to poor performance and
> power. The same idea applies for RAM/DDR.
>
> To achieve this, this patch adds support for cpu based scaling to the
> passive governor. This is accomplished by taking the current frequency
> of each CPU frequency domain and then adjust the frequency of the cache
> (or any devfreq device) based on the frequency of the CPUs. It listens
> to CPU frequency transition notifiers to keep itself up to date on the
> current CPU frequency.
>
> To decide the frequency of the device, the governor does one of the
> following:
> * Derives the optimal devfreq device opp from required-opps property of
> the parent cpu opp_table.
>
> * Scales the device frequency in proportion to the CPU frequency. So, if
> the CPUs are running at their max frequency, the device runs at its
> max frequency. If the CPUs are running at their min frequency, the
> device runs at its min frequency. It is interpolated for frequencies
> in between.
>
> Tested-by: Chen-Yu Tsai <wenst@...omium.org>
> Tested-by: Johnson Wang <johnson.wang@...iatek.com>
> Signed-off-by: Saravana Kannan <skannan@...eaurora.org>
> [Sibi: Integrated cpu-freqmap governor into passive_governor]
> Signed-off-by: Sibi Sankar <sibis@...eaurora.org>
> [Chanwoo: Fix conflict with latest code and cleanup code]
> Signed-off-by: Chanwoo Choi <cw00.choi@...sung.com>
This patch landed in today's linux next-20220512 as commit 2ab415d4e4e6
("PM / devfreq: Add cpu based scaling support to passive governor").
It triggers the following NULL pointer dereference on Exynos based boards:
exynos-bus: new bus device registered: soc:bus-leftbus (100000 KHz ~
200000 KHz)
exynos-bus: new bus device registered: soc:bus-rightbus (100000 KHz ~
200000 KHz)
exynos-bus: new bus device registered: soc:bus-display (160000 KHz ~
200000 KHz)
exynos-bus: new bus device registered: soc:bus-fsys (100000 KHz ~ 134000
KHz)
exynos-bus: new bus device registered: soc:bus-peri ( 50000 KHz ~ 100000
KHz)
exynos-bus: new bus device registered: soc:bus-mfc (100000 KHz ~ 200000 KHz)
8<--- cut here ---
Unable to handle kernel NULL pointer dereference at virtual address 0000003c
[0000003c] *pgd=00000000
Internal error: Oops: 5 [#1] PREEMPT SMP ARM
Modules linked in:
CPU: 3 PID: 8 Comm: kworker/u8:0 Not tainted 5.18.0-rc6-next-20220512 #5014
Hardware name: Samsung Exynos (Flattened Device Tree)
Workqueue: devfreq_wq devfreq_monitor
PC is at __mutex_lock+0x48/0x948
LR is at lock_is_held_type+0x104/0x1a4
pc : [<c0b93098>] lr : [<c0b8f2ec>] psr: 60000053
sp : f0889dc0 ip : 600000d3 fp : c1dca624
r10: c1dca44c r9 : 00000000 r8 : c1984104
r7 : c1d7f000 r6 : 00000000 r5 : 00000001 r4 : 00000008
r3 : 00000000 r2 : 00000000 r1 : 2de44000 r0 : 00000000
Flags: nZCv IRQs on FIQs off Mode SVC_32 ISA ARM Segment none
Control: 10c5387d Table: 4000404a DAC: 00000051
Register r0 information: NULL pointer
Register r1 information: non-paged memory
Register r2 information: NULL pointer
Register r3 information: NULL pointer
Register r4 information: non-paged memory
Register r5 information: non-paged memory
Register r6 information: NULL pointer
Register r7 information: slab task_struct start c1d7f000 pointer offset 0
Register r8 information: non-slab/vmalloc memory
Register r9 information: NULL pointer
Register r10 information: slab kmalloc-2k start c1dca000 pointer offset
1100 size 2048
Register r11 information: slab kmalloc-2k start c1dca000 pointer offset
1572 size 2048
Register r12 information: non-paged memory
Process kworker/u8:0 (pid: 8, stack limit = 0x(ptrval))
Stack: (0xf0889dc0 to 0xf088a000)
...
__mutex_lock from mutex_lock_nested+0x1c/0x24
mutex_lock_nested from devfreq_passive_notifier_call+0x24/0x90
devfreq_passive_notifier_call from srcu_notifier_call_chain+0x98/0x114
srcu_notifier_call_chain from devfreq_set_target+0x6c/0x304
devfreq_set_target from devfreq_update_target+0x98/0xe8
devfreq_update_target from devfreq_monitor+0x28/0x1c0
devfreq_monitor from process_one_work+0x288/0x774
process_one_work from worker_thread+0x44/0x504
worker_thread from kthread+0xf4/0x128
kthread from ret_from_fork+0x14/0x2c
Exception stack(0xf0889fb0 to 0xf0889ff8)
...
---[ end trace 0000000000000000 ]---
The issue is caused by the lack of setting devfreq_passive_data->this
pointer in devfreq_passive_register_notifier. However, after adding:
@@ -395,6 +395,9 @@ static int devfreq_passive_register_notifier(struct
devfreq *devfreq)
if (!parent)
return -EPROBE_DEFER;
+ if (!p_data->this)
+ p_data->this = devfreq;
+
nb->notifier_call = devfreq_passive_notifier_call;
return devfreq_register_notifier(parent, nb,
DEVFREQ_TRANSITION_NOTIFIER);
}
the NULL pointer dereference is gone, but I see the following warnings
on Odroid U3 board, which were not present before this patch:
devfreq soc:bus-acp: failed to update devfreq using passive governor
devfreq soc:bus-c2c: failed to update devfreq using passive governor
devfreq soc:bus-acp: failed to update devfreq using passive governor
devfreq soc:bus-c2c: failed to update devfreq using passive governor
> ---
> drivers/devfreq/governor.h | 22 +++
> drivers/devfreq/governor_passive.c | 297 +++++++++++++++++++++++++++--
> include/linux/devfreq.h | 17 +-
> 3 files changed, 322 insertions(+), 14 deletions(-)
> ...
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
Powered by blists - more mailing lists