Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753913AbbGWT3u (ORCPT ); Thu, 23 Jul 2015 15:29:50 -0400 Received: from mail-wi0-f178.google.com ([209.85.212.178]:38593 "EHLO mail-wi0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752896AbbGWT3s (ORCPT ); Thu, 23 Jul 2015 15:29:48 -0400 MIME-Version: 1.0 In-Reply-To: References: <20150718163149.GP7557@n2100.arm.linux.org.uk> <35d0bb6829c6c8a5fec7c55e45d9293946c0221b.1437566548.git.viresh.kumar@linaro.org> <20150722131539.GU7557@n2100.arm.linux.org.uk> <20150723060938.GD5322@linux> Date: Thu, 23 Jul 2015 21:29:46 +0200 X-Google-Sender-Auth: fPLls5p_jpYk6kEiN5tL59qVw4o Message-ID: Subject: Re: [PATCH V2] cpufreq: Fix double addition of sysfs links From: "Rafael J. Wysocki" To: "Rafael J. Wysocki" Cc: Viresh Kumar , Russell King - ARM Linux , Rafael Wysocki , Lists linaro-kernel , "linux-pm@vger.kernel.org" , open list Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2855 Lines: 62 On Thu, Jul 23, 2015 at 7:22 PM, Rafael J. Wysocki wrote: > Hi Viresh, > > On Thu, Jul 23, 2015 at 8:09 AM, Viresh Kumar wrote: >> On 22-07-15, 18:42, Rafael J. Wysocki wrote: >>> > 3. what happens when 'policy' is NULL at the point when the first (few) CPUs >>> > are added - how do the symlinks get created later if/when policy becomes >>> > non-NULL (can it?) >>> >>> Yes, it can, and we have a design issue here that bothers me a bit. >> >> I replied to Russell with a NO here as the first CPU should have >> created the policy. BUT... >> >>> Namley, we need a driver's ->init callback to populate policy->cpus >>> for us, but this is not the only thing it is doing, so the concern is >>> that it may not be able to deal with CPUs that aren't online. >> >> ... the first few CPUs could have been offline and so we might not >> have tried to add the policy at all.. Need to fix that for sure. > > Wait here. > > The current Linus' tree doesn't have that problem as far as I can say. > > Say cpufreq_interface->add_dev() is called for an offline CPU (say > CPU2). It points to cpufreq_add_dev(), so we see that the CPU is > offline and call add_cpu_dev_symlink() for it. But the first argument > we pass to that is per_cpu(cpufreq_cpu_data, cpu) and that is NULL, > because the policy is not there yet. So we just return 0 (and the CPU > has no policy and no link). > > Now say cpufreq_interface->add_dev() is called for an online CPU (say > CPU3). It goes and creates the policy for it and the driver's > ->init() tells us that CPU2 is related to it. So > cpufreq_add_dev_interface() creates the link for CPU2 and we're fine. > > Now say CPU3 was offline too when cpufreq_interface->add_dev() was > called for it. We don't create a policy or a link for it. Now say > CPU2 becomes online. cpufreq_cpu_callback() calls cpufreq_add_dev() > for it and we land in the previous case. > > The *broken* case is when CPU2 is online to start with and it had > created the link for CPU3, so when an offline CPU3 is now being added, > we try to create the link for it again. That is the case we need to > address in -rc without introducing new problems. The $subject patch > adresses that issue, but it introduces the above problem. On the > other hand, my patch at https://patchwork.kernel.org/patch/6839151/ > should take care of this too (unless it is broken in a way I'm not > seeing now). It doesn't address the case when the CPU being removed is the policy owner. Let me prepare a new version of it and we'll start over from there. 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/