Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1193080 > unrolled thread

Re: [PATCH v2] cpufreq: Avoid attempts to create duplicate symbolic links

Started by"Rafael J. Wysocki" <rjw@rjwysocki.net>
First post2015-07-27 15:20 +0200
Last post2015-07-30 11:10 +0200
Articles 9 — 4 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH v2] cpufreq: Avoid attempts to create duplicate symbolic links "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-07-27 15:20 +0200
    Re: [PATCH v2] cpufreq: Avoid attempts to create duplicate symbolic  links Viresh Kumar <viresh.kumar@linaro.org> - 2015-07-27 16:40 +0200
      Re: [PATCH v2] cpufreq: Avoid attempts to create duplicate symbolic links "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-07-29 03:20 +0200
        Re: [PATCH v2] cpufreq: Avoid attempts to create duplicate symbolic  links Viresh Kumar <viresh.kumar@linaro.org> - 2015-07-29 07:50 +0200
        Re: [PATCH v2] cpufreq: Avoid attempts to create duplicate symbolic  links Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-07-29 11:20 +0200
          Re: [PATCH v2] cpufreq: Avoid attempts to create duplicate symbolic links "Rafael J. Wysocki" <rafael@kernel.org> - 2015-07-29 16:00 +0200
            Re: [PATCH v2] cpufreq: Avoid attempts to create duplicate symbolic  links Viresh Kumar <viresh.kumar@linaro.org> - 2015-07-29 16:30 +0200
              Re: [PATCH v2] cpufreq: Avoid attempts to create duplicate symbolic links "Rafael J. Wysocki" <rafael@kernel.org> - 2015-07-29 22:40 +0200
                Re: [PATCH v2] cpufreq: Avoid attempts to create duplicate symbolic  links Viresh Kumar <viresh.kumar@linaro.org> - 2015-07-30 11:10 +0200

#1193080 — Re: [PATCH v2] cpufreq: Avoid attempts to create duplicate symbolic links

From"Rafael J. Wysocki" <rjw@rjwysocki.net>
Date2015-07-27 15:20 +0200
SubjectRe: [PATCH v2] cpufreq: Avoid attempts to create duplicate symbolic links
Message-ID<pQQtY-2W3-21@gated-at.bofh.it>
On Monday, July 27, 2015 07:57:18 AM Viresh Kumar wrote:
> On 26-07-15, 00:46, Rafael J. Wysocki wrote:
> > OK, I'll prepare a new version of that patch then, but as I said this
> > choice means that we'll be creating the links to the policy at the
> > policy creation time going forward.
> 
> Atleast for the rc fix, we should do exactly this. Right.
> 
> But we can rethink about getting both my earlier patches merged for
> 4.3, which did this:
> 
> - Keep adding CPUs to a global mask, which didn't had a existing
>   policy and were offline when subsys-callback for called for them.
> - And then create the links only when the subsys callback is called
>   for CPUs, for which policy already exist, as Russell suggested.

Say the subsys add callback runs for a CPU and it doesn't have a policy.
If it is offline, we ignore it and the add callback won't be executed
for it again.

In turn, if it is online, we create a policy for it and we should (right
away) link the policy to all of the CPUs that were offline when the subsys add
callback was called for them.  That's what we do today.

Is there anything missing in that?

Thanks,
Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1193195 — Re: [PATCH v2] cpufreq: Avoid attempts to create duplicate symbolic links

FromViresh Kumar <viresh.kumar@linaro.org>
Date2015-07-27 16:40 +0200
SubjectRe: [PATCH v2] cpufreq: Avoid attempts to create duplicate symbolic links
Message-ID<pQRJo-4Ft-19@gated-at.bofh.it>
In reply to#1193080
On 27-07-15, 15:45, Rafael J. Wysocki wrote:
> Say the subsys add callback runs for a CPU and it doesn't have a policy.
> If it is offline, we ignore it and the add callback won't be executed
> for it again.
> 
> In turn, if it is online, we create a policy for it and we should (right
> away) link the policy to all of the CPUs that were offline when the subsys add
> callback was called for them.  That's what we do today.
> 
> Is there anything missing in that?

So the code is working properly after your patch, but I was talking
on the lines of what Russell suggested.

We should play with the links only when we receive add-dev/remove-dev
from subsys callbacks. The exception to that will be the offline CPUs
for which add-dev is called before their policy existed.

-- 
viresh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1194755

From"Rafael J. Wysocki" <rjw@rjwysocki.net>
Date2015-07-29 03:20 +0200
Message-ID<pRoch-1pi-3@gated-at.bofh.it>
In reply to#1193195
On Monday, July 27, 2015 08:09:35 PM Viresh Kumar wrote:
> On 27-07-15, 15:45, Rafael J. Wysocki wrote:
> > Say the subsys add callback runs for a CPU and it doesn't have a policy.
> > If it is offline, we ignore it and the add callback won't be executed
> > for it again.
> > 
> > In turn, if it is online, we create a policy for it and we should (right
> > away) link the policy to all of the CPUs that were offline when the subsys add
> > callback was called for them.  That's what we do today.
> > 
> > Is there anything missing in that?
> 
> So the code is working properly after your patch, but I was talking
> on the lines of what Russell suggested.
> 
> We should play with the links only when we receive add-dev/remove-dev
> from subsys callbacks. The exception to that will be the offline CPUs
> for which add-dev is called before their policy existed.

The rule is supposed to be "all of the present CPUs which do not own
a policy should point to one, unless it doesn't exist".  The right
approach is then to create links from them to a policy object as soon
as we create one for them.  Waiting for something else to happen is just
pointless and this approach covers both the offline and online CPUs, so
I don't think that changing it would improve things really.

Thanks,
Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1194822 — Re: [PATCH v2] cpufreq: Avoid attempts to create duplicate symbolic links

FromViresh Kumar <viresh.kumar@linaro.org>
Date2015-07-29 07:50 +0200
SubjectRe: [PATCH v2] cpufreq: Avoid attempts to create duplicate symbolic links
Message-ID<pRspC-7rV-25@gated-at.bofh.it>
In reply to#1194755
On 29-07-15, 03:38, Rafael J. Wysocki wrote:
> The rule is supposed to be "all of the present CPUs which do not own
> a policy should point to one, unless it doesn't exist".  The right
> approach is then to create links from them to a policy object as soon
> as we create one for them.  Waiting for something else to happen is just
> pointless and this approach covers both the offline and online CPUs, so
> I don't think that changing it would improve things really.

Ack.

-- 
viresh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1194986 — Re: [PATCH v2] cpufreq: Avoid attempts to create duplicate symbolic links

FromRussell King - ARM Linux <linux@arm.linux.org.uk>
Date2015-07-29 11:20 +0200
SubjectRe: [PATCH v2] cpufreq: Avoid attempts to create duplicate symbolic links
Message-ID<pRvGP-3OC-23@gated-at.bofh.it>
In reply to#1194755
On Wed, Jul 29, 2015 at 03:38:03AM +0200, Rafael J. Wysocki wrote:
> On Monday, July 27, 2015 08:09:35 PM Viresh Kumar wrote:
> > On 27-07-15, 15:45, Rafael J. Wysocki wrote:
> > > Say the subsys add callback runs for a CPU and it doesn't have a policy.
> > > If it is offline, we ignore it and the add callback won't be executed
> > > for it again.
> > > 
> > > In turn, if it is online, we create a policy for it and we should (right
> > > away) link the policy to all of the CPUs that were offline when the subsys add
> > > callback was called for them.  That's what we do today.
> > > 
> > > Is there anything missing in that?
> > 
> > So the code is working properly after your patch, but I was talking
> > on the lines of what Russell suggested.
> > 
> > We should play with the links only when we receive add-dev/remove-dev
> > from subsys callbacks. The exception to that will be the offline CPUs
> > for which add-dev is called before their policy existed.
> 
> The rule is supposed to be "all of the present CPUs which do not own
> a policy should point to one, unless it doesn't exist".  The right
> approach is then to create links from them to a policy object as soon
> as we create one for them.  Waiting for something else to happen is just
> pointless and this approach covers both the offline and online CPUs, so
> I don't think that changing it would improve things really.

I'm not sure we disagree with that.  It's more about when the symlinks
are created, and when you define that a CPU exists.

If you're attaching to subsystem in sysfs, then the point that the
subsystem interface gets to know about a sysfs node existing is when
it's add_dev method is called - it should not assume that a node exists
prior to that point, otherwise things are racy.

Consider a policy initialisation in parallel with an update of the
CPU present map and adding a CPU to sysfs.  The CPU present map will be
updated first, and then it will be added to sysfs.  If you're initialising
a cpufreq policy in the middle of that and creating symlinks for all
present CPUs, there's a window where the CPU present map indicates that
a CPU is present, but there is no sysfs directory for you to create a
symlink in.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1195211

From"Rafael J. Wysocki" <rafael@kernel.org>
Date2015-07-29 16:00 +0200
Message-ID<pRA3M-1sA-21@gated-at.bofh.it>
In reply to#1194986
Hi Russell,

On Wed, Jul 29, 2015 at 11:11 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Wed, Jul 29, 2015 at 03:38:03AM +0200, Rafael J. Wysocki wrote:
>> On Monday, July 27, 2015 08:09:35 PM Viresh Kumar wrote:
>> > On 27-07-15, 15:45, Rafael J. Wysocki wrote:
>> > > Say the subsys add callback runs for a CPU and it doesn't have a policy.
>> > > If it is offline, we ignore it and the add callback won't be executed
>> > > for it again.
>> > >
>> > > In turn, if it is online, we create a policy for it and we should (right
>> > > away) link the policy to all of the CPUs that were offline when the subsys add
>> > > callback was called for them.  That's what we do today.
>> > >
>> > > Is there anything missing in that?
>> >
>> > So the code is working properly after your patch, but I was talking
>> > on the lines of what Russell suggested.
>> >
>> > We should play with the links only when we receive add-dev/remove-dev
>> > from subsys callbacks. The exception to that will be the offline CPUs
>> > for which add-dev is called before their policy existed.
>>
>> The rule is supposed to be "all of the present CPUs which do not own
>> a policy should point to one, unless it doesn't exist".  The right
>> approach is then to create links from them to a policy object as soon
>> as we create one for them.  Waiting for something else to happen is just
>> pointless and this approach covers both the offline and online CPUs, so
>> I don't think that changing it would improve things really.
>
> I'm not sure we disagree with that.  It's more about when the symlinks
> are created, and when you define that a CPU exists.
>
> If you're attaching to subsystem in sysfs, then the point that the
> subsystem interface gets to know about a sysfs node existing is when
> it's add_dev method is called - it should not assume that a node exists
> prior to that point, otherwise things are racy.
>
> Consider a policy initialisation in parallel with an update of the
> CPU present map and adding a CPU to sysfs.  The CPU present map will be
> updated first, and then it will be added to sysfs.  If you're initialising
> a cpufreq policy in the middle of that and creating symlinks for all
> present CPUs, there's a window where the CPU present map indicates that
> a CPU is present, but there is no sysfs directory for you to create a
> symlink in.

In practice, this means a cpufreq driver registration done in parallel
with CPU hotplug.

Indeed, there is a small race window in that case, but it can be
closed easily by making cpufreq driver registration and CPU hotplug
mutually exclusive (we do that already for cpufreq driver
unregistration in linux-next anyway).

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1195240 — Re: [PATCH v2] cpufreq: Avoid attempts to create duplicate symbolic links

FromViresh Kumar <viresh.kumar@linaro.org>
Date2015-07-29 16:30 +0200
SubjectRe: [PATCH v2] cpufreq: Avoid attempts to create duplicate symbolic links
Message-ID<pRAwO-2g5-1@gated-at.bofh.it>
In reply to#1195211
On 29-07-15, 15:57, Rafael J. Wysocki wrote:
> In practice, this means a cpufreq driver registration done in parallel
> with CPU hotplug.

Not necessarily. Also consider the case where cpufreq driver is already working
for a set of CPUs. And a new set of CPUs (that will share the policy) are
getting physically added to the system.

Anyway, even if there is no problem at all, I do agree with Russell that it will
be better to do operations on behalf of the devices only when we are requested
for those devices.

-- 
viresh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1195490

From"Rafael J. Wysocki" <rafael@kernel.org>
Date2015-07-29 22:40 +0200
Message-ID<pRGiU-27b-39@gated-at.bofh.it>
In reply to#1195240
Hi Viresh,

On Wed, Jul 29, 2015 at 4:21 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 29-07-15, 15:57, Rafael J. Wysocki wrote:
>> In practice, this means a cpufreq driver registration done in parallel
>> with CPU hotplug.
>
> Not necessarily. Also consider the case where cpufreq driver is already working
> for a set of CPUs. And a new set of CPUs (that will share the policy) are
> getting physically added to the system.

That's what I mean by "hotplug" (as opposed to online/offline).

Adding a CPU involves (a) updating the present map and (b) registering
the CPU device and both are carried out together with CPU
online/offline locked.  At least that's the expectation and I'm not
aware of any architecture doing that differently.

So locking CPU online/offline around the cpufreq driver registration
would close that race and there's one more reason for doing that
(other than we do it for the cpufreq driver unregistration as I said
before): we need the CPU *online* map to be stable too while we're
registering the cpufreq driver.

> Anyway, even if there is no problem at all, I do agree with Russell that it will
> be better to do operations on behalf of the devices only when we are requested
> for those devices.

Problem is, we can't do that for all of them.  If the CPU is ofline
while being registered (the common case for hot-add) and it doesn't
have a policy already, we can't link it to an existing policy anyway,
so that operation has to be carried out later.  That is, we need to
create links for those CPUs after the policy has been created in any
case.

Moreover, the only case when we need to create links for online CPUs
is the registration of a cpufreq driver, because only then we can see
online CPUs that should be linked to a policy, but aren't.  It takes
less code to treat them in the same way as the offline CPUs at that
point (and create the links for them right after the policy has been
created) than to defer it to until sif->add() is called for each of
them, because we know that sif->add() is practically guaraneed to
succeeed for them (this is the code path in which we call
cpufreq_add_policy_cpu() and the governor "stop" only fails if it
hasn't been started for that policy).

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1195829 — Re: [PATCH v2] cpufreq: Avoid attempts to create duplicate symbolic links

FromViresh Kumar <viresh.kumar@linaro.org>
Date2015-07-30 11:10 +0200
SubjectRe: [PATCH v2] cpufreq: Avoid attempts to create duplicate symbolic links
Message-ID<pRS0G-2ew-11@gated-at.bofh.it>
In reply to#1195490
I am not going to fight hard to prove a point as the code is in good
working conditions, but wanted to finish the discussion ..

On 29-07-15, 22:32, Rafael J. Wysocki wrote:
> On Wed, Jul 29, 2015 at 4:21 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > On 29-07-15, 15:57, Rafael J. Wysocki wrote:
> >> In practice, this means a cpufreq driver registration done in parallel
> >> with CPU hotplug.
> >
> > Not necessarily. Also consider the case where cpufreq driver is already working
> > for a set of CPUs. And a new set of CPUs (that will share the policy) are
> > getting physically added to the system.
> 
> That's what I mean by "hotplug" (as opposed to online/offline).

You were talking about driver registration done in parallel with
hotplug. But I was pointing at something else.

Suppose a system that has:
- 8 CPUs, 0-3 and 4-7 share policy
- 4-7 physically hotplugged out
- cpufreq driver registered and so policy0 active for cpu 0-3.
- We hotplug 4-7.

So, this is a bit different compared to the case where Russell mentioned
the Racy thing. Not sure if this case also has similary racy situations
though.

> Problem is, we can't do that for all of them.

Right.

> If the CPU is ofline
> while being registered (the common case for hot-add) and it doesn't
> have a policy already, we can't link it to an existing policy anyway,
> so that operation has to be carried out later.  That is, we need to
> create links for those CPUs after the policy has been created in any
> case.

Right, but at least the cpufreq-core is already requested on behalf of
such CPUs. We aren't creating links based on the assumption that a
add_dev() will be called for these devices.

> Moreover, the only case when we need to create links for online CPUs

offline CPUs as well..

> is the registration of a cpufreq driver, because only then we can see
> online CPUs that should be linked to a policy, but aren't.  It takes
> less code to treat them in the same way as the offline CPUs at that
> point (and create the links for them right after the policy has been
> created) than to defer it to until sif->add() is called for each of
> them, because we know that sif->add() is practically guaraneed to
> succeeed for them (this is the code path in which we call
> cpufreq_add_policy_cpu() and the governor "stop" only fails if it
> hasn't been started for that policy).

Choose whatever you feel is right. I have already written below code, so
just pasting it here. Its not to say, that this code looks better :P

---
 drivers/cpufreq/cpufreq.c | 108 +++++++++++++++++++++-------------------------
 1 file changed, 48 insertions(+), 60 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 24e4ba568e77..87faabce777d 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -31,6 +31,12 @@
 #include <linux/tick.h>
 #include <trace/events/power.h>
 
+/*
+ * CPUs that were offline when a request to allocate policy was issued, symlinks
+ * for them should be created once the policy is available for them.
+ */
+cpumask_t real_cpus_pending;
+
 static LIST_HEAD(cpufreq_policy_list);
 
 static inline bool policy_is_inactive(struct cpufreq_policy *policy)
@@ -941,62 +947,46 @@ EXPORT_SYMBOL(cpufreq_sysfs_remove_file);
 static int add_cpu_dev_symlink(struct cpufreq_policy *policy, int cpu)
 {
 	struct device *cpu_dev;
-
-	pr_debug("%s: Adding symlink for CPU: %u\n", __func__, cpu);
-
-	if (!policy)
-		return 0;
+	int ret;
 
 	cpu_dev = get_cpu_device(cpu);
 	if (WARN_ON(!cpu_dev))
 		return 0;
 
-	return sysfs_create_link(&cpu_dev->kobj, &policy->kobj, "cpufreq");
-}
-
-static void remove_cpu_dev_symlink(struct cpufreq_policy *policy, int cpu)
-{
-	struct device *cpu_dev;
-
-	pr_debug("%s: Removing symlink for CPU: %u\n", __func__, cpu);
+	dev_dbg(cpu_dev, "%s: Adding symlink for CPU: %u\n", __func__, cpu);
 
-	cpu_dev = get_cpu_device(cpu);
-	if (WARN_ON(!cpu_dev))
-		return;
+	ret = sysfs_create_link(&cpu_dev->kobj, &policy->kobj, "cpufreq");
+	if (ret)
+		dev_err(cpu_dev, "%s: Failed to create link (%d)\n", __func__,
+			ret);
+	else
+		cpumask_set_cpu(cpu, policy->real_cpus);
 
-	sysfs_remove_link(&cpu_dev->kobj, "cpufreq");
+	return ret;
 }
 
-/* Add/remove symlinks for all related CPUs */
-static int cpufreq_add_dev_symlink(struct cpufreq_policy *policy)
+/*
+ * Create symlinks for CPUs which are already added via subsys callbacks (and
+ * were offline then), before the policy was created.
+ */
+static int cpufreq_add_pending_symlinks(struct cpufreq_policy *policy)
 {
-	unsigned int j;
-	int ret = 0;
+	struct cpumask mask;
+	int cpu, ret;
+
+	cpumask_and(&mask, policy->related_cpus, &real_cpus_pending);
 
-	/* Some related CPUs might not be present (physically hotplugged) */
-	for_each_cpu(j, policy->real_cpus) {
-		if (j == policy->kobj_cpu)
-			continue;
+	if (cpumask_empty(&mask))
+		return 0;
 
-		ret = add_cpu_dev_symlink(policy, j);
+	for_each_cpu(cpu, &mask) {
+		ret = add_cpu_dev_symlink(policy, cpu);
 		if (ret)
-			break;
+			return ret;
+		cpumask_clear_cpu(cpu, &real_cpus_pending);
 	}
 
-	return ret;
-}
-
-static void cpufreq_remove_dev_symlink(struct cpufreq_policy *policy)
-{
-	unsigned int j;
-
-	/* Some related CPUs might not be present (physically hotplugged) */
-	for_each_cpu(j, policy->real_cpus) {
-		if (j == policy->kobj_cpu)
-			continue;
-
-		remove_cpu_dev_symlink(policy, j);
-	}
+	return 0;
 }
 
 static int cpufreq_add_dev_interface(struct cpufreq_policy *policy)
@@ -1028,7 +1018,7 @@ static int cpufreq_add_dev_interface(struct cpufreq_policy *policy)
 			return ret;
 	}
 
-	return cpufreq_add_dev_symlink(policy);
+	return cpufreq_add_pending_symlinks(policy);
 }
 
 static int cpufreq_init_policy(struct cpufreq_policy *policy)
@@ -1155,7 +1145,6 @@ static void cpufreq_policy_put_kobj(struct cpufreq_policy *policy, bool notify)
 					     CPUFREQ_REMOVE_POLICY, policy);
 
 	down_write(&policy->rwsem);
-	cpufreq_remove_dev_symlink(policy);
 	kobj = &policy->kobj;
 	cmp = &policy->kobj_unregister;
 	up_write(&policy->rwsem);
@@ -1235,10 +1224,9 @@ static int cpufreq_online(unsigned int cpu)
 	down_write(&policy->rwsem);
 
 	if (new_policy) {
+		cpumask_copy(policy->real_cpus, cpumask_of(cpu));
 		/* related_cpus should at least include policy->cpus. */
 		cpumask_or(policy->related_cpus, policy->related_cpus, policy->cpus);
-		/* Remember CPUs present at the policy creation time. */
-		cpumask_and(policy->real_cpus, policy->cpus, cpu_present_mask);
 	}
 
 	/*
@@ -1363,23 +1351,17 @@ static int cpufreq_online(unsigned int cpu)
 static int cpufreq_add_dev(struct device *dev, struct subsys_interface *sif)
 {
 	unsigned cpu = dev->id;
-	int ret;
+	struct cpufreq_policy *policy = per_cpu(cpufreq_cpu_data, cpu);
+	int ret = 0;
 
 	dev_dbg(dev, "%s: adding CPU%u\n", __func__, cpu);
 
-	if (cpu_online(cpu)) {
+	if (policy)
+		ret = add_cpu_dev_symlink(policy, cpu);
+	else if (cpu_online(cpu))
 		ret = cpufreq_online(cpu);
-	} else {
-		/*
-		 * A hotplug notifier will follow and we will handle it as CPU
-		 * online then.  For now, just create the sysfs link, unless
-		 * there is no policy or the link is already present.
-		 */
-		struct cpufreq_policy *policy = per_cpu(cpufreq_cpu_data, cpu);
-
-		ret = policy && !cpumask_test_and_set_cpu(cpu, policy->real_cpus)
-			? add_cpu_dev_symlink(policy, cpu) : 0;
-	}
+	else
+		cpumask_set_cpu(cpu, &real_cpus_pending);
 
 	return ret;
 }
@@ -1469,8 +1451,10 @@ static int cpufreq_remove_dev(struct device *dev, struct subsys_interface *sif)
 	unsigned int cpu = dev->id;
 	struct cpufreq_policy *policy = per_cpu(cpufreq_cpu_data, cpu);
 
-	if (!policy)
+	if (!policy) {
+		cpumask_clear_cpu(cpu, &real_cpus_pending);
 		return 0;
+	}
 
 	if (cpu_online(cpu)) {
 		cpufreq_offline_prepare(cpu);
@@ -1485,7 +1469,8 @@ static int cpufreq_remove_dev(struct device *dev, struct subsys_interface *sif)
 	}
 
 	if (cpu != policy->kobj_cpu) {
-		remove_cpu_dev_symlink(policy, cpu);
+		dev_dbg(dev, "%s: Removing symlink\n", __func__);
+		sysfs_remove_link(&dev->kobj, "cpufreq");
 	} else {
 		/*
 		 * The CPU owning the policy object is going away.  Move it to
@@ -2550,6 +2535,9 @@ int cpufreq_unregister_driver(struct cpufreq_driver *driver)
 	if (cpufreq_boost_supported())
 		cpufreq_sysfs_remove_file(&boost.attr);
 
+	if (WARN_ON(!cpumask_empty(&real_cpus_pending)))
+		cpumask_clear(&real_cpus_pending);
+
 	unregister_hotcpu_notifier(&cpufreq_cpu_notifier);
 
 	write_lock_irqsave(&cpufreq_driver_lock, flags);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web