diff mbox

[5/5] cpufreq: use correct values of cpus in __cpufreq_remove_dev_finish()

Message ID CAKohponzohnOL4Y81Q1W4Yg=paRO46c-gY=HMbkFUqgrjsT+xw@mail.gmail.com
State New
Headers show

Commit Message

Viresh Kumar Sept. 12, 2013, 9:21 a.m. UTC
On 12 September 2013 12:46, Srivatsa S. Bhat
<srivatsa.bhat@linux.vnet.ibm.com> wrote:
> That sounds good! Even the naming is much better, it conveys the intent
> clearly.

Folded below change in my patch (attached):

        if (frozen)

--
thanks

Comments

Viresh Kumar Sept. 12, 2013, 10:43 a.m. UTC | #1
On 12 September 2013 16:17, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> Please resend.  And I honestly don't think that [1-3/5] are fixes and
> [4/5] is not needed any more.

Okay, I will resend 5/5.. I didn't get you last two statements clearly :(
- don't think that [1-3/5] are fixes: You meant they are cleanups rather?
Yes, and that's why I asked you to take those on for 3.13 and not for 12..

- [4/5] is not needed any more: 4/5 is already applied by you and it came
from Rafael.. I reinvented the wheel, nothing more. And so it is not required
anymore :)
Rafael J. Wysocki Sept. 12, 2013, 10:47 a.m. UTC | #2
On Thursday, September 12, 2013 02:51:58 PM Viresh Kumar wrote:
> On 12 September 2013 12:46, Srivatsa S. Bhat
> <srivatsa.bhat@linux.vnet.ibm.com> wrote:
> > That sounds good! Even the naming is much better, it conveys the intent
> > clearly.
> 
> Folded below change in my patch (attached):

Please resend.  And I honestly don't think that [1-3/5] are fixes and
[4/5] is not needed any more.

> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index b556d46..23f5845 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1111,7 +1111,7 @@ static int
> cpufreq_nominate_new_policy_cpu(struct cpufreq_policy *policy,
>         int ret;
> 
>         /* first sibling now owns the new sysfs dir */
> -       cpu_dev = get_cpu_device(cpumask_first(policy->cpus));
> +       cpu_dev = get_cpu_device(cpumask_any_but(policy->cpus, old_cpu));
> 
>         /* Don't touch sysfs files during light-weight tear-down */
>         if (frozen)
> 
> --
Viresh Kumar Sept. 12, 2013, 10:49 a.m. UTC | #3
On 12 September 2013 16:26, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> Surely from Srivatsa?

Ofcourse.. I kept repeating this to myself... "Viresh, please read mails
you have written before you send.. So many times you are pointed
about these silly mistakes"... And I keep forgetting them, ah its a
fairly small mail, what can go wrong.. :)

--
viresh
Rafael J. Wysocki Sept. 12, 2013, 10:56 a.m. UTC | #4
On Thursday, September 12, 2013 04:13:03 PM Viresh Kumar wrote:
> On 12 September 2013 16:17, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > Please resend.  And I honestly don't think that [1-3/5] are fixes and
> > [4/5] is not needed any more.
> 
> Okay, I will resend 5/5.. I didn't get you last two statements clearly :(
> - don't think that [1-3/5] are fixes: You meant they are cleanups rather?

Yes.

> Yes, and that's why I asked you to take those on for 3.13 and not for 12..

Ah, sorry, I must have overlooked that.  I didn't say I wasn't going to take
them, though.

> - [4/5] is not needed any more: 4/5 is already applied by you and it came
> from Rafael.. I reinvented the wheel, nothing more. And so it is not required
> anymore :)

Surely from Srivatsa?

Thanks,
Rafael
Stephen Warren Sept. 12, 2013, 6:08 p.m. UTC | #5
On 09/12/2013 03:21 AM, Viresh Kumar wrote:
> On 12 September 2013 12:46, Srivatsa S. Bhat
> <srivatsa.bhat@linux.vnet.ibm.com> wrote:
>> That sounds good! Even the naming is much better, it conveys the intent
>> clearly.
> 
> Folded below change in my patch (attached):
...

For the record, the patch which was attached to Viresh's email solves
the hangs I was getting in next-20130912 during (the second?) CPU
hotplug and resume. So,

Tested-by: Stephen Warren <swarren@nvidia.com>

Without this, I saw:

> root@localhost:~# ./cpuonline.py 
> echo 0 > /sys/devices/system/cpu/cpu1/online
> [   24.049729] CPU1: shutdown
> echo 1 > /sys/devices/system/cpu/cpu1/online
> [   26.090439] CPU1: Booted secondary processor
> root@localhost:~# ./cpuonline.py 
> echo 0 > /sys/devices/system/cpu/cpu1/online
> [   29.003016] CPU1: shutdown
> echo 1 > /sys/devices/system/cpu/cpu1/online
> [   31.032562] CPU1: Booted secondary processor
[hung]
> [  240.300393] INFO: task ondemand:678 blocked for more than 120 seconds.
> [  240.312842] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> [  240.326658] ondemand        D c050f6cc     0   678      1 0x00000000
> [  240.339062] [<c050f6cc>] (__schedule+0x330/0x6d0) from [<c050fe50>] (schedule_preempt_disabled+0x24/0x34)
> [  240.354904] [<c050fe50>] (schedule_preempt_disabled+0x24/0x34) from [<c050e810>] (__mutex_lock_slowpath+0x178/0x370)
> [  240.371767] [<c050e810>] (__mutex_lock_slowpath+0x178/0x370) from [<c050ea14>] (mutex_lock+0xc/0x24)
> [  240.387363] [<c050ea14>] (mutex_lock+0xc/0x24) from [<c00245e8>] (get_online_cpus+0x2c/0x48)
> [  240.402273] [<c00245e8>] (get_online_cpus+0x2c/0x48) from [<c0342c0c>] (store+0x18/0xbc)
> [  240.417688] [<c0342c0c>] (store+0x18/0xbc) from [<c011d598>] (sysfs_write_file+0x168/0x198)
> [  240.432687] [<c011d598>] (sysfs_write_file+0x168/0x198) from [<c00cbf98>] (vfs_write+0xb0/0x188)
> [  240.448185] [<c00cbf98>] (vfs_write+0xb0/0x188) from [<c00cc348>] (SyS_write+0x3c/0x70)
> [  240.462964] [<c00cc348>] (SyS_write+0x3c/0x70) from [<c000e540>] (ret_fast_syscall+0x0/0x30)
> [  240.478239] INFO: task sh:827 blocked for more than 120 seconds.
> [  240.491027] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> [  240.505795] sh              D c050f6cc     0   827    825 0x00000001
> [  240.519169] [<c050f6cc>] (__schedule+0x330/0x6d0) from [<c0510660>] (__down_write_nested+0x78/0xcc)
> [  240.535666] [<c0510660>] (__down_write_nested+0x78/0xcc) from [<c0342ae0>] (lock_policy_rwsem_write+0x34/0x48)
> [  240.552950] [<c0342ae0>] (lock_policy_rwsem_write+0x34/0x48) from [<c0343a04>] (cpufreq_update_policy+0x20/0xec)
> [  240.570631] [<c0343a04>] (cpufreq_update_policy+0x20/0xec) from [<c0344410>] (cpufreq_cpu_callback+0x78/0x8c)
> [  240.588051] [<c0344410>] (cpufreq_cpu_callback+0x78/0x8c) from [<c0042f44>] (notifier_call_chain+0x44/0x84)
> [  240.605401] [<c0042f44>] (notifier_call_chain+0x44/0x84) from [<c0024594>] (__cpu_notify+0x28/0x44)
> [  240.622085] [<c0024594>] (__cpu_notify+0x28/0x44) from [<c00247fc>] (_cpu_up+0x100/0x154)
> [  240.638006] [<c00247fc>] (_cpu_up+0x100/0x154) from [<c00248c4>] (cpu_up+0x5c/0x84)
> [  240.653405] [<c00248c4>] (cpu_up+0x5c/0x84) from [<c028a2a8>] (device_online+0x64/0x88)
> [  240.669263] [<c028a2a8>] (device_online+0x64/0x88) from [<c028a328>] (store_online+0x5c/0x64)
> [  240.685683] [<c028a328>] (store_online+0x5c/0x64) from [<c028816c>] (dev_attr_store+0x18/0x24)
> [  240.702337] [<c028816c>] (dev_attr_store+0x18/0x24) from [<c011d598>] (sysfs_write_file+0x168/0x198)
> [  240.719520] [<c011d598>] (sysfs_write_file+0x168/0x198) from [<c00cbf98>] (vfs_write+0xb0/0x188)
> [  240.736614] [<c00cbf98>] (vfs_write+0xb0/0x188) from [<c00cc348>] (SyS_write+0x3c/0x70)
> [  240.752799] [<c00cc348>] (SyS_write+0x3c/0x70) from [<c000e540>] (ret_fast_syscall+0x0/0x30)
diff mbox

Patch

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index b556d46..23f5845 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1111,7 +1111,7 @@  static int
cpufreq_nominate_new_policy_cpu(struct cpufreq_policy *policy,
        int ret;

        /* first sibling now owns the new sysfs dir */
-       cpu_dev = get_cpu_device(cpumask_first(policy->cpus));
+       cpu_dev = get_cpu_device(cpumask_any_but(policy->cpus, old_cpu));

        /* Don't touch sysfs files during light-weight tear-down */