Re: [PATCH 2/2] cpufreq: Separate CPU device removal from CPU online

[Rafael J. Wysocki]

On Friday, July 24, 2015 07:49:16 AM Viresh Kumar wrote:
> On 23-07-15, 22:56, Rafael J. Wysocki wrote:
> > > > +	if (policy && policy->kobj_cpu != cpu) {
> > > 
> > > Why are you comparing cpu against kobj_cpu ? I don't think it can ever
> > > be false.
> 
> So what I meant was that the expression 'policy->kobj_cpu != cpu' will
> never return 'false'. Because policy->kobj_cpu is going to get set to
> the cpu for which we allocated the policy. And so it wouldn't match
> for any other CPU.

Unless kobj_cpu is removed before another one is registered (not a very
realistic scenario, but there's no guarantee that it will never happen).

But in that case we won't do the right thing anyway ...

> > It can.  When we're adding a CPU that has a policy already, because it is
> > "related" to a previously registered CPU.
> 
> In this case also the expression will return true.
> 
> > > > +		ret = sysfs_create_link(&dev->kobj, &policy->kobj, "cpufreq");
> > > > +		if (ret) {
> > > > +			dev_dbg(dev, "%s: Failed to create link (%d)\n",
> > > 
> > > dev_err
> > 
> > Well, I'm wondering about this.  Why does this have to be dev_err()?
> 
> Isn't this an error? We need to create a symlink, we failed and
> atleast the user should know about it. Why hide such failures ?

dev_err() doesn't mean "device error message".  It means "high priority device
message".

The user will know that the link is not there anyway and the question here is
whether or not the associated kernel message has to be high-priority.


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/