From: Marek Szyprowski <m.szyprowski@samsung.com>
To: Chanwoo Choi <cwchoi00@gmail.com>,
linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: johnson.wang@mediatek.com, mka@chromium.org, wenst@chromium.org,
jia-wei.chang@mediatek.com, andrew-sh.cheng@mediatek.com,
hsinyi@chromium.org, saravanak@google.com, cw00.choi@samsung.com,
myungjoo.ham@samsung.com, kyungmin.park@samsung.com,
Saravana Kannan <skannan@codeaurora.org>,
Sibi Sankar <sibis@codeaurora.org>,
Krzysztof Kozlowski <krzk@kernel.org>,
'Linux Samsung SOC' <linux-samsung-soc@vger.kernel.org>
Subject: Re: [PATCH v4 2/4] PM / devfreq: Add cpu based scaling support to passive governor
Date: Fri, 13 May 2022 00:34:50 +0200 [thread overview]
Message-ID: <7ecd0d6a-cbf2-448c-92ef-2adb5184342c@samsung.com> (raw)
In-Reply-To: <20220511093554.17535-3-cw00.choi@samsung.com>
Hi Chanwoo,
On 11.05.2022 11:35, Chanwoo Choi wrote:
> From: Saravana Kannan <skannan@codeaurora.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@chromium.org>
> Tested-by: Johnson Wang <johnson.wang@mediatek.com>
> Signed-off-by: Saravana Kannan <skannan@codeaurora.org>
> [Sibi: Integrated cpu-freqmap governor into passive_governor]
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> [Chanwoo: Fix conflict with latest code and cleanup code]
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.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
next prev parent reply other threads:[~2022-05-12 22:35 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-11 9:35 [PATCH v4 0/4] PM / devfreq: Add cpu based scaling support to passive governor Chanwoo Choi
2022-05-11 9:35 ` [PATCH v4 1/4] PM / devfreq: Export devfreq_get_freq_range symbol within devfreq Chanwoo Choi
2022-05-11 9:35 ` [PATCH v4 2/4] PM / devfreq: Add cpu based scaling support to passive governor Chanwoo Choi
[not found] ` <CGME20220512223450eucas1p203b702e114dd2cd1bafcfd7d4c80b638@eucas1p2.samsung.com>
2022-05-12 22:34 ` Marek Szyprowski [this message]
2022-05-13 4:46 ` Chanwoo Choi
2022-05-11 9:35 ` [PATCH v4 3/4] PM / devfreq: passive: Reduce duplicate code when passive_devfreq case Chanwoo Choi
2022-05-11 9:35 ` [PATCH v4 4/4] PM / devfreq: passive: Keep cpufreq_policy for possible cpus Chanwoo Choi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7ecd0d6a-cbf2-448c-92ef-2adb5184342c@samsung.com \
--to=m.szyprowski@samsung.com \
--cc=andrew-sh.cheng@mediatek.com \
--cc=cw00.choi@samsung.com \
--cc=cwchoi00@gmail.com \
--cc=hsinyi@chromium.org \
--cc=jia-wei.chang@mediatek.com \
--cc=johnson.wang@mediatek.com \
--cc=krzk@kernel.org \
--cc=kyungmin.park@samsung.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=linux-samsung-soc@vger.kernel.org \
--cc=mka@chromium.org \
--cc=myungjoo.ham@samsung.com \
--cc=saravanak@google.com \
--cc=sibis@codeaurora.org \
--cc=skannan@codeaurora.org \
--cc=wenst@chromium.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).