Re: [PATCH v6 3/7] powercap/drivers/dtpm: Simplify the dtpm table

From: Doug Smythies
Date: Fri Nov 26 2021 - 12:31:42 EST

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@xxxxxxxxxx> 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@xxxxxxxxxx>
> Reviewed-by: Lukasz Luba <lukasz.luba@xxxxxxx>
> ---
> 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
>
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.
...