suspend broken in next-20190704 on Thinkpad X60

suspend broken in next-20190704 on Thinkpad X60

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 251 bytes --]

Hi!

Suspend is broken in next-20190704 on Thinkpad X60. It very very
probably worked ok in 20190701.



-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: suspend broken in next-20190704 on Thinkpad X60

[Rafael J. Wysocki]

On Thu, Jul 4, 2019 at 9:20 PM Pavel Machek <pavel@ucw.cz> wrote:
>
> Hi!
>
> Suspend is broken in next-20190704 on Thinkpad X60.

Broken in what way?  Any details?

> It very very probably worked ok in 20190701.

Well, please try the linux-next branch from linux-pm.git
(git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git)
alone and see if that fails.

Re: suspend broken in next-20190704 on Thinkpad X60

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 2394 bytes --]

On Fri 2019-07-05 00:59:35, Rafael J. Wysocki wrote:
> On Thu, Jul 4, 2019 at 9:20 PM Pavel Machek <pavel@ucw.cz> wrote:
> >
> > Hi!
> >
> > Suspend is broken in next-20190704 on Thinkpad X60.
> 
> Broken in what way?  Any details?
> 
> > It very very probably worked ok in 20190701.
> 
> Well, please try the linux-next branch from linux-pm.git
> (git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git)
> alone and see if that fails.

So... let me try this one?

commit	1e2a4c9019eb53f62790fadf86c14a54f4cf4888 (patch)
tree	cb5339fcaae2166832f91f4ce9f40575cc6cb6e5
parent	3836c60c063581294c3a82f8cbccf3f702951358 (diff)
parent	0a811974f3f79eea299af79c29595d8e1cb80a15 (diff)
download
linux-pm-1e2a4c9019eb53f62790fadf86c14a54f4cf4888.tar.gz
Merge branch 'pm-cpufreq-new' into
linux-nexttestinglinux-nextbleeding-edge
* pm-cpufreq-new:

That one is broken, too.

pavel@amd:~$ sudo pm-suspend

Machine suspends, resumes, but I don't get my prompt back.

Nothing suspect in dmesg:

[   63.925151] usb 5-1: reset full-speed USB device number 2 using
uhci_hcd
[   67.105121] ata1: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
[   67.106401] ata1.00: ACPI cmd ef/02:00:00:00:00:a0 (SET FEATURES)
succeeded
[   67.106410] ata1.00: ACPI cmd f5/00:00:00:00:00:a0 (SECURITY FREEZE
LOCK) filtered out
[   67.106418] ata1.00: ACPI cmd ef/10:03:00:00:00:a0 (SET FEATURES)
filtered out
[   67.108575] ata1.00: ACPI cmd ef/02:00:00:00:00:a0 (SET FEATURES)
succeeded
[   67.108585] ata1.00: ACPI cmd f5/00:00:00:00:00:a0 (SECURITY FREEZE
LOCK) filtered out
[   67.108593] ata1.00: ACPI cmd ef/10:03:00:00:00:a0 (SET FEATURES)
filtered out
[   67.109152] ata1.00: configured for UDMA/133
[   71.672932] PM: resume devices took 8.668 seconds
[   71.673955] OOM killer enabled.
[   71.673961] Restarting tasks ... done.
[   73.970718] wlan0: authenticate with 30:b5:c2:f5:9f:1e
[   73.972610] wlan0: send auth to 30:b5:c2:f5:9f:1e (try 1/3)
[   73.977518] wlan0: authenticated
[   73.985092] wlan0: associate with 30:b5:c2:f5:9f:1e (try 1/3)
[   73.989844] wlan0: RX AssocResp from 30:b5:c2:f5:9f:1e (capab=0x431
status=0 aid=2)
[   74.002908] wlan0: associated
pavel@amd:~$

									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: suspend broken in next-20190704 on Thinkpad X60

[Rafael J. Wysocki]

On Fri, Jul 5, 2019 at 8:50 PM Pavel Machek <pavel@ucw.cz> wrote:
>
> On Fri 2019-07-05 00:59:35, Rafael J. Wysocki wrote:
> > On Thu, Jul 4, 2019 at 9:20 PM Pavel Machek <pavel@ucw.cz> wrote:
> > >
> > > Hi!
> > >
> > > Suspend is broken in next-20190704 on Thinkpad X60.
> >
> > Broken in what way?  Any details?
> >
> > > It very very probably worked ok in 20190701.
> >
> > Well, please try the linux-next branch from linux-pm.git
> > (git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git)
> > alone and see if that fails.
>
> So... let me try this one?
>
> commit  1e2a4c9019eb53f62790fadf86c14a54f4cf4888 (patch)
> tree    cb5339fcaae2166832f91f4ce9f40575cc6cb6e5
> parent  3836c60c063581294c3a82f8cbccf3f702951358 (diff)
> parent  0a811974f3f79eea299af79c29595d8e1cb80a15 (diff)
> download
> linux-pm-1e2a4c9019eb53f62790fadf86c14a54f4cf4888.tar.gz
> Merge branch 'pm-cpufreq-new' into
> linux-nexttestinglinux-nextbleeding-edge
> * pm-cpufreq-new:
>
> That one is broken, too.
>
> pavel@amd:~$ sudo pm-suspend
>
> Machine suspends, resumes, but I don't get my prompt back.

I'm not sure what you mean here.  I'm guessing that you don't get back
to the console from which you ran the pm-suspend command, but is X
restored, for example?  Is there any way to get into the system in
that state?

Anyway, if 5.2-rc7 is OK, something in this branch causes the problem
to happen for you.

I would try

https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/commit/?h=linux-next&id=f012a132824fc870b90980540f727c76fc72e244

to narrow down the scope somewhat.

Re: suspend broken in next-20190704 on Thinkpad X60

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1893 bytes --]

Hi!

> > > > Suspend is broken in next-20190704 on Thinkpad X60.
> > >
> > > Broken in what way?  Any details?
> > >
> > > > It very very probably worked ok in 20190701.
> > >
> > > Well, please try the linux-next branch from linux-pm.git
> > > (git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git)
> > > alone and see if that fails.
> >
> > So... let me try this one?
> >
> > commit  1e2a4c9019eb53f62790fadf86c14a54f4cf4888 (patch)
> > tree    cb5339fcaae2166832f91f4ce9f40575cc6cb6e5
> > parent  3836c60c063581294c3a82f8cbccf3f702951358 (diff)
> > parent  0a811974f3f79eea299af79c29595d8e1cb80a15 (diff)
> > download
> > linux-pm-1e2a4c9019eb53f62790fadf86c14a54f4cf4888.tar.gz
> > Merge branch 'pm-cpufreq-new' into
> > linux-nexttestinglinux-nextbleeding-edge
> > * pm-cpufreq-new:
> >
> > That one is broken, too.
> >
> > pavel@amd:~$ sudo pm-suspend
> >
> > Machine suspends, resumes, but I don't get my prompt back.
> 
> I'm not sure what you mean here.  I'm guessing that you don't get back
> to the console from which you ran the pm-suspend command, but is X
> restored, for example?  Is there any way to get into the system in
> that state?

X is restored, and the rest of system works, but "pm-suspend" command
never completes, so I don't get shell prompt back in that window.

> Anyway, if 5.2-rc7 is OK, something in this branch causes the problem
> to happen for you.
> 
> I would try
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/commit/?h=linux-next&id=f012a132824fc870b90980540f727c76fc72e244
> 
> to narrow down the scope somewhat.

next-20190701 is ok, next-20190704 is broken. Can we use that to
narrow it down?

Best regards,
							Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: suspend broken in next-20190704 on Thinkpad X60

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1257 bytes --]

Hi!

> > commit  1e2a4c9019eb53f62790fadf86c14a54f4cf4888 (patch)
> > tree    cb5339fcaae2166832f91f4ce9f40575cc6cb6e5
> > parent  3836c60c063581294c3a82f8cbccf3f702951358 (diff)
> > parent  0a811974f3f79eea299af79c29595d8e1cb80a15 (diff)
> > download
> > linux-pm-1e2a4c9019eb53f62790fadf86c14a54f4cf4888.tar.gz
> > Merge branch 'pm-cpufreq-new' into
> > linux-nexttestinglinux-nextbleeding-edge
> > * pm-cpufreq-new:
> >
> > That one is broken, too.
> >
> > pavel@amd:~$ sudo pm-suspend
> >
> > Machine suspends, resumes, but I don't get my prompt back.
> 
> I'm not sure what you mean here.  I'm guessing that you don't get back
> to the console from which you ran the pm-suspend command, but is X
> restored, for example?  Is there any way to get into the system in
> that state?
> 
> Anyway, if 5.2-rc7 is OK, something in this branch causes the problem
> to happen for you.
> 
> I would try
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/commit/?h=linux-next&id=f012a132824fc870b90980540f727c76fc72e244

That one is good.

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: suspend broken in next-20190704 on Thinkpad X60

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1774 bytes --]

Hi!

> > > > Suspend is broken in next-20190704 on Thinkpad X60.
> > >
> > > Broken in what way?  Any details?
> > >
> > > > It very very probably worked ok in 20190701.
> > >
> > > Well, please try the linux-next branch from linux-pm.git
> > > (git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git)
> > > alone and see if that fails.
> >
> > So... let me try this one?
> >
> > commit  1e2a4c9019eb53f62790fadf86c14a54f4cf4888 (patch)
...
> I'm not sure what you mean here.  I'm guessing that you don't get back
> to the console from which you ran the pm-suspend command, but is X
> restored, for example?  Is there any way to get into the system in
> that state?
> 
> Anyway, if 5.2-rc7 is OK, something in this branch causes the problem
> to happen for you.
> 
> I would try
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/commit/?h=linux-next&id=f012a132824fc870b90980540f727c76fc72e244
> 
> to narrow down the scope somewhat.

pavel@amd:/data/l/linux-next-32$ git bisect log
# bad: [1e2a4c9019eb53f62790fadf86c14a54f4cf4888] Merge branch
'pm-cpufreq-new' into linux-next
# good: [f012a132824fc870b90980540f727c76fc72e244] Merge branches
'acpica', 'acpi-osl', 'acpi-tables', 'acpi-misc' and 'acpi-tools' into
linux-next
git bisect start '1e2a4c9019eb53f62790fadf86c14a54f4cf4888'
'f012a132824fc870b90980540f727c76fc72e244'
# good: [48a8a5f9a326d1c1a5505d51fb98086e5003f37e] Add linux-next
specific files for 20190701
git bisect good 48a8a5f9a326d1c1a5505d51fb98086e5003f37e

I think I can handle this... when I'm near the AC power.

								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

cpufreq notifiers break suspend -- Re: suspend broken in next-20190704 on Thinkpad X60

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 2666 bytes --]

Hi!

> Anyway, if 5.2-rc7 is OK, something in this branch causes the problem
> to happen for you.
> 
> I would try
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/commit/?h=linux-next&id=f012a132824fc870b90980540f727c76fc72e244
> 
> to narrow down the scope somewhat.

Bisect says:

572542c81dec533b7dd3778ea9f5949a00595f68 is the first bad commit
Author: Viresh Kumar <viresh.kumar@linaro.org>

    cpufreq: Register notifiers with the PM QoS framework

    This registers the notifiers for min/max frequency constraints
    with the

 Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
 Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
 Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

Unfortunately, it does not revert cleanly:

pavel@duo:/data/l/linux-next-32$ git show
572542c81dec533b7dd3778ea9f5949a00595f68 | patch -REsp1
6 out of 11 hunks FAILED -- saving rejects to file
drivers/cpufreq/cpufreq.c.rej


pavel@duo:/data/l/linux-next-32$  git bisect log
# bad: [1e2a4c9019eb53f62790fadf86c14a54f4cf4888] Merge branch
'pm-cpufreq-new' into linux-next
# good: [f012a132824fc870b90980540f727c76fc72e244] Merge branches
'acpica', 'acpi-osl', 'acpi-tables', 'acpi-misc' and 'acpi-tools' into
linux-next
git bisect start '1e2a4c9019eb53f62790fadf86c14a54f4cf4888'
'f012a132824fc870b90980540f727c76fc72e244'
# good: [48a8a5f9a326d1c1a5505d51fb98086e5003f37e] Add linux-next
specific files for 20190701
git bisect good 48a8a5f9a326d1c1a5505d51fb98086e5003f37e
# good: [96021e491dbf30bd1c5c1a753992838c8d8d00cb] Merge branches
'acpi-apei', 'acpi-doc', 'acpi-soc' and 'acpi-pmic' into linux-next
git bisect good 96021e491dbf30bd1c5c1a753992838c8d8d00cb
# bad: [141467868c1f7bf1c4e8394a39d47d4db38cd2f1] cpufreq:
intel_pstate: Reuse refresh_frequency_limits()
git bisect bad 141467868c1f7bf1c4e8394a39d47d4db38cd2f1
# good: [2a79ea5ec53973c8711b54d33ace5c77659dc8f8] PM / QOS: Pass
request type to dev_pm_qos_read_value()
git bisect good 2a79ea5ec53973c8711b54d33ace5c77659dc8f8
# bad: [572542c81dec533b7dd3778ea9f5949a00595f68] cpufreq: Register
notifiers with the PM QoS framework
git bisect bad 572542c81dec533b7dd3778ea9f5949a00595f68
# good: [208637b37824c8956fe28d277835a403ee35fa84] PM / QoS: Add
support for MIN/MAX frequency constraints
git bisect good 208637b37824c8956fe28d277835a403ee35fa84
# first bad commit: [572542c81dec533b7dd3778ea9f5949a00595f68]
cpufreq: Register notifiers with the PM QoS framework

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: cpufreq notifiers break suspend -- Re: suspend broken in next-20190704 on Thinkpad X60

[Viresh Kumar]

On 06-07-19, 22:30, Pavel Machek wrote:
> Hi!
> 
> > Anyway, if 5.2-rc7 is OK, something in this branch causes the problem
> > to happen for you.
> > 
> > I would try
> > 
> > https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/commit/?h=linux-next&id=f012a132824fc870b90980540f727c76fc72e244
> > 
> > to narrow down the scope somewhat.

I couldn't find the original mail, what exactly is the problem with
suspend in your case ?

> Bisect says:
> 
> 572542c81dec533b7dd3778ea9f5949a00595f68 is the first bad commit
> Author: Viresh Kumar <viresh.kumar@linaro.org>
> 
>     cpufreq: Register notifiers with the PM QoS framework
> 
>     This registers the notifiers for min/max frequency constraints
>     with the
> 
>  Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
>  Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
>  Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> 
> Unfortunately, it does not revert cleanly:

I tried following on my ARM board (both single policy and multiple
policy configurations):

rtcwake --seconds 5 -v -m mem

And everything worked as expected. Please make sure the top commit of
my series in pm/linux-next is, some issues were fixed on Friday:

0a811974f3f7 cpufreq: Add QoS requests for userspace constraints

-- 
viresh

Re: cpufreq notifiers break suspend -- Re: suspend broken in next-20190704 on Thinkpad X60

[Rafael J. Wysocki]

On Mon, Jul 8, 2019 at 5:05 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 06-07-19, 22:30, Pavel Machek wrote:
> > Hi!
> >
> > > Anyway, if 5.2-rc7 is OK, something in this branch causes the problem
> > > to happen for you.
> > >
> > > I would try
> > >
> > > https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/commit/?h=linux-next&id=f012a132824fc870b90980540f727c76fc72e244
> > >
> > > to narrow down the scope somewhat.
>
> I couldn't find the original mail, what exactly is the problem with
> suspend in your case ?

Something unusual:

https://lore.kernel.org/linux-pm/20190706190123.GA11603@amd/T/#mca22dd7c1e8836e9253702df9f56a68ab65192a4

> > Bisect says:
> >
> > 572542c81dec533b7dd3778ea9f5949a00595f68 is the first bad commit
> > Author: Viresh Kumar <viresh.kumar@linaro.org>
> >
> >     cpufreq: Register notifiers with the PM QoS framework
> >
> >     This registers the notifiers for min/max frequency constraints
> >     with the
> >
> >  Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
> >  Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
> >  Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> >
> > Unfortunately, it does not revert cleanly:
>
> I tried following on my ARM board (both single policy and multiple
> policy configurations):
>
> rtcwake --seconds 5 -v -m mem
>
> And everything worked as expected. Please make sure the top commit of
> my series in pm/linux-next is, some issues were fixed on Friday:
>
> 0a811974f3f7 cpufreq: Add QoS requests for userspace constraints

Pavel has tested the latest version of the patch series AFAICS.

The locking added by the commit in question to
refresh_frequency_limits() requires an update of
cpufreq_update_policy(), or it will deadlock in there because of the
lock acquired by cpufreq_cpu_get() if I haven't missed anything.

Re: cpufreq notifiers break suspend -- Re: suspend broken in next-20190704 on Thinkpad X60

[Viresh Kumar]

On 08-07-19, 10:28, Rafael J. Wysocki wrote:
> Pavel has tested the latest version of the patch series AFAICS.
> 
> The locking added by the commit in question to
> refresh_frequency_limits() requires an update of
> cpufreq_update_policy(), or it will deadlock in there because of the
> lock acquired by cpufreq_cpu_get() if I haven't missed anything.

Ah, looks quite straight forward.

@Pavel: Can you please try this diff ?

-------------------------8<-------------------------

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 9f68d0f306b8..4d6043ee7834 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1109,16 +1109,12 @@ void refresh_frequency_limits(struct cpufreq_policy *policy)
 {
        struct cpufreq_policy new_policy;
 
-       down_write(&policy->rwsem);
-
        if (!policy_is_inactive(policy)) {
                new_policy = *policy;
                pr_debug("updating policy for CPU %u\n", policy->cpu);
 
                cpufreq_set_policy(policy, &new_policy);
        }
-
-       up_write(&policy->rwsem);
 }
 EXPORT_SYMBOL(refresh_frequency_limits);
 
@@ -1128,7 +1124,9 @@ static void handle_update(struct work_struct *work)
                container_of(work, struct cpufreq_policy, update);
 
        pr_debug("handle_update for cpu %u called\n", policy->cpu);
+       down_write(&policy->rwsem);
        refresh_frequency_limits(policy);
+       up_write(&policy->rwsem);
 }
 
-------------------------8<-------------------------

Though it makes me wonder why I didn't hit this thing. I was using the
cpu_cooling device the other day, which calls cpufreq_update_policy()
very frequently on heat-up. And I had a hair dryer blowing over my
board to heat it up. Lemme check that again :)

@Rafael: You want me to send a new diff patch with Fixes tag this time
if this works out fine ?

-- 
viresh

Re: cpufreq notifiers break suspend -- Re: suspend broken in next-20190704 on Thinkpad X60

[Rafael J. Wysocki]

On Mon, Jul 8, 2019 at 11:28 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 08-07-19, 10:28, Rafael J. Wysocki wrote:
> > Pavel has tested the latest version of the patch series AFAICS.
> >
> > The locking added by the commit in question to
> > refresh_frequency_limits() requires an update of
> > cpufreq_update_policy(), or it will deadlock in there because of the
> > lock acquired by cpufreq_cpu_get() if I haven't missed anything.
>
> Ah, looks quite straight forward.
>
> @Pavel: Can you please try this diff ?
>
> -------------------------8<-------------------------
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 9f68d0f306b8..4d6043ee7834 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1109,16 +1109,12 @@ void refresh_frequency_limits(struct cpufreq_policy *policy)
>  {
>         struct cpufreq_policy new_policy;
>
> -       down_write(&policy->rwsem);
> -
>         if (!policy_is_inactive(policy)) {
>                 new_policy = *policy;
>                 pr_debug("updating policy for CPU %u\n", policy->cpu);
>
>                 cpufreq_set_policy(policy, &new_policy);
>         }
> -
> -       up_write(&policy->rwsem);
>  }
>  EXPORT_SYMBOL(refresh_frequency_limits);
>
> @@ -1128,7 +1124,9 @@ static void handle_update(struct work_struct *work)
>                 container_of(work, struct cpufreq_policy, update);
>
>         pr_debug("handle_update for cpu %u called\n", policy->cpu);
> +       down_write(&policy->rwsem);
>         refresh_frequency_limits(policy);
> +       up_write(&policy->rwsem);
>  }
>
> -------------------------8<-------------------------
>
> Though it makes me wonder why I didn't hit this thing. I was using the
> cpu_cooling device the other day, which calls cpufreq_update_policy()
> very frequently on heat-up. And I had a hair dryer blowing over my
> board to heat it up. Lemme check that again :)
>
> @Rafael: You want me to send a new diff patch with Fixes tag this time
> if this works out fine ?

I would prefer the original patch to be updated to avoid possible
bisection woes in the future.

Re: cpufreq notifiers break suspend -- Re: suspend broken in next-20190704 on Thinkpad X60

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 2652 bytes --]

On Mon 2019-07-08 14:58:40, Viresh Kumar wrote:
> On 08-07-19, 10:28, Rafael J. Wysocki wrote:
> > Pavel has tested the latest version of the patch series AFAICS.
> > 
> > The locking added by the commit in question to
> > refresh_frequency_limits() requires an update of
> > cpufreq_update_policy(), or it will deadlock in there because of the
> > lock acquired by cpufreq_cpu_get() if I haven't missed anything.
> 
> Ah, looks quite straight forward.
> 
> @Pavel: Can you please try this diff ?

I tried to apply it on top of current next
(d58b5ab90ee7528126fd5833df7fc5bda8331ce8, 20190708) and linux-pm-next
(1e2a4c9019eb53f62790fadf86c14a54f4cf4888), but failed due to
whitespace (?!).

Yes, symptoms would be consistent with deadlock on resume.

And yes, the patch seems to fix problem for me.

Tested-by: Pavel Machek <pavel@ucw.cz>
									Pavel

> -------------------------8<-------------------------
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 9f68d0f306b8..4d6043ee7834 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1109,16 +1109,12 @@ void refresh_frequency_limits(struct cpufreq_policy *policy)
>  {
>         struct cpufreq_policy new_policy;
>  
> -       down_write(&policy->rwsem);
> -
>         if (!policy_is_inactive(policy)) {
>                 new_policy = *policy;
>                 pr_debug("updating policy for CPU %u\n", policy->cpu);
>  
>                 cpufreq_set_policy(policy, &new_policy);
>         }
> -
> -       up_write(&policy->rwsem);
>  }
>  EXPORT_SYMBOL(refresh_frequency_limits);
>  
> @@ -1128,7 +1124,9 @@ static void handle_update(struct work_struct *work)
>                 container_of(work, struct cpufreq_policy, update);
>  
>         pr_debug("handle_update for cpu %u called\n", policy->cpu);
> +       down_write(&policy->rwsem);
>         refresh_frequency_limits(policy);
> +       up_write(&policy->rwsem);
>  }
>  
> -------------------------8<-------------------------
> 
> Though it makes me wonder why I didn't hit this thing. I was using the
> cpu_cooling device the other day, which calls cpufreq_update_policy()
> very frequently on heat-up. And I had a hair dryer blowing over my
> board to heat it up. Lemme check that again :)

Can you test on some x86 ACPI? No dryers needed :-).

> @Rafael: You want me to send a new diff patch with Fixes tag this time
> if this works out fine ?



									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: cpufreq notifiers break suspend -- Re: suspend broken in next-20190704 on Thinkpad X60

[Viresh Kumar]

On 08-07-19, 16:13, Pavel Machek wrote:
> On Mon 2019-07-08 14:58:40, Viresh Kumar wrote:
> > Though it makes me wonder why I didn't hit this thing. I was using the
> > cpu_cooling device the other day, which calls cpufreq_update_policy()
> > very frequently on heat-up. And I had a hair dryer blowing over my
> > board to heat it up. Lemme check that again :)
>
> Can you test on some x86 ACPI? No dryers needed :-).
> 

Found out why I didn't hit it then. I tested it after converting
cpu_cooling driver to use QoS APIs and there is no double locking with
that.

-- 
viresh