[BUG,3.12.rc4] Oops: unable to handle kernel paging request during shutdown

Commit Message

Rafael J. Wysocki Oct. 25, 2013, 10:10 a.m. UTC

On Friday, October 25, 2013 11:51:40 AM Rafael J. Wysocki wrote:
> On Friday, October 25, 2013 11:28:02 AM Rafael J. Wysocki wrote:
> > On Friday, October 25, 2013 10:02:22 AM Linus Torvalds wrote:
> > > Adding more people, so quoting the whole email for them.
> > > 
> > > We definitely have some module unload issues. Guys, try the following
> > > a few times to unload modules:
> > > 
> > >     lsmod | grep ' 0 '| cut -d' ' -f1 | xargs sudo rmmod
> > > 
> > > (a few times because unloading one module will then potentially make
> > > other modules unloadable).
> > > 
> > > On my machine, I can trigger this, for example:
> > > 
> > >   ------------[ cut here ]------------
> > >   WARNING: CPU: 0 PID: 3217 at fs/sysfs/file.c:498 sysfs_attr_ns+0x91/0xa0()
> > >   sysfs: kobject (null) without dirent
> > >   Modules linked in: fuse nf_conntrack_broadcast ipt_MASQUERADE ip6t_REJECT xt_$
> > >   CPU: 0 PID: 3217 Comm: rmmod Not tainted 3.12.0-rc6-00284-ge6036c0b8896 #19
> > >   Hardware name: Sony Corporation SVP11213CXB/VAIO, BIOS R0270V7 05/17/2013
> > >    0000000000000009 ffff8800aca35df8 ffffffff8160aab5 ffff8800aca35e40
> > >    ffff8800aca35e30 ffffffff810514b8 ffffffffa013f080 ffff8801194a6040
> > >    0000000000000800 0000000000000000 0000000000c5b3e0 ffff8800aca35e90
> > >   Call Trace:
> > >    [<ffffffff8160aab5>] dump_stack+0x45/0x56
> > >    [<ffffffff810514b8>] warn_slowpath_common+0x78/0xa0
> > >    [<ffffffff81051527>] warn_slowpath_fmt+0x47/0x50
> > >    [<ffffffff810b5960>] ? module_refcount+0xb0/0xb0
> > >    [<ffffffff811e5c61>] sysfs_attr_ns+0x91/0xa0
> > >    [<ffffffff811e5d2a>] sysfs_remove_file+0x1a/0x50
> > >    [<ffffffff814c88a3>] cpufreq_sysfs_remove_file+0x13/0x30
> > >    [<ffffffffa013d350>] acpi_cpufreq_exit+0x2e/0xcde [acpi_cpufreq]
> > >    [<ffffffff810b7d1d>] SyS_delete_module+0x15d/0x2c0
> > >    [<ffffffff81002929>] ? do_notify_resume+0x59/0x90
> > >    [<ffffffff81618f62>] system_call_fastpath+0x16/0x1b
> > >   ---[ end trace f887112caaa5c4ab ]---
> > > 
> > > so at least we have a cpufreq/sysfs interaction bug. There may be others.
> > > 
> > > This particular cpufreq issue may be triggered by the fact that
> > > acpi-cpufreq isn't actually in use (pstate is). Or it might be some
> > > generic cpufreq/sysfs bug. Rafael, Greg, ideas?
> > 
> > I *think* that this indeed is related to acpi-cpufreq being unused.  That said,
> > we've been fixing sysfs-related bugs in cpufreq recently and we may have
> > overlooked something.
> > 
> > I'll have a deeper look at that.
> 
> Well, if the ACPI cpufreq driver is not registered, the exit function of the
> module shouldn't try to unregister it, so I have the appended patch (untested)
> to fix that particular thing.

And in my opinion acpi_cpufreq_init() should just return an error code if it
can't register the driver, like in the (untested) patch below.

Rafael


---
 drivers/cpufreq/acpi-cpufreq.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Viresh Kumar Oct. 25, 2013, 10 a.m. UTC | #1

On 25 October 2013 15:40, Rafael J. Wysocki <[e-mail]> wrote:
> And in my opinion acpi_cpufreq_init() should just return an error code if it
> can't register the driver, like in the (untested) patch below.
>
> Rafael
>
>
> ---
>  drivers/cpufreq/acpi-cpufreq.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> Index: linux-pm/drivers/cpufreq/acpi-cpufreq.c
> ===================================================================
> --- linux-pm.orig/drivers/cpufreq/acpi-cpufreq.c
> +++ linux-pm/drivers/cpufreq/acpi-cpufreq.c
> @@ -988,10 +988,10 @@ static int __init acpi_cpufreq_init(void
>
>         /* don't keep reloading if cpufreq_driver exists */
>         if (cpufreq_get_current_driver())
> -               return 0;
> +               return -EEXIST;
>
>         if (acpi_disabled)
> -               return 0;
> +               return -ENODEV;
>
>         pr_debug("acpi_cpufreq_init\n");

Much better.. And probably the first return has caused the bug
Linus faced..

Acked-by: Viresh Kumar <[e-mail]>
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Linus Torvalds Oct. 25, 2013, 10:07 a.m. UTC | #2

On Fri, Oct 25, 2013 at 11:10 AM, Rafael J. Wysocki <[e-mail]> wrote:
> On Friday, October 25, 2013 11:51:40 AM Rafael J. Wysocki wrote:
>
>         /* don't keep reloading if cpufreq_driver exists */
>         if (cpufreq_get_current_driver())
> -               return 0;
> +               return -EEXIST;
>
>         if (acpi_disabled)
> -               return 0;
> +               return -ENODEV;
>
>         pr_debug("acpi_cpufreq_init\n");

I think:

 - EBUSY is a better error return than EEXIST here/

  - do the ENODEV case first, because claiming something is busy when
you couldn't have used it anyway is kind of stupid)

but yeah, this looks like the right thing. Let's make sure there
aren't any annoying error messages etc printed out as a result of
this, though.

                  Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

Index: linux-pm/drivers/cpufreq/acpi-cpufreq.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/acpi-cpufreq.c
+++ linux-pm/drivers/cpufreq/acpi-cpufreq.c
@@ -988,10 +988,10 @@  static int __init acpi_cpufreq_init(void
 
 	/* don't keep reloading if cpufreq_driver exists */
 	if (cpufreq_get_current_driver())
-		return 0;
+		return -EEXIST;
 
 	if (acpi_disabled)
-		return 0;
+		return -ENODEV;
 
 	pr_debug("acpi_cpufreq_init\n");