From: Doug Smythies <dsmythies@telus.net>
To: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Linux PM list <linux-pm@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
lukasz.luba@arm.com, "Rafael J. Wysocki" <rafael@kernel.org>,
gregkh@linuxfoundation.org, dsmythies <dsmythies@telus.net>
Subject: Re: [PATCH v6 3/7] powercap/drivers/dtpm: Simplify the dtpm table
Date: Fri, 26 Nov 2021 09:08:25 -0800 [thread overview]
Message-ID: <CAAYoRsURO1tf03nfiki1uaXYEmTKQyYKUeTyKW+vefrVzCO7jg@mail.gmail.com> (raw)
In-Reply-To: <20210401183654.27214-3-daniel.lezcano@linaro.org>
[-- Attachment #1: Type: text/plain, Size: 4041 bytes --]
Hi Daniel,
This patch introduces a regression, at least on my test system.
I can no longer change CPU frequency scaling drivers, for example
from intel_cpufreq (A.K.A intel_pstate in passive mode) to intel_pstate
(A.K.A. active mode). The task just hangs forever.
I bisected the kernel and got this commit as the result.
As a double check, I reverted this commit:
7a89d7eacf8e84f2afb94db5ae9d9f9faa93f01c
on kernel 5.16-rc2 and the issue was resolved.
While your email is fairly old, I observe that it was only included as of
kernel 5.16-rc1.
Command Example that never completes:
$ echo passive | sudo tee /sys/devices/system/cpu/intel_pstate/status
syslog excerpt attached.
... Doug
On Thu, Apr 1, 2021 at 12:24 PM Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
>
> The dtpm table is an array of pointers, that forces the user of the
> table to define initdata along with the declaration of the table
> entry. It is more efficient to create an array of dtpm structure, so
> the declaration of the table entry can be done by initializing the
> different fields.
>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>
> ---
> drivers/powercap/dtpm.c | 4 ++--
> drivers/powercap/dtpm_cpu.c | 4 +++-
> include/linux/dtpm.h | 20 +++++++++-----------
> 3 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/powercap/dtpm.c b/drivers/powercap/dtpm.c
> index a707cc2965a1..d9aac107c927 100644
> --- a/drivers/powercap/dtpm.c
> +++ b/drivers/powercap/dtpm.c
> @@ -583,7 +583,7 @@ int dtpm_register(const char *name, struct dtpm *dtpm, struct dtpm *parent)
>
> static int __init dtpm_init(void)
> {
> - struct dtpm_descr **dtpm_descr;
> + struct dtpm_descr *dtpm_descr;
>
> pct = powercap_register_control_type(NULL, "dtpm", NULL);
> if (IS_ERR(pct)) {
> @@ -592,7 +592,7 @@ static int __init dtpm_init(void)
> }
>
> for_each_dtpm_table(dtpm_descr)
> - (*dtpm_descr)->init(*dtpm_descr);
> + dtpm_descr->init();
>
> return 0;
> }
> diff --git a/drivers/powercap/dtpm_cpu.c b/drivers/powercap/dtpm_cpu.c
> index 9deafd16a197..74b39a1082e5 100644
> --- a/drivers/powercap/dtpm_cpu.c
> +++ b/drivers/powercap/dtpm_cpu.c
> @@ -204,7 +204,7 @@ static int cpuhp_dtpm_cpu_online(unsigned int cpu)
> return ret;
> }
>
> -int dtpm_register_cpu(struct dtpm *parent)
> +static int __init dtpm_cpu_init(void)
> {
> int ret;
>
> @@ -241,3 +241,5 @@ int dtpm_register_cpu(struct dtpm *parent)
>
> return 0;
> }
> +
> +DTPM_DECLARE(dtpm_cpu, dtpm_cpu_init);
> diff --git a/include/linux/dtpm.h b/include/linux/dtpm.h
> index 577c71d4e098..2ec2821111d1 100644
> --- a/include/linux/dtpm.h
> +++ b/include/linux/dtpm.h
> @@ -33,25 +33,23 @@ struct dtpm_ops {
> void (*release)(struct dtpm *);
> };
>
> -struct dtpm_descr;
> -
> -typedef int (*dtpm_init_t)(struct dtpm_descr *);
> +typedef int (*dtpm_init_t)(void);
>
> struct dtpm_descr {
> - struct dtpm *parent;
> - const char *name;
> dtpm_init_t init;
> };
>
> /* Init section thermal table */
> -extern struct dtpm_descr *__dtpm_table[];
> -extern struct dtpm_descr *__dtpm_table_end[];
> +extern struct dtpm_descr __dtpm_table[];
> +extern struct dtpm_descr __dtpm_table_end[];
>
> -#define DTPM_TABLE_ENTRY(name) \
> - static typeof(name) *__dtpm_table_entry_##name \
> - __used __section("__dtpm_table") = &name
> +#define DTPM_TABLE_ENTRY(name, __init) \
> + static struct dtpm_descr __dtpm_table_entry_##name \
> + __used __section("__dtpm_table") = { \
> + .init = __init, \
> + }
>
> -#define DTPM_DECLARE(name) DTPM_TABLE_ENTRY(name)
> +#define DTPM_DECLARE(name, init) DTPM_TABLE_ENTRY(name, init)
>
> #define for_each_dtpm_table(__dtpm) \
> for (__dtpm = __dtpm_table; \
> --
> 2.17.1
>
[-- Attachment #2: syslog_example.txt --]
[-- Type: text/plain, Size: 3973 bytes --]
Nov 26 07:31:13 s19 systemd[1]: Finished Update UTMP about System Runlevel Changes.
Nov 26 07:31:13 s19 systemd[1]: Startup finished in 10.788s (firmware) + 12.351s (loader) + 1.808s (kernel) + 1min 38.047s (userspace) = 2min 2.995s.
Nov 26 07:31:58 s19 kernel: [ 144.568026] INFO: task tee:1270 blocked for more than 20 seconds.
Nov 26 07:31:58 s19 kernel: [ 144.568069] Not tainted 5.15.0-rc1-n13 #977 <<<< 13th kernel of bisection
Nov 26 07:31:58 s19 kernel: [ 144.568096] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
Nov 26 07:31:58 s19 kernel: [ 144.568135] task:tee state:D stack: 0 pid: 1270 ppid: 1269 flags:0x00004000
Nov 26 07:31:58 s19 kernel: [ 144.568141] Call Trace:
Nov 26 07:31:58 s19 kernel: [ 144.568145] __schedule+0xce9/0x13f0
Nov 26 07:31:58 s19 kernel: [ 144.568155] ? __slab_free+0x217/0x400
Nov 26 07:31:58 s19 kernel: [ 144.568162] ? __slab_free+0x217/0x400
Nov 26 07:31:58 s19 kernel: [ 144.568167] ? radix_tree_lookup+0xd/0x10
Nov 26 07:31:58 s19 kernel: [ 144.568172] ? idr_find+0xf/0x20
Nov 26 07:31:58 s19 kernel: [ 144.568177] ? get_work_pool+0x2d/0x40
Nov 26 07:31:58 s19 kernel: [ 144.568182] schedule+0x67/0xe0
Nov 26 07:31:58 s19 kernel: [ 144.568187] schedule_timeout+0x200/0x290
Nov 26 07:31:58 s19 kernel: [ 144.568191] ? freq_qos_apply+0x2d/0x50
Nov 26 07:31:58 s19 kernel: [ 144.568198] wait_for_completion+0x8b/0xf0
Nov 26 07:31:58 s19 kernel: [ 144.568202] cpufreq_policy_put_kobj+0x4d/0x90
Nov 26 07:31:58 s19 kernel: [ 144.568207] cpufreq_policy_free+0x12f/0x160
Nov 26 07:31:58 s19 kernel: [ 144.568211] cpufreq_remove_dev+0x91/0xb0
Nov 26 07:31:58 s19 kernel: [ 144.568216] subsys_interface_unregister+0x9d/0x100
Nov 26 07:31:58 s19 kernel: [ 144.568223] cpufreq_unregister_driver+0x35/0xd0
Nov 26 07:31:58 s19 kernel: [ 144.568227] store_status+0x102/0x1c0
Nov 26 07:31:58 s19 kernel: [ 144.568232] kobj_attr_store+0xf/0x20
Nov 26 07:31:58 s19 kernel: [ 144.568237] sysfs_kf_write+0x3b/0x50
Nov 26 07:31:58 s19 kernel: [ 144.568242] kernfs_fop_write_iter+0x135/0x1c0
Nov 26 07:31:58 s19 kernel: [ 144.568246] new_sync_write+0x11d/0x1b0
Nov 26 07:31:58 s19 kernel: [ 144.568253] vfs_write+0x1a7/0x290
Nov 26 07:31:58 s19 kernel: [ 144.568256] ksys_write+0x67/0xe0
Nov 26 07:31:58 s19 kernel: [ 144.568259] __x64_sys_write+0x1a/0x20
Nov 26 07:31:58 s19 kernel: [ 144.568261] do_syscall_64+0x59/0xc0
Nov 26 07:31:58 s19 kernel: [ 144.568268] ? vfs_write+0x1a7/0x290
Nov 26 07:31:58 s19 kernel: [ 144.568271] ? exit_to_user_mode_prepare+0x3d/0x1c0
Nov 26 07:31:58 s19 kernel: [ 144.568277] ? ksys_write+0x67/0xe0
Nov 26 07:31:58 s19 kernel: [ 144.568280] ? syscall_exit_to_user_mode+0x27/0x50
Nov 26 07:31:58 s19 kernel: [ 144.568283] ? __x64_sys_write+0x1a/0x20
Nov 26 07:31:58 s19 kernel: [ 144.568286] ? do_syscall_64+0x69/0xc0
Nov 26 07:31:58 s19 kernel: [ 144.568291] entry_SYSCALL_64_after_hwframe+0x44/0xae
Nov 26 07:31:58 s19 kernel: [ 144.568296] RIP: 0033:0x7fca551ca1e7
Nov 26 07:31:58 s19 kernel: [ 144.568300] RSP: 002b:00007fffad47a6b8 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
Nov 26 07:31:58 s19 kernel: [ 144.568304] RAX: ffffffffffffffda RBX: 0000000000000007 RCX: 00007fca551ca1e7
Nov 26 07:31:58 s19 kernel: [ 144.568307] RDX: 0000000000000007 RSI: 00007fffad47a7b0 RDI: 0000000000000003
Nov 26 07:31:58 s19 kernel: [ 144.568309] RBP: 00007fffad47a7b0 R08: 0000000000000007 R09: 0000000000000001
Nov 26 07:31:58 s19 kernel: [ 144.568311] R10: 00000000000001b6 R11: 0000000000000246 R12: 0000000000000007
Nov 26 07:31:58 s19 kernel: [ 144.568313] R13: 000055eb03a6f4a0 R14: 0000000000000007 R15: 00007fca552a58a0
Nov 26 07:32:13 s19 systemd[1]: Stopping Session 1 of user doug.
Nov 26 07:32:13 s19 systemd[1]: Stopping Session 3 of user doug.
Nov 26 07:32:13 s19 systemd[1]: Stopping Session 4 of user doug.
Nov 26 07:32:13 s19 systemd[1]: Removed slice system-modprobe.slice.
...
next prev parent reply other threads:[~2021-11-26 17:31 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-01 18:36 [PATCH v6 1/7] powercap/drivers/dtpm: Encapsulate even more the code Daniel Lezcano
2021-04-01 18:36 ` [PATCH v6 2/7] powercap/drivers/dtpm: Create a registering system Daniel Lezcano
2021-04-01 19:28 ` Greg KH
2021-04-01 22:08 ` Daniel Lezcano
2021-04-02 8:02 ` Greg KH
2021-04-02 11:10 ` Daniel Lezcano
2021-04-02 11:48 ` Greg KH
2021-04-01 18:36 ` [PATCH v6 3/7] powercap/drivers/dtpm: Simplify the dtpm table Daniel Lezcano
2021-11-26 17:08 ` Doug Smythies [this message]
2021-11-26 17:21 ` Rafael J. Wysocki
2021-11-26 17:43 ` Daniel Lezcano
2021-11-26 18:18 ` Rafael J. Wysocki
2021-11-26 21:56 ` Doug Smythies
2021-11-26 23:05 ` Daniel Lezcano
2021-11-26 23:08 ` [PATCH] powercap/drivers/dtpm: Disable dtpm at boot time Daniel Lezcano
2021-11-26 23:10 ` Daniel Lezcano
2021-11-27 1:13 ` Doug Smythies
2021-12-01 18:56 ` Rafael J. Wysocki
2021-11-26 19:10 ` [PATCH v6 3/7] powercap/drivers/dtpm: Simplify the dtpm table Doug Smythies
2021-11-26 19:29 ` Rafael J. Wysocki
2021-11-30 16:46 ` Daniel Lezcano
2021-11-26 17:40 ` Daniel Lezcano
2021-11-26 18:23 ` Rafael J. Wysocki
2021-04-01 18:36 ` [PATCH v6 4/7] powercap/drivers/dtpm: Use container_of instead of a private data field Daniel Lezcano
2021-04-01 18:36 ` [PATCH v6 5/7] powercap/drivers/dtpm: Scale the power with the load Daniel Lezcano
2021-04-01 18:36 ` [PATCH v6 6/7] powercap/drivers/dtpm: Export the symbols for the modules Daniel Lezcano
2021-04-01 18:36 ` [PATCH v6 7/7] powercap/drivers/dtpm: Allow dtpm node device creation through configfs Daniel Lezcano
2021-04-01 19:37 ` Greg KH
2021-04-02 10:54 ` Daniel Lezcano
2021-04-02 10:54 ` Daniel Lezcano
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=CAAYoRsURO1tf03nfiki1uaXYEmTKQyYKUeTyKW+vefrVzCO7jg@mail.gmail.com \
--to=dsmythies@telus.net \
--cc=daniel.lezcano@linaro.org \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=lukasz.luba@arm.com \
--cc=rafael@kernel.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).