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

[Viresh Kumar <viresh.kumar@linaro.org>]

On 25 October 2013 15:21, Rafael J. Wysocki <rjw@rjwysocki.net> 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:

>> > 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 agree.. Recently I have tested few other cpufreq drivers for module
insert/removal along with governors insertion/removal... So that part must
be okay..

> 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.
>
> Rafael
>
>
> ---
>  drivers/cpufreq/acpi-cpufreq.c |   10 ++++++++--
>  1 file changed, 8 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
> @@ -982,6 +982,8 @@ static void __exit acpi_cpufreq_boost_ex
>         }
>  }
>
> +static bool driver_registered;
> +
>  static int __init acpi_cpufreq_init(void)
>  {
>         int ret;
> @@ -1021,10 +1023,12 @@ static int __init acpi_cpufreq_init(void
>  #endif
>
>         ret = cpufreq_register_driver(&acpi_cpufreq_driver);
> -       if (ret)
> +       if (ret) {
>                 free_acpi_perf_data();
> -       else
> +       } else {
>                 acpi_cpufreq_boost_init();
> +               driver_registered = true;
> +       }
>
>         return ret;
>  }
> @@ -1032,6 +1036,8 @@ static int __init acpi_cpufreq_init(void
>  static void __exit acpi_cpufreq_exit(void)
>  {
>         pr_debug("acpi_cpufreq_exit\n");
> +       if (!driver_registered)
> +               return;
>
>         acpi_cpufreq_boost_exit();

Looks like the right solution here. But this kind of issues look to
be somewhat generic, doesn't they? And probably most of the
drivers would be struggling with such issues.. They are working
because we normally have something like this in core unregister
parts:

int cpufreq_unregister_driver(struct cpufreq_driver *driver)
{
...
        if (!cpufreq_driver || (driver != cpufreq_driver))
                return -EINVAL;
....

So, even in this case if we could actually check return value of
cpufreq_unregister_driver() and then do the other stuff, then we
wouldn't require this extra variable..

But the problem is the order in which things happen. Would this
be a big problem if we do unregister first and then
acpi_cpufreq_boost_exit(), based on what unregister returned?