From: Viresh Kumar <viresh.kumar@linaro.org> To: "Rafael J. Wysocki" <rjw@rjwysocki.net> Cc: Russell King - ARM Linux <linux@armlinux.org.uk>, linux-pm@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: Kernel warning in cpufreq_add_dev() Date: Sat, 20 Aug 2016 07:16:34 +0530 [thread overview] Message-ID: <20160820014634.GA25143@ubuntu> (raw) In-Reply-To: <1601399.OxUAWWKTJN@vostro.rjw.lan> On 20-08-16, 03:29, Rafael J. Wysocki wrote: > On Friday, August 19, 2016 12:00:32 PM Russell King - ARM Linux wrote: > > While checking the kernel on SA1110 Assabet, CPUFREQ issues a warning: > > > > ------------[ cut here ]------------ > > WARNING: CPU: 0 PID: 1 at /home/rmk/git/linux-rmk/drivers/cpufreq/cpufreq.c:1080 cpufreq_add_dev+0x140/0x62c > > Modules linked in: > > CPU: 0 PID: 1 Comm: swapper Not tainted 4.8.0-rc2+ #883 > > Hardware name: Intel-Assabet > > Backtrace: > > [<c0212190>] (dump_backtrace) from [<c021249c>] (show_stack+0x18/0x1c) > > r6:00000000 r5:c05e87c3 r4:00000000 > > [<c0212484>] (show_stack) from [<c037260c>] (dump_stack+0x20/0x28) > > [<c03725ec>] (dump_stack) from [<c021f4cc>] (__warn+0xd0/0xfc) > > [<c021f3fc>] (__warn) from [<c021f520>] (warn_slowpath_null+0x28/0x30) > > r10:00000000 r8:00000000 r7:00000000 r6:c064525c r5:00000000 r4:00000000 > > [<c021f4f8>] (warn_slowpath_null) from [<c04343a8>] (cpufreq_add_dev+0x140/0x62c) > > [<c0434268>] (cpufreq_add_dev) from [<c03d83f4>] (bus_probe_device+0x5c/0x84) > > r10:00000000 r8:00000000 r7:00000000 r6:c064525c r5:c0657d60 r4:c065a9f8 > > [<c03d8398>] (bus_probe_device) from [<c03d677c>] (device_add+0x390/0x520) > > r6:c0645264 r5:00000000 r4:c064525c > > [<c03d63ec>] (device_add) from [<c03d6a90>] (device_register+0x1c/0x20) > > r10:c0639848 r8:c061e524 r7:00000001 r6:00000000 r5:c064525c r4:c064525c > > [<c03d6a74>] (device_register) from [<c03db5a0>] (register_cpu+0x88/0xac) > > r4:c0645254 > > [<c03db518>] (register_cpu) from [<c061e544>] (topology_init+0x20/0x2c) > > r7:c0660b20 r6:c063f4a0 r5:c0639834 r4:00000000 > > [<c061e524>] (topology_init) from [<c020974c>] (do_one_initcall+0xc0/0x178) > > r4:00000004 > > [<c020968c>] (do_one_initcall) from [<c061be70>] (kernel_init_freeable+0xfc/0x1c4) > > r10:c0639848 r9:00000000 r8:00000088 r7:c0660b20 r6:c063f4a0 r5:c0639834 > > r4:00000004 > > [<c061bd74>] (kernel_init_freeable) from [<c050d730>] (kernel_init+0x10/0xf4) > > r10:00000000 r8:00000000 r7:00000000 r6:00000000 r5:c050d720 r4:00000000 > > [<c050d720>] (kernel_init) from [<c020fcf0>] (ret_from_fork+0x14/0x24) > > r4:00000000 > > ---[ end trace df94656649275917 ]--- > > > > This is because of an incompatibility between the expectations of cpufreq > > and how register_cpu() works: > > > > int register_cpu(struct cpu *cpu, int num) > > { > > ... > > error = device_register(&cpu->dev); > > if (!error) > > per_cpu(cpu_sys_devices, num) = &cpu->dev; > > > > When the device is registered via device_register(), any subsystems > > registered for the cpu_subsys will have their "add_dev" method called. > > > > The cpufreq add_dev, via cpufreq_online() and cpufreq_policy_alloc(), > > tries to get the CPU device: > > > > static struct cpufreq_policy *cpufreq_policy_alloc(unsigned int cpu) > > { > > struct device *dev = get_cpu_device(cpu); > > if (WARN_ON(!dev)) > > return NULL; > > > > but this fails: > > > > struct device *get_cpu_device(unsigned cpu) > > { > > if (cpu < nr_cpu_ids && cpu_possible(cpu)) > > return per_cpu(cpu_sys_devices, cpu); > > > > because the percpu data has not yet been written - it'll be written > > after a successful device registration. So, using get_cpu_device() > > from within cpufreq_add_dev() is broken, and results in the above > > kernel warning. Hmm, I am wondering why is your case special here and why we never saw the same behavior ? Is this because the driver is registered as arch_initcall() ? In all the cases that I have seen at least, cpufreq_add_dev() doesn't get called via the path you mentioned, but only during the cpufreq driver is registered. > Ironically enough, cpufreq_policy_alloc() doesn't even use the value of dev > for anything other than the check, so we can simply get rid of it (as per the > appended patch). > > add_cpu_dev_symlink() will still be problematic, though, if I'm not mistaken. Right, it will be. We try to create links for all the *real* (currently plugged in) CPUs on policy creation and that needs the kobjects of those devices. -- viresh
WARNING: multiple messages have this Message-ID (diff)
From: viresh.kumar@linaro.org (Viresh Kumar) To: linux-arm-kernel@lists.infradead.org Subject: Kernel warning in cpufreq_add_dev() Date: Sat, 20 Aug 2016 07:16:34 +0530 [thread overview] Message-ID: <20160820014634.GA25143@ubuntu> (raw) In-Reply-To: <1601399.OxUAWWKTJN@vostro.rjw.lan> On 20-08-16, 03:29, Rafael J. Wysocki wrote: > On Friday, August 19, 2016 12:00:32 PM Russell King - ARM Linux wrote: > > While checking the kernel on SA1110 Assabet, CPUFREQ issues a warning: > > > > ------------[ cut here ]------------ > > WARNING: CPU: 0 PID: 1 at /home/rmk/git/linux-rmk/drivers/cpufreq/cpufreq.c:1080 cpufreq_add_dev+0x140/0x62c > > Modules linked in: > > CPU: 0 PID: 1 Comm: swapper Not tainted 4.8.0-rc2+ #883 > > Hardware name: Intel-Assabet > > Backtrace: > > [<c0212190>] (dump_backtrace) from [<c021249c>] (show_stack+0x18/0x1c) > > r6:00000000 r5:c05e87c3 r4:00000000 > > [<c0212484>] (show_stack) from [<c037260c>] (dump_stack+0x20/0x28) > > [<c03725ec>] (dump_stack) from [<c021f4cc>] (__warn+0xd0/0xfc) > > [<c021f3fc>] (__warn) from [<c021f520>] (warn_slowpath_null+0x28/0x30) > > r10:00000000 r8:00000000 r7:00000000 r6:c064525c r5:00000000 r4:00000000 > > [<c021f4f8>] (warn_slowpath_null) from [<c04343a8>] (cpufreq_add_dev+0x140/0x62c) > > [<c0434268>] (cpufreq_add_dev) from [<c03d83f4>] (bus_probe_device+0x5c/0x84) > > r10:00000000 r8:00000000 r7:00000000 r6:c064525c r5:c0657d60 r4:c065a9f8 > > [<c03d8398>] (bus_probe_device) from [<c03d677c>] (device_add+0x390/0x520) > > r6:c0645264 r5:00000000 r4:c064525c > > [<c03d63ec>] (device_add) from [<c03d6a90>] (device_register+0x1c/0x20) > > r10:c0639848 r8:c061e524 r7:00000001 r6:00000000 r5:c064525c r4:c064525c > > [<c03d6a74>] (device_register) from [<c03db5a0>] (register_cpu+0x88/0xac) > > r4:c0645254 > > [<c03db518>] (register_cpu) from [<c061e544>] (topology_init+0x20/0x2c) > > r7:c0660b20 r6:c063f4a0 r5:c0639834 r4:00000000 > > [<c061e524>] (topology_init) from [<c020974c>] (do_one_initcall+0xc0/0x178) > > r4:00000004 > > [<c020968c>] (do_one_initcall) from [<c061be70>] (kernel_init_freeable+0xfc/0x1c4) > > r10:c0639848 r9:00000000 r8:00000088 r7:c0660b20 r6:c063f4a0 r5:c0639834 > > r4:00000004 > > [<c061bd74>] (kernel_init_freeable) from [<c050d730>] (kernel_init+0x10/0xf4) > > r10:00000000 r8:00000000 r7:00000000 r6:00000000 r5:c050d720 r4:00000000 > > [<c050d720>] (kernel_init) from [<c020fcf0>] (ret_from_fork+0x14/0x24) > > r4:00000000 > > ---[ end trace df94656649275917 ]--- > > > > This is because of an incompatibility between the expectations of cpufreq > > and how register_cpu() works: > > > > int register_cpu(struct cpu *cpu, int num) > > { > > ... > > error = device_register(&cpu->dev); > > if (!error) > > per_cpu(cpu_sys_devices, num) = &cpu->dev; > > > > When the device is registered via device_register(), any subsystems > > registered for the cpu_subsys will have their "add_dev" method called. > > > > The cpufreq add_dev, via cpufreq_online() and cpufreq_policy_alloc(), > > tries to get the CPU device: > > > > static struct cpufreq_policy *cpufreq_policy_alloc(unsigned int cpu) > > { > > struct device *dev = get_cpu_device(cpu); > > if (WARN_ON(!dev)) > > return NULL; > > > > but this fails: > > > > struct device *get_cpu_device(unsigned cpu) > > { > > if (cpu < nr_cpu_ids && cpu_possible(cpu)) > > return per_cpu(cpu_sys_devices, cpu); > > > > because the percpu data has not yet been written - it'll be written > > after a successful device registration. So, using get_cpu_device() > > from within cpufreq_add_dev() is broken, and results in the above > > kernel warning. Hmm, I am wondering why is your case special here and why we never saw the same behavior ? Is this because the driver is registered as arch_initcall() ? In all the cases that I have seen at least, cpufreq_add_dev() doesn't get called via the path you mentioned, but only during the cpufreq driver is registered. > Ironically enough, cpufreq_policy_alloc() doesn't even use the value of dev > for anything other than the check, so we can simply get rid of it (as per the > appended patch). > > add_cpu_dev_symlink() will still be problematic, though, if I'm not mistaken. Right, it will be. We try to create links for all the *real* (currently plugged in) CPUs on policy creation and that needs the kobjects of those devices. -- viresh
next prev parent reply other threads:[~2016-08-20 1:46 UTC|newest] Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-08-19 11:00 Kernel warning in cpufreq_add_dev() Russell King - ARM Linux 2016-08-19 11:00 ` Russell King - ARM Linux 2016-08-20 1:29 ` Rafael J. Wysocki 2016-08-20 1:29 ` Rafael J. Wysocki 2016-08-20 1:46 ` Viresh Kumar [this message] 2016-08-20 1:46 ` Viresh Kumar 2016-08-22 17:32 ` Rafael J. Wysocki 2016-08-22 17:32 ` Rafael J. Wysocki 2016-08-24 13:13 ` Viresh Kumar 2016-08-24 13:13 ` Viresh Kumar 2016-08-31 1:26 ` Rafael J. Wysocki 2016-08-31 1:26 ` Rafael J. Wysocki 2016-08-31 4:11 ` Viresh Kumar 2016-08-31 4:11 ` Viresh Kumar 2016-08-31 11:58 ` Rafael J. Wysocki 2016-08-31 11:58 ` Rafael J. Wysocki 2016-09-09 9:57 ` Viresh Kumar 2016-09-09 9:57 ` Viresh Kumar 2016-09-09 9:54 ` [PATCH] cpufreq: create link to policy only for registered CPUs Viresh Kumar 2016-09-09 9:54 ` Viresh Kumar 2016-09-09 11:16 ` Russell King - ARM Linux 2016-09-09 11:16 ` Russell King - ARM Linux 2016-09-09 11:22 ` Viresh Kumar 2016-09-09 11:22 ` Viresh Kumar 2016-09-09 11:28 ` Russell King - ARM Linux 2016-09-09 11:28 ` Russell King - ARM Linux 2016-09-09 11:34 ` Viresh Kumar 2016-09-09 11:34 ` Viresh Kumar 2016-09-09 12:53 ` Russell King - ARM Linux 2016-09-09 12:53 ` Russell King - ARM Linux 2016-09-12 6:37 ` [PATCH V2] " Viresh Kumar 2016-09-12 6:37 ` Viresh Kumar 2016-09-14 1:00 ` Rafael J. Wysocki 2016-09-14 1:00 ` Rafael J. Wysocki
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=20160820014634.GA25143@ubuntu \ --to=viresh.kumar@linaro.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-pm@vger.kernel.org \ --cc=linux@armlinux.org.uk \ --cc=rjw@rjwysocki.net \ /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: linkBe 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.