From: Viresh Kumar <viresh.kumar@linaro.org>
To: "Srivatsa S. Bhat" <srivatsa.bhat@linux.vnet.ibm.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>,
Meelis Roos <mroos@linux.ee>,
"cpufreq@vger.kernel.org" <cpufreq@vger.kernel.org>,
"linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] cpufreq: Catch double invocations of cpufreq_freq_transition_begin/end
Date: Tue, 29 Apr 2014 18:53:40 +0530 [thread overview]
Message-ID: <CAKohpo=UYR3sgtutEBRcvRtXjvNpph9wu2TgoM1168HXQrWjLA@mail.gmail.com> (raw)
In-Reply-To: <20140429130506.7052.54268.stgit@srivatsabhat.in.ibm.com>
On 29 April 2014 18:36, Srivatsa S. Bhat
<srivatsa.bhat@linux.vnet.ibm.com> wrote:
> Some cpufreq drivers were redundantly invoking the _begin() and _end()
> APIs around frequency transitions, and this double invocation (one from
> the cpufreq core and the other from the cpufreq driver) used to result
> in a self-deadlock, leading to system hangs during boot. (The _begin()
> API makes contending callers wait until the previous invocation is
> complete. Hence, the cpufreq driver would end up waiting on itself!).
>
> Now all such drivers have been fixed, but debugging this issue was not
> very straight-forward (even lockdep didn't catch this). So let us add a
> debug infrastructure to the cpufreq core to catch such issues more easily
> in the future.
>
> We add a new field called 'transition_task' to the policy structure, to keep
> track of the task which is performing the frequency transition. Using this
> field, we make note of this task during _begin() and print a warning if we
> find a case where the same task is calling _begin() again, before completing
> the previous frequency transition using the corresponding _end().
>
> We have left out ASYNC_NOTIFICATION drivers from this debug infrastructure
> for 2 reasons:
>
> 1. At the moment, we have no way to avoid a particular scenario where this
> debug infrastructure can emit false-positive warnings for such drivers.
> The scenario is depicted below:
>
>
> Task A Task B
>
> /* 1st freq transition */
> Invoke _begin() {
> ...
> ...
> }
>
> Change the frequency
>
> /* 2nd freq transition */
> Invoke _begin() {
> ... //waiting for B to
> ... //finish _end() for
> ... //the 1st transition
> ... | Got interrupt for successful
> ... | change of frequency (1st one).
> ... |
> ... | /* 1st freq transition */
> ... | Invoke _end() {
> ... | ...
> ... V }
> ...
> ...
> }
>
> This scenario is actually deadlock-free because, once Task A changes the
> frequency, it is Task B's responsibility to invoke the corresponding
> _end() for the 1st frequency transition. Hence it is perfectly legal for
> Task A to go ahead and attempt another frequency transition in the meantime.
> (Of course it won't be able to proceed until Task B finishes the 1st _end(),
> but this doesn't cause a deadlock or a hang).
>
> The debug infrastructure cannot handle this scenario and will treat it as
> a deadlock and print a warning. To avoid this, we exclude such drivers
> from the purview of this code.
>
> 2. Luckily, we don't _need_ this infrastructure for ASYNC_NOTIFICATION drivers
> at all! The cpufreq core does not automatically invoke the _begin() and
> _end() APIs during frequency transitions in such drivers. Thus, the driver
> alone is responsible for invoking _begin()/_end() and hence there shouldn't
> be any conflicts which lead to double invocations. So, we can skip these
> drivers, since the probability that such drivers will hit this problem is
> extremely low, as outlined above.
>
> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
> ---
>
> v2: Removed the coverage of ASYNC_NOTIFICATION drivers, in order to avoid
> false-positives.
>
> drivers/cpufreq/cpufreq.c | 7 +++++++
> include/linux/cpufreq.h | 1 +
> 2 files changed, 8 insertions(+)
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
prev parent reply other threads:[~2014-04-29 13:23 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-04-29 13:06 [PATCH v2] cpufreq: Catch double invocations of cpufreq_freq_transition_begin/end Srivatsa S. Bhat
2014-04-29 13:09 ` Meelis Roos
2014-04-29 13:18 ` Srivatsa S. Bhat
2014-04-29 13:23 ` Viresh Kumar [this message]
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='CAKohpo=UYR3sgtutEBRcvRtXjvNpph9wu2TgoM1168HXQrWjLA@mail.gmail.com' \
--to=viresh.kumar@linaro.org \
--cc=cpufreq@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=mroos@linux.ee \
--cc=rjw@rjwysocki.net \
--cc=srivatsa.bhat@linux.vnet.ibm.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).