All of lore.kernel.org
 help / color / mirror / Atom feed
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.
...


  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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.