All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bo Yan <byan@nvidia.com>
To: viresh.kumar@linaro.org, rjw@rjwysocki.net, sgurrappadi@nvidia.com
Cc: linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
	Bo Yan <byan@nvidia.com>
Subject: [PATCH] cpufreq: skip cpufreq resume if it's not suspended
Date: Tue, 23 Jan 2018 13:57:55 -0800	[thread overview]
Message-ID: <1516744675-21233-1-git-send-email-byan@nvidia.com> (raw)

cpufreq_resume can be called even without preceding cpufreq_suspend.
This can happen in following scenario:

    suspend_devices_and_enter
       --> dpm_suspend_start
          --> dpm_prepare
              --> device_prepare : this function errors out
          --> dpm_suspend: this is skipped due to dpm_prepare failure
                           this means cpufreq_suspend is skipped over
       --> goto Recover_platform, due to previous error
       --> goto Resume_devices
       --> dpm_resume_end
           --> dpm_resume
               --> cpufreq_resume

In case schedutil is used as frequency governor, cpufreq_resume will
eventually call sugov_start, which does following:

    memset(sg_cpu, 0, sizeof(*sg_cpu));
    ....

This effectively erases function pointer for frequency update, causing
crash later on. The function pointer would have been set correctly if
subsequent cpufreq_add_update_util_hook runs successfully, but that
function returns earlier because cpufreq_suspend was not called:

    if (WARN_ON(per_cpu(cpufreq_update_util_data, cpu)))
		return;

Ideally, suspend should succeed, then things will be fine. But even
in case of suspend failure, system should not crash.

The fix is to check cpufreq_suspended first, if it's false, that means
cpufreq_suspend was not called in the first place, so do not resume
cpufreq.

Signed-off-by: Bo Yan <byan@nvidia.com>
---
 drivers/cpufreq/cpufreq.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 41d148af7748..95b1c4afe14e 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1680,6 +1680,10 @@ void cpufreq_resume(void)
 	if (!cpufreq_driver)
 		return;
 
+	if (unlikely(!cpufreq_suspended)) {
+		pr_warn("%s: resume after failing suspend\n", __func__);
+		return;
+	}
 	cpufreq_suspended = false;
 
 	if (!has_target() && !cpufreq_driver->resume)
-- 
2.7.4

WARNING: multiple messages have this Message-ID (diff)
From: Bo Yan <byan@nvidia.com>
To: <viresh.kumar@linaro.org>, <rjw@rjwysocki.net>, <sgurrappadi@nvidia.com>
Cc: <linux-pm@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	Bo Yan <byan@nvidia.com>
Subject: [PATCH] cpufreq: skip cpufreq resume if it's not suspended
Date: Tue, 23 Jan 2018 13:57:55 -0800	[thread overview]
Message-ID: <1516744675-21233-1-git-send-email-byan@nvidia.com> (raw)

cpufreq_resume can be called even without preceding cpufreq_suspend.
This can happen in following scenario:

    suspend_devices_and_enter
       --> dpm_suspend_start
          --> dpm_prepare
              --> device_prepare : this function errors out
          --> dpm_suspend: this is skipped due to dpm_prepare failure
                           this means cpufreq_suspend is skipped over
       --> goto Recover_platform, due to previous error
       --> goto Resume_devices
       --> dpm_resume_end
           --> dpm_resume
               --> cpufreq_resume

In case schedutil is used as frequency governor, cpufreq_resume will
eventually call sugov_start, which does following:

    memset(sg_cpu, 0, sizeof(*sg_cpu));
    ....

This effectively erases function pointer for frequency update, causing
crash later on. The function pointer would have been set correctly if
subsequent cpufreq_add_update_util_hook runs successfully, but that
function returns earlier because cpufreq_suspend was not called:

    if (WARN_ON(per_cpu(cpufreq_update_util_data, cpu)))
		return;

Ideally, suspend should succeed, then things will be fine. But even
in case of suspend failure, system should not crash.

The fix is to check cpufreq_suspended first, if it's false, that means
cpufreq_suspend was not called in the first place, so do not resume
cpufreq.

Signed-off-by: Bo Yan <byan@nvidia.com>
---
 drivers/cpufreq/cpufreq.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 41d148af7748..95b1c4afe14e 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1680,6 +1680,10 @@ void cpufreq_resume(void)
 	if (!cpufreq_driver)
 		return;
 
+	if (unlikely(!cpufreq_suspended)) {
+		pr_warn("%s: resume after failing suspend\n", __func__);
+		return;
+	}
 	cpufreq_suspended = false;
 
 	if (!has_target() && !cpufreq_driver->resume)
-- 
2.7.4

             reply	other threads:[~2018-01-23 21:57 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-23 21:57 Bo Yan [this message]
2018-01-23 21:57 ` [PATCH] cpufreq: skip cpufreq resume if it's not suspended Bo Yan
2018-01-24  2:02 ` Rafael J. Wysocki
2018-01-24 20:53   ` Bo Yan
2018-01-24 20:53     ` Bo Yan
2018-02-02 11:54     ` Rafael J. Wysocki
2018-02-02 19:34       ` Saravana Kannan
2018-02-02 21:28         ` Bo Yan
2018-02-02 21:28           ` Bo Yan
2018-02-05  4:01           ` Viresh Kumar
2018-02-05  8:50             ` Rafael J. Wysocki
2018-02-05  9:05               ` Viresh Kumar
2018-02-15 21:27                 ` Saravana Kannan
2018-02-15 22:06                   ` Rafael J. Wysocki
2018-01-25 19:15   ` [PATCH v2] " Bo Yan
2018-01-25 19:15     ` Bo Yan
2018-02-05  9:19 ` [PATCH] " Rafael J. Wysocki
2018-02-05  9:23   ` Viresh Kumar

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=1516744675-21233-1-git-send-email-byan@nvidia.com \
    --to=byan@nvidia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=rjw@rjwysocki.net \
    --cc=sgurrappadi@nvidia.com \
    --cc=viresh.kumar@linaro.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.