Re: [PATCH V7 2/6] cpufreq: Stop migrating sysfs files on hotplug

[Viresh Kumar <viresh.kumar@linaro.org>]

On 09-06-15, 17:20, Saravana Kannan wrote:
> >[ Something similar attempted by Saravana earlier ]
> Full name and email would be nice.

Sure. @Rafael can you please hand edit this in case I am not required
to resend the patch ?

[ Something similar attempted by Saravana Kannan <skannan@codeaurora.org> earlier ]

> >+static void update_policy_cpu(struct cpufreq_policy *policy, unsigned int cpu)
> >  {
> >-	int ret;
> >-
> >  	if (WARN_ON(cpu == policy->cpu))
> Can you remind me again why this is a warning? Would we still need
> this check?

Its removed in a later commit. Its a warning because we are updating
policy->cpu here and that must have been done only if policy->cpu !=
cpu.

> >+	bool recover_policy = !sif;
> The policy is always going to be there, so calling it
> "recover_policy" is kinda confusing. But I can't suggest a better
> name now.

Not always. Its created as well sometimes :)

> >
> >  	pr_debug("adding CPU %u\n", cpu);
> >
> >+	/*
> >+	 * Only possible if 'cpu' wasn't physically present earlier and we are
> >+	 * here from subsys_interface add callback. A hotplug notifier will
> >+	 * follow and we will handle it like logical CPU hotplug then. For now,
> >+	 * just create the sysfs link.
> >+	 */
> >+	if (cpu_is_offline(cpu)) {
> My changes were on an older code base, so things might have changed
> by now. But at this location, there was definitely a case where I
> had to check for "sif" before creating symlinks. I need to think a

cpu will be offline here only for sif==true.

> bit more to remember what that reason was and see if you have to do
> it too.
> 
> I'll try to respond more later. But I just wanted to send out what I
> could when I have little time to review this.

Okay. Good to see you again.

-- 
viresh