From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Prakash, Prashanth" Subject: Re: [PATCH] cpufreq: create sysfs symlink for cpus onlined after boot Date: Mon, 27 Mar 2017 10:55:39 -0600 Message-ID: <276b4adc-3525-d736-795e-09ccb2be1c7c@codeaurora.org> References: <1490304286-18311-1-git-send-email-pprakash@codeaurora.org> <372627201.N4ERmeoXuC@aspire.rjw.lan> <2716739.7ZKP1HnjcS@aspire.rjw.lan> <2246294.s9H9y0mptv@aspire.rjw.lan> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Return-path: Received: from smtp.codeaurora.org ([198.145.29.96]:55526 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751303AbdC0Q4i (ORCPT ); Mon, 27 Mar 2017 12:56:38 -0400 In-Reply-To: <2246294.s9H9y0mptv@aspire.rjw.lan> Sender: linux-pm-owner@vger.kernel.org List-Id: linux-pm@vger.kernel.org To: "Rafael J. Wysocki" Cc: "Rafael J. Wysocki" , Linux PM , Viresh Kumar Hi Rafael, On 3/25/2017 7:19 PM, Rafael J. Wysocki wrote: > On Sunday, March 26, 2017 03:05:56 AM Rafael J. Wysocki wrote: >> On Saturday, March 25, 2017 06:15:07 PM Rafael J. Wysocki wrote: >>> On Saturday, March 25, 2017 02:30:52 PM Rafael J. Wysocki wrote: >>>> On Friday, March 24, 2017 10:31:51 AM Prakash, Prashanth wrote: >>>>> On 3/23/2017 5:30 PM, Rafael J. Wysocki wrote: >>>>>> On Thu, Mar 23, 2017 at 10:24 PM, Prashanth Prakash >>>>>> wrote: >>>>>>> Adds an additional code path within cpufreq_online to create sysfs >>>>>>> symlinks for cpus that are bought onilne for the first time after >>>>>>> completion of boot. >>>>>>> >>>>>>> With maxcpus=N kernel parameter, it is possible to bring additional >>>>>>> cpus online after boot. In these cases per-cpu "cpufreq" symlinks >>>>>>> are not being created during registration as policy may not exist >>>>>>> for the cpus that were offline during boot. >>>>>>> >>>>>>> Signed-off-by: Prashanth Prakash >>>>>> This looks way overly complicated to me. Let me look at it in the >>>>>> next couple of days. >>>>> Sure. Thanks! >>>> So the patch below works for me. Please test. >>>> >>>> The problem is that we only try to create links once, when CPUs are registered >>>> or when the cpufreq driver is registered (depending on which happens first), >>>> but if all CPUs that would share a policy are offline at that time, the policy is not >>>> there and we don't create the links at all. >>>> >>>> This means we also need to try to create the links when creating a new policy >>>> (because the CPUs may have been added without the links at this point already). >>>> >>>> A slight side effect of this is that failures to create links are not regarded >>>> as hard errors now and only cause messages to be printed, but I don't see >>>> how that could be a big issue. >>> Actually, the rwlock around the for_each_cpu() loop in cpufreq_online() doesn't >>> matter AFAICS, so the code can be slightly simplified by dropping it. >> And I forgot about the cleanup on errors in cpufreq_online(), so one more >> update follows (with a changelog and all). > One more update, sorry. > > remove_cpu_dev_symlink() needs a device pointer, not a CPU number. Also the > links cleanup need not be done under policy->rwsem. > > --- > From: Rafael J. Wysocki > Subject: [PATCH] cpufreq: Fix creation of symbolic links to policy directories > > The cpufreq core only tries to create symbolic links from CPU > directories in sysfs to policy directories in cpufreq_add_dev(), > either when a given CPU is registered or when the cpufreq driver > is registered, whichever happens first. That is not sufficient, > however, because cpufreq_add_dev() may be called for an offline CPU > whose policy object has not been created yet and, quite obviously, > the symbolic cannot be added in that case. > > Fix that by making cpufreq_online() attempt to add symbolic links to > policy objects for the CPUs in the related_cpus mask of every new > policy object created by it. > > The cpufreq_driver_lock locking around the for_each_cpu() loop > in cpufreq_online() is dropped, because it is not necessary and the > code is somewhat simpler without it. Moreover, failures to create > a symbolic link will not be regarded as hard errors any more and > the CPUs without those links will not be taken offline automatically, > but that should not be problematic in practice. > > Reported-by: Prashanth Prakash > Signed-off-by: Rafael J. Wysocki Tested on a Qualcomm Centriq board and it works as expected. Thanks! -Prashanth