From patchwork Tue May 14 13:54:41 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Srivatsa S. Bhat" X-Patchwork-Id: 2566021 Return-Path: X-Original-To: patchwork-linux-pm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork1.kernel.org (Postfix) with ESMTP id BAC003FD85 for ; Tue, 14 May 2013 13:57:48 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756553Ab3ENN5s (ORCPT ); Tue, 14 May 2013 09:57:48 -0400 Received: from e23smtp07.au.ibm.com ([202.81.31.140]:52483 "EHLO e23smtp07.au.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753439Ab3ENN5r (ORCPT ); Tue, 14 May 2013 09:57:47 -0400 Received: from /spool/local by e23smtp07.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 14 May 2013 23:47:38 +1000 Received: from d23dlp03.au.ibm.com (202.81.31.214) by e23smtp07.au.ibm.com (202.81.31.204) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Tue, 14 May 2013 23:47:35 +1000 Received: from d23relay04.au.ibm.com (d23relay04.au.ibm.com [9.190.234.120]) by d23dlp03.au.ibm.com (Postfix) with ESMTP id 580973578018 for ; Tue, 14 May 2013 23:57:38 +1000 (EST) Received: from d23av04.au.ibm.com (d23av04.au.ibm.com [9.190.235.139]) by d23relay04.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r4EDhc5S21823584 for ; Tue, 14 May 2013 23:43:39 +1000 Received: from d23av04.au.ibm.com (loopback [127.0.0.1]) by d23av04.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r4EDvaqp001898 for ; Tue, 14 May 2013 23:57:37 +1000 Received: from srivatsabhat.in.ibm.com ([9.79.177.149]) by d23av04.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVin) with ESMTP id r4EDvYYd001884; Tue, 14 May 2013 23:57:35 +1000 Message-ID: <51924221.2080707@linux.vnet.ibm.com> Date: Tue, 14 May 2013 19:24:41 +0530 From: "Srivatsa S. Bhat" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:15.0) Gecko/20120828 Thunderbird/15.0 MIME-Version: 1.0 To: "Rafael J. Wysocki" CC: "Jarzmik, Robert" , "R, Durgadoss" , Viresh Kumar , "linux-pm@vger.kernel.org" Subject: Re: S3, SMP non boot cpus and /sys/devices/system/cpu[1-9]/cpufreq/scaling_max_freq References: <65F5F98566038744B1B43C8FD3B7549F191101C4@IRSMSX104.ger.corp.intel.com> <65F5F98566038744B1B43C8FD3B7549F1911A240@IRSMSX104.ger.corp.intel.com> <51922F68.2010404@linux.vnet.ibm.com> <2396135.fo7RrTTQCl@vostro.rjw.lan> In-Reply-To: <2396135.fo7RrTTQCl@vostro.rjw.lan> X-Content-Scanned: Fidelis XPS MAILER x-cbid: 13051413-0260-0000-0000-000002FF58EB Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org On 05/14/2013 06:30 PM, Rafael J. Wysocki wrote: > On Tuesday, May 14, 2013 06:04:48 PM Srivatsa S. Bhat wrote: >> On 05/14/2013 05:24 PM, Jarzmik, Robert wrote: >>>> Okay, agreed after looking at this discussion ;) I am fine with the approach of kernel preserving permissions too. >>> >>> Ok, that's great, yet I don't see a clean solution here. This is >>> the path I see and that bothers me in the S3 suspend path: >>> cpufreq_sysfs_release >>> kobject_cleanup >>> kobject_release >>> kobject_put >>> __cpufreq_remove_dev >>> cpufreq_cpu_callback >>> notifier_call_chain >>> __raw_notifier_call_chain >>> __cpu_notify >>> _cpu_down >>> >>> Here the sysfs attributes are destroyed. Cpu onlining will >>> recreate them with root permissions. I don't see a good clean way >>> to preserve permissions in sysfs across suspend/resume for >>> deleted nodes. Do you ? >>> >>> And here we come to my actual worry : what is the technical clean >>> way to solve this problem ? >>> >>> So far I have no idea (the cpufreq example is only a subset of >>> the cases where it could happen). So if someone comes up with a >>> good idea I'll volunteer to implement it :) >>> >> >> Does the below (untested) patch help? I haven't spent time investigating >> whether not doing the add_dev/remove_dev stuff during CPU hotplug in S3 path >> will break something else. But it would be great if this works, since >> its the simplest solution that I can think of. > > Well, what if the CPU doesn't come up during resume? > Hmm, that's a good point. We will need to remove the sysfs files in that case. The updated patch below uses CPU_UP_CANCELED_FROZEN notification to do that. Regards, Srivatsa S. Bhat -----------------------------------------------------------------------> From: Srivatsa S. Bhat Subject: [PATCH v2] cpufreq: Preserve sysfs file permissions across suspend/resume The file permissions of cpufreq per-cpu sysfs files are not preserved across suspend/resume because we internally go through the CPU Hotplug path which reinitializes the file permissions on CPU online. But the user is not supposed to know that we are using CPU hotplug internally within suspend/resume (IOW, the kernel should not silently wreck the user-set file permissions across a suspend cycle). Therefore, we need to preserve the file permissions as it is, across suspend/resume. The simplest way to achieve that is to just not touch the sysfs files at all - ie., just ignore the CPU hotplug events in the suspend/resume path (_FROZEN) in the cpufreq hotplug callback. Reported-by: Robert Jarzmik Reported-by: Durgadoss R Signed-off-by: Srivatsa S. Bhat Acked-by: Viresh Kumar --- v2: Use UP_CANCELED_FROZEN notification to delete the sysfs files if the CPUs don't come up during resume. drivers/cpufreq/cpufreq.c | 4 +--- drivers/cpufreq/cpufreq_stats.c | 7 ++++--- 2 files changed, 5 insertions(+), 6 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 1b8a48e..284ba63 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1832,15 +1832,13 @@ static int __cpuinit cpufreq_cpu_callback(struct notifier_block *nfb, if (dev) { switch (action) { case CPU_ONLINE: - case CPU_ONLINE_FROZEN: cpufreq_add_dev(dev, NULL); break; case CPU_DOWN_PREPARE: - case CPU_DOWN_PREPARE_FROZEN: + case CPU_UP_CANCELED_FROZEN: __cpufreq_remove_dev(dev, NULL); break; case CPU_DOWN_FAILED: - case CPU_DOWN_FAILED_FROZEN: cpufreq_add_dev(dev, NULL); break; } diff --git a/drivers/cpufreq/cpufreq_stats.c b/drivers/cpufreq/cpufreq_stats.c index bfd6273..fb65dec 100644 --- a/drivers/cpufreq/cpufreq_stats.c +++ b/drivers/cpufreq/cpufreq_stats.c @@ -349,15 +349,16 @@ static int __cpuinit cpufreq_stat_cpu_callback(struct notifier_block *nfb, switch (action) { case CPU_ONLINE: - case CPU_ONLINE_FROZEN: cpufreq_update_policy(cpu); break; case CPU_DOWN_PREPARE: - case CPU_DOWN_PREPARE_FROZEN: cpufreq_stats_free_sysfs(cpu); break; case CPU_DEAD: - case CPU_DEAD_FROZEN: + cpufreq_stats_free_table(cpu); + break; + case CPU_UP_CANCELED_FROZEN: + cpufreq_stats_free_sysfs(cpu); cpufreq_stats_free_table(cpu); break; }