All of lore.kernel.org
 help / color / mirror / Atom feed
* BUG in bleeding edge c560f3d
@ 2013-02-04 22:19 Dirk Brandewie
  2013-02-05  0:09 ` Rafael J. Wysocki
  0 siblings, 1 reply; 11+ messages in thread
From: Dirk Brandewie @ 2013-02-04 22:19 UTC (permalink / raw)
  To: cpufreq; +Cc: dirk.brandewie

Hi All,

There is a bug in the bleeding-edge branch. Using the ondemand governor
and acpi_cpufreq scaling driver the system hangs while trying to reboot or
trying to offline a cpu manually.  The last call where I have tracing turned
on is __cpufreq_governor(data, CPUFREQ_GOV_STOP);

[   38.138236] __cpufreq_remove_dev: cpufreq: __cpufreq_remove_dev: 
unregistering CPU 1
[   38.146663] __cpufreq_governor: cpufreq: __cpufreq_governor for CPU 0, event 2
[   71.562262] ata3.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen


It looks like the breakage comes from the removal of the sysfs files in the
incorrect order.  While rebasing my patches onto bleeding edge I found this
problem as well.  I have a couple of hack patches that workaround the issue
when using my driver if anyone would like to see them.

--Dirk

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: BUG in bleeding edge c560f3d
  2013-02-05  0:09 ` Rafael J. Wysocki
@ 2013-02-05  0:05   ` Dirk Brandewie
  2013-02-05  2:43     ` Rafael J. Wysocki
  2013-02-05  2:47     ` Rafael J. Wysocki
  0 siblings, 2 replies; 11+ messages in thread
From: Dirk Brandewie @ 2013-02-05  0:05 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: Dirk Brandewie, cpufreq

On 02/04/2013 04:09 PM, Rafael J. Wysocki wrote:
> On Monday, February 04, 2013 02:19:22 PM Dirk Brandewie wrote:
>> Hi All,
>>
>> There is a bug in the bleeding-edge branch. Using the ondemand governor
>> and acpi_cpufreq scaling driver the system hangs while trying to reboot or
>> trying to offline a cpu manually.  The last call where I have tracing turned
>> on is __cpufreq_governor(data, CPUFREQ_GOV_STOP);
>>
>> [   38.138236] __cpufreq_remove_dev: cpufreq: __cpufreq_remove_dev:
>> unregistering CPU 1
>> [   38.146663] __cpufreq_governor: cpufreq: __cpufreq_governor for CPU 0, event 2
>> [   71.562262] ata3.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
>>
>>
>> It looks like the breakage comes from the removal of the sysfs files in the
>> incorrect order.  While rebasing my patches onto bleeding edge I found this
>> problem as well.  I have a couple of hack patches that workaround the issue
>> when using my driver if anyone would like to see them.
>
> There have been a few fixes since c560f3d, they are in linux-pm.git/linux-next
> now.  Can you please test that tree and see if the problem is still there?
>
Rebased a couple of hours ago testing now.  ATM it looks like it is related to
cpufreq_stats handling of the sysfs files


> Rafael
>
>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: BUG in bleeding edge c560f3d
  2013-02-04 22:19 BUG in bleeding edge c560f3d Dirk Brandewie
@ 2013-02-05  0:09 ` Rafael J. Wysocki
  2013-02-05  0:05   ` Dirk Brandewie
  0 siblings, 1 reply; 11+ messages in thread
From: Rafael J. Wysocki @ 2013-02-05  0:09 UTC (permalink / raw)
  To: Dirk Brandewie; +Cc: cpufreq

On Monday, February 04, 2013 02:19:22 PM Dirk Brandewie wrote:
> Hi All,
> 
> There is a bug in the bleeding-edge branch. Using the ondemand governor
> and acpi_cpufreq scaling driver the system hangs while trying to reboot or
> trying to offline a cpu manually.  The last call where I have tracing turned
> on is __cpufreq_governor(data, CPUFREQ_GOV_STOP);
> 
> [   38.138236] __cpufreq_remove_dev: cpufreq: __cpufreq_remove_dev: 
> unregistering CPU 1
> [   38.146663] __cpufreq_governor: cpufreq: __cpufreq_governor for CPU 0, event 2
> [   71.562262] ata3.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
> 
> 
> It looks like the breakage comes from the removal of the sysfs files in the
> incorrect order.  While rebasing my patches onto bleeding edge I found this
> problem as well.  I have a couple of hack patches that workaround the issue
> when using my driver if anyone would like to see them.

There have been a few fixes since c560f3d, they are in linux-pm.git/linux-next
now.  Can you please test that tree and see if the problem is still there?

Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: BUG in bleeding edge c560f3d
  2013-02-05  0:05   ` Dirk Brandewie
@ 2013-02-05  2:43     ` Rafael J. Wysocki
  2013-02-05  5:45       ` Viresh Kumar
  2013-02-05  2:47     ` Rafael J. Wysocki
  1 sibling, 1 reply; 11+ messages in thread
From: Rafael J. Wysocki @ 2013-02-05  2:43 UTC (permalink / raw)
  To: Dirk Brandewie, Viresh Kumar; +Cc: cpufreq, Linux PM list

On Monday, February 04, 2013 04:05:27 PM Dirk Brandewie wrote:
> On 02/04/2013 04:09 PM, Rafael J. Wysocki wrote:
> > On Monday, February 04, 2013 02:19:22 PM Dirk Brandewie wrote:
> >> Hi All,
> >>
> >> There is a bug in the bleeding-edge branch. Using the ondemand governor
> >> and acpi_cpufreq scaling driver the system hangs while trying to reboot or
> >> trying to offline a cpu manually.  The last call where I have tracing turned
> >> on is __cpufreq_governor(data, CPUFREQ_GOV_STOP);
> >>
> >> [   38.138236] __cpufreq_remove_dev: cpufreq: __cpufreq_remove_dev:
> >> unregistering CPU 1
> >> [   38.146663] __cpufreq_governor: cpufreq: __cpufreq_governor for CPU 0, event 2
> >> [   71.562262] ata3.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
> >>
> >>
> >> It looks like the breakage comes from the removal of the sysfs files in the
> >> incorrect order.  While rebasing my patches onto bleeding edge I found this
> >> problem as well.  I have a couple of hack patches that workaround the issue
> >> when using my driver if anyone would like to see them.
> >
> > There have been a few fixes since c560f3d, they are in linux-pm.git/linux-next
> > now.  Can you please test that tree and see if the problem is still there?
> >
> Rebased a couple of hours ago testing now.  ATM it looks like it is related to
> cpufreq_stats handling of the sysfs files

This looks like a problem with commit b8eed8a.  Viresh?

Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: BUG in bleeding edge c560f3d
  2013-02-05  0:05   ` Dirk Brandewie
  2013-02-05  2:43     ` Rafael J. Wysocki
@ 2013-02-05  2:47     ` Rafael J. Wysocki
  1 sibling, 0 replies; 11+ messages in thread
From: Rafael J. Wysocki @ 2013-02-05  2:47 UTC (permalink / raw)
  To: Dirk Brandewie; +Cc: cpufreq, Linux PM list

On Monday, February 04, 2013 04:05:27 PM Dirk Brandewie wrote:
> On 02/04/2013 04:09 PM, Rafael J. Wysocki wrote:
> > On Monday, February 04, 2013 02:19:22 PM Dirk Brandewie wrote:
> >> Hi All,
> >>
> >> There is a bug in the bleeding-edge branch. Using the ondemand governor
> >> and acpi_cpufreq scaling driver the system hangs while trying to reboot or
> >> trying to offline a cpu manually.  The last call where I have tracing turned
> >> on is __cpufreq_governor(data, CPUFREQ_GOV_STOP);
> >>
> >> [   38.138236] __cpufreq_remove_dev: cpufreq: __cpufreq_remove_dev:
> >> unregistering CPU 1
> >> [   38.146663] __cpufreq_governor: cpufreq: __cpufreq_governor for CPU 0, event 2
> >> [   71.562262] ata3.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
> >>
> >>
> >> It looks like the breakage comes from the removal of the sysfs files in the
> >> incorrect order.  While rebasing my patches onto bleeding edge I found this
> >> problem as well.  I have a couple of hack patches that workaround the issue
> >> when using my driver if anyone would like to see them.
> >
> > There have been a few fixes since c560f3d, they are in linux-pm.git/linux-next
> > now.  Can you please test that tree and see if the problem is still there?
> >
> Rebased a couple of hours ago testing now.  ATM it looks like it is related to
> cpufreq_stats handling of the sysfs files

Can you reproduce it without your patches?

Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: BUG in bleeding edge c560f3d
  2013-02-05  2:43     ` Rafael J. Wysocki
@ 2013-02-05  5:45       ` Viresh Kumar
  2013-02-05  7:08         ` Viresh Kumar
  0 siblings, 1 reply; 11+ messages in thread
From: Viresh Kumar @ 2013-02-05  5:45 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: Dirk Brandewie, cpufreq, Linux PM list

On 5 February 2013 08:13, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Monday, February 04, 2013 04:05:27 PM Dirk Brandewie wrote:

>> Rebased a couple of hours ago testing now.  ATM it looks like it is related to
>> cpufreq_stats handling of the sysfs files
>
> This looks like a problem with commit b8eed8a.  Viresh?

I had some direct chat with Dirk to understand his problem. He has got lots
of minor changes, which by themselves looks incorrect, for ex:
- He is required to cpufreq_frequency_table_put_attr() from his drivers
  init() code to make it NULL... But it should already have been NULL as per
  my understanding. I really want him to test his driver without any additional
  target/set_policy() patches he has.. to see if linux-next works for
him or not.

He will do this testing tomorrow and will let us know of any issues he faces.

We have already tested linux-next and these patches on ARM TC2 and STE
u8500 (by Fabio), with reboots and lots of hotplugging.

Though i am still going to review my own patches once more to see if there is
scope for improvement.

--
viresh

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: BUG in bleeding edge c560f3d
  2013-02-05  5:45       ` Viresh Kumar
@ 2013-02-05  7:08         ` Viresh Kumar
  2013-02-05 10:08           ` Rafael J. Wysocki
  2013-02-05 18:45           ` Dirk Brandewie
  0 siblings, 2 replies; 11+ messages in thread
From: Viresh Kumar @ 2013-02-05  7:08 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: Dirk Brandewie, cpufreq, Linux PM list

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

On 5 February 2013 11:15, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 5 February 2013 08:13, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> On Monday, February 04, 2013 04:05:27 PM Dirk Brandewie wrote:
>
>>> Rebased a couple of hours ago testing now.  ATM it looks like it is related to
>>> cpufreq_stats handling of the sysfs files
>>
>> This looks like a problem with commit b8eed8a.  Viresh?
>
> I had some direct chat with Dirk to understand his problem. He has got lots
> of minor changes, which by themselves looks incorrect, for ex:
> - He is required to cpufreq_frequency_table_put_attr() from his drivers
>   init() code to make it NULL... But it should already have been NULL as per
>   my understanding. I really want him to test his driver without any additional
>   target/set_policy() patches he has.. to see if linux-next works for
> him or not.
>
> He will do this testing tomorrow and will let us know of any issues he faces.
>
> We have already tested linux-next and these patches on ARM TC2 and STE
> u8500 (by Fabio), with reboots and lots of hotplugging.
>
> Though i am still going to review my own patches once more to see if there is
> scope for improvement.

The system on which Dirk is testing his patches has following configuration:
1 Package, 4 cpus, 8 virtual cores...

Though they share their clock line in some way, it is handled by the
cpufreq driver
only and for the outside world (cpufreq core), it looks as there are 8
independent cpus,
and so 8 policy structures.

I tried to reproduce the issue at my end by removing extra cpus from
my clusters and
just keeping one per cluster alive from DT.

And i *wasn't* able to reproduce the problem. Though i was able to
find a small bug/issue
in the current code for which i have following patch (attached too):

@Dirk: Please give this one a try. Atleast on my system with various
configurations, i
couldn't see any different this patch has made, but is more logical to me.

commit 9bdd6d47403e696d05953870019e791806f8d6bf
Author: Viresh Kumar <viresh.kumar@linaro.org>
Date:   Tue Feb 5 12:28:18 2013 +0530

    cpufreq: Don't remove sysfs link for policy->cpu

    "cpufreq" directory in policy->cpu is never created using
sysfs_create_link(),
    but using kobject_init_and_add(). And so we shouldn't call
sysfs_remove_link()
    for policy->cpu(). sysfs stuff for policy->cpu is automatically
removed when we
    call kobject_put() for dying policy.

    Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cpufreq.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 7aacfbf..9567451 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1047,7 +1047,9 @@ static int __cpufreq_remove_dev(struct device
*dev, struct subsys_interface *sif
        cpus = cpumask_weight(data->cpus);
        cpumask_clear_cpu(cpu, data->cpus);

-       if (unlikely((cpu == data->cpu) && (cpus > 1))) {
+       if (cpu != data->cpu) {
+               sysfs_remove_link(&dev->kobj, "cpufreq");
+       } else if (cpus > 1) {
                /* first sibling now owns the new sysfs dir */
                cpu_dev = get_cpu_device(cpumask_first(data->cpus));
                sysfs_remove_link(&cpu_dev->kobj, "cpufreq");
@@ -1072,7 +1074,6 @@ static int __cpufreq_remove_dev(struct device
*dev, struct subsys_interface *sif
        pr_debug("%s: removing link, cpu: %d\n", __func__, cpu);
        cpufreq_cpu_put(data);
        unlock_policy_rwsem_write(cpu);
-       sysfs_remove_link(&dev->kobj, "cpufreq");

        /* If cpu is last user of policy, free policy */
        if (cpus == 1) {

[-- Attachment #2: 0001-cpufreq-Don-t-remove-sysfs-link-for-policy-cpu.patch --]
[-- Type: application/octet-stream, Size: 1744 bytes --]

From 9bdd6d47403e696d05953870019e791806f8d6bf Mon Sep 17 00:00:00 2001
Message-Id: <9bdd6d47403e696d05953870019e791806f8d6bf.1360047744.git.viresh.kumar@linaro.org>
From: Viresh Kumar <viresh.kumar@linaro.org>
Date: Tue, 5 Feb 2013 12:28:18 +0530
Subject: [PATCH] cpufreq: Don't remove sysfs link for policy->cpu

"cpufreq" directory in policy->cpu is never created using sysfs_create_link(),
but using kobject_init_and_add(). And so we shouldn't call sysfs_remove_link()
for policy->cpu(). sysfs stuff for policy->cpu is automatically removed when we
call kobject_put() for dying policy.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cpufreq.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 7aacfbf..9567451 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1047,7 +1047,9 @@ static int __cpufreq_remove_dev(struct device *dev, struct subsys_interface *sif
 	cpus = cpumask_weight(data->cpus);
 	cpumask_clear_cpu(cpu, data->cpus);
 
-	if (unlikely((cpu == data->cpu) && (cpus > 1))) {
+	if (cpu != data->cpu) {
+		sysfs_remove_link(&dev->kobj, "cpufreq");
+	} else if (cpus > 1) {
 		/* first sibling now owns the new sysfs dir */
 		cpu_dev = get_cpu_device(cpumask_first(data->cpus));
 		sysfs_remove_link(&cpu_dev->kobj, "cpufreq");
@@ -1072,7 +1074,6 @@ static int __cpufreq_remove_dev(struct device *dev, struct subsys_interface *sif
 	pr_debug("%s: removing link, cpu: %d\n", __func__, cpu);
 	cpufreq_cpu_put(data);
 	unlock_policy_rwsem_write(cpu);
-	sysfs_remove_link(&dev->kobj, "cpufreq");
 
 	/* If cpu is last user of policy, free policy */
 	if (cpus == 1) {
-- 
1.7.12.rc2.18.g61b472e


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: BUG in bleeding edge c560f3d
  2013-02-05  7:08         ` Viresh Kumar
@ 2013-02-05 10:08           ` Rafael J. Wysocki
  2013-02-05 18:45           ` Dirk Brandewie
  1 sibling, 0 replies; 11+ messages in thread
From: Rafael J. Wysocki @ 2013-02-05 10:08 UTC (permalink / raw)
  To: Viresh Kumar; +Cc: Dirk Brandewie, cpufreq, Linux PM list

On Tuesday, February 05, 2013 12:38:44 PM Viresh Kumar wrote:
> On 5 February 2013 11:15, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > On 5 February 2013 08:13, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> >> On Monday, February 04, 2013 04:05:27 PM Dirk Brandewie wrote:
> >
> >>> Rebased a couple of hours ago testing now.  ATM it looks like it is related to
> >>> cpufreq_stats handling of the sysfs files
> >>
> >> This looks like a problem with commit b8eed8a.  Viresh?
> >
> > I had some direct chat with Dirk to understand his problem. He has got lots
> > of minor changes, which by themselves looks incorrect, for ex:
> > - He is required to cpufreq_frequency_table_put_attr() from his drivers
> >   init() code to make it NULL... But it should already have been NULL as per
> >   my understanding. I really want him to test his driver without any additional
> >   target/set_policy() patches he has.. to see if linux-next works for
> > him or not.
> >
> > He will do this testing tomorrow and will let us know of any issues he faces.
> >
> > We have already tested linux-next and these patches on ARM TC2 and STE
> > u8500 (by Fabio), with reboots and lots of hotplugging.
> >
> > Though i am still going to review my own patches once more to see if there is
> > scope for improvement.
> 
> The system on which Dirk is testing his patches has following configuration:
> 1 Package, 4 cpus, 8 virtual cores...
> 
> Though they share their clock line in some way, it is handled by the
> cpufreq driver
> only and for the outside world (cpufreq core), it looks as there are 8
> independent cpus,
> and so 8 policy structures.
> 
> I tried to reproduce the issue at my end by removing extra cpus from
> my clusters and
> just keeping one per cluster alive from DT.
> 
> And i *wasn't* able to reproduce the problem. Though i was able to
> find a small bug/issue
> in the current code for which i have following patch (attached too):

Thanks for working on this!

> @Dirk: Please give this one a try. Atleast on my system with various
> configurations, i
> couldn't see any different this patch has made, but is more logical to me.

It looks good, I'm going to take it.

Thanks,
Rafael


> commit 9bdd6d47403e696d05953870019e791806f8d6bf
> Author: Viresh Kumar <viresh.kumar@linaro.org>
> Date:   Tue Feb 5 12:28:18 2013 +0530
> 
>     cpufreq: Don't remove sysfs link for policy->cpu
> 
>     "cpufreq" directory in policy->cpu is never created using
> sysfs_create_link(),
>     but using kobject_init_and_add(). And so we shouldn't call
> sysfs_remove_link()
>     for policy->cpu(). sysfs stuff for policy->cpu is automatically
> removed when we
>     call kobject_put() for dying policy.
> 
>     Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/cpufreq/cpufreq.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 7aacfbf..9567451 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1047,7 +1047,9 @@ static int __cpufreq_remove_dev(struct device
> *dev, struct subsys_interface *sif
>         cpus = cpumask_weight(data->cpus);
>         cpumask_clear_cpu(cpu, data->cpus);
> 
> -       if (unlikely((cpu == data->cpu) && (cpus > 1))) {
> +       if (cpu != data->cpu) {
> +               sysfs_remove_link(&dev->kobj, "cpufreq");
> +       } else if (cpus > 1) {
>                 /* first sibling now owns the new sysfs dir */
>                 cpu_dev = get_cpu_device(cpumask_first(data->cpus));
>                 sysfs_remove_link(&cpu_dev->kobj, "cpufreq");
> @@ -1072,7 +1074,6 @@ static int __cpufreq_remove_dev(struct device
> *dev, struct subsys_interface *sif
>         pr_debug("%s: removing link, cpu: %d\n", __func__, cpu);
>         cpufreq_cpu_put(data);
>         unlock_policy_rwsem_write(cpu);
> -       sysfs_remove_link(&dev->kobj, "cpufreq");
> 
>         /* If cpu is last user of policy, free policy */
>         if (cpus == 1) {
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: BUG in bleeding edge c560f3d
  2013-02-05  7:08         ` Viresh Kumar
  2013-02-05 10:08           ` Rafael J. Wysocki
@ 2013-02-05 18:45           ` Dirk Brandewie
  2013-02-05 21:30             ` Rafael J. Wysocki
  2013-02-06  1:34             ` Viresh Kumar
  1 sibling, 2 replies; 11+ messages in thread
From: Dirk Brandewie @ 2013-02-05 18:45 UTC (permalink / raw)
  To: Viresh Kumar; +Cc: Rafael J. Wysocki, Dirk Brandewie, cpufreq, Linux PM list

On 02/04/2013 11:08 PM, Viresh Kumar wrote:
> @Dirk: Please give this one a try. Atleast on my system with various
> configurations, i
> couldn't see any different this patch has made, but is more logical to me.
>

Tested works fine with and without my driver present.

I added it to my patch set with my Tested-by tag it seems gmail decided
to swallow it :-(

Please feel to add my Tested-by:

--Dirk
> commit 9bdd6d47403e696d05953870019e791806f8d6bf
> Author: Viresh Kumar <viresh.kumar@linaro.org>
> Date:   Tue Feb 5 12:28:18 2013 +0530
>
>      cpufreq: Don't remove sysfs link for policy->cpu
>
>      "cpufreq" directory in policy->cpu is never created using
> sysfs_create_link(),
>      but using kobject_init_and_add(). And so we shouldn't call
> sysfs_remove_link()
>      for policy->cpu(). sysfs stuff for policy->cpu is automatically
> removed when we
>      call kobject_put() for dying policy.
>
>      Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>   drivers/cpufreq/cpufreq.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 7aacfbf..9567451 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1047,7 +1047,9 @@ static int __cpufreq_remove_dev(struct device
> *dev, struct subsys_interface *sif
>          cpus = cpumask_weight(data->cpus);
>          cpumask_clear_cpu(cpu, data->cpus);
>
> -       if (unlikely((cpu == data->cpu) && (cpus > 1))) {
> +       if (cpu != data->cpu) {
> +               sysfs_remove_link(&dev->kobj, "cpufreq");
> +       } else if (cpus > 1) {
>                  /* first sibling now owns the new sysfs dir */
>                  cpu_dev = get_cpu_device(cpumask_first(data->cpus));
>                  sysfs_remove_link(&cpu_dev->kobj, "cpufreq");
> @@ -1072,7 +1074,6 @@ static int __cpufreq_remove_dev(struct device
> *dev, struct subsys_interface *sif
>          pr_debug("%s: removing link, cpu: %d\n", __func__, cpu);
>          cpufreq_cpu_put(data);
>          unlock_policy_rwsem_write(cpu);
> -       sysfs_remove_link(&dev->kobj, "cpufreq");
>
>          /* If cpu is last user of policy, free policy */
>          if (cpus == 1) {
>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: BUG in bleeding edge c560f3d
  2013-02-05 18:45           ` Dirk Brandewie
@ 2013-02-05 21:30             ` Rafael J. Wysocki
  2013-02-06  1:34             ` Viresh Kumar
  1 sibling, 0 replies; 11+ messages in thread
From: Rafael J. Wysocki @ 2013-02-05 21:30 UTC (permalink / raw)
  To: Dirk Brandewie; +Cc: Viresh Kumar, cpufreq, Linux PM list

On Tuesday, February 05, 2013 10:45:03 AM Dirk Brandewie wrote:
> On 02/04/2013 11:08 PM, Viresh Kumar wrote:
> > @Dirk: Please give this one a try. Atleast on my system with various
> > configurations, i
> > couldn't see any different this patch has made, but is more logical to me.
> >
> 
> Tested works fine with and without my driver present.
> 
> I added it to my patch set with my Tested-by tag it seems gmail decided
> to swallow it :-(
> 
> Please feel to add my Tested-by:

I will, thanks Dirk!

Rafael


> > commit 9bdd6d47403e696d05953870019e791806f8d6bf
> > Author: Viresh Kumar <viresh.kumar@linaro.org>
> > Date:   Tue Feb 5 12:28:18 2013 +0530
> >
> >      cpufreq: Don't remove sysfs link for policy->cpu
> >
> >      "cpufreq" directory in policy->cpu is never created using
> > sysfs_create_link(),
> >      but using kobject_init_and_add(). And so we shouldn't call
> > sysfs_remove_link()
> >      for policy->cpu(). sysfs stuff for policy->cpu is automatically
> > removed when we
> >      call kobject_put() for dying policy.
> >
> >      Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> > ---
> >   drivers/cpufreq/cpufreq.c | 5 +++--
> >   1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> > index 7aacfbf..9567451 100644
> > --- a/drivers/cpufreq/cpufreq.c
> > +++ b/drivers/cpufreq/cpufreq.c
> > @@ -1047,7 +1047,9 @@ static int __cpufreq_remove_dev(struct device
> > *dev, struct subsys_interface *sif
> >          cpus = cpumask_weight(data->cpus);
> >          cpumask_clear_cpu(cpu, data->cpus);
> >
> > -       if (unlikely((cpu == data->cpu) && (cpus > 1))) {
> > +       if (cpu != data->cpu) {
> > +               sysfs_remove_link(&dev->kobj, "cpufreq");
> > +       } else if (cpus > 1) {
> >                  /* first sibling now owns the new sysfs dir */
> >                  cpu_dev = get_cpu_device(cpumask_first(data->cpus));
> >                  sysfs_remove_link(&cpu_dev->kobj, "cpufreq");
> > @@ -1072,7 +1074,6 @@ static int __cpufreq_remove_dev(struct device
> > *dev, struct subsys_interface *sif
> >          pr_debug("%s: removing link, cpu: %d\n", __func__, cpu);
> >          cpufreq_cpu_put(data);
> >          unlock_policy_rwsem_write(cpu);
> > -       sysfs_remove_link(&dev->kobj, "cpufreq");
> >
> >          /* If cpu is last user of policy, free policy */
> >          if (cpus == 1) {
> >
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: BUG in bleeding edge c560f3d
  2013-02-05 18:45           ` Dirk Brandewie
  2013-02-05 21:30             ` Rafael J. Wysocki
@ 2013-02-06  1:34             ` Viresh Kumar
  1 sibling, 0 replies; 11+ messages in thread
From: Viresh Kumar @ 2013-02-06  1:34 UTC (permalink / raw)
  To: Dirk Brandewie; +Cc: Rafael J. Wysocki, cpufreq, Linux PM list

On 6 February 2013 00:15, Dirk Brandewie <dirk.brandewie@gmail.com> wrote:
> On 02/04/2013 11:08 PM, Viresh Kumar wrote:
>>
>> @Dirk: Please give this one a try. Atleast on my system with various
>> configurations, i
>> couldn't see any different this patch has made, but is more logical to me.
>>
>
> Tested works fine with and without my driver present.

Wow!! I was shooting in the dark without knowing where the right target is
and luckily i found the target :)

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2013-02-06  1:34 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-04 22:19 BUG in bleeding edge c560f3d Dirk Brandewie
2013-02-05  0:09 ` Rafael J. Wysocki
2013-02-05  0:05   ` Dirk Brandewie
2013-02-05  2:43     ` Rafael J. Wysocki
2013-02-05  5:45       ` Viresh Kumar
2013-02-05  7:08         ` Viresh Kumar
2013-02-05 10:08           ` Rafael J. Wysocki
2013-02-05 18:45           ` Dirk Brandewie
2013-02-05 21:30             ` Rafael J. Wysocki
2013-02-06  1:34             ` Viresh Kumar
2013-02-05  2:47     ` Rafael J. Wysocki

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.