From patchwork Sat Aug 3 11:49:27 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Viresh Kumar X-Patchwork-Id: 18754 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-vb0-f71.google.com (mail-vb0-f71.google.com [209.85.212.71]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 8215B23981 for ; Sat, 3 Aug 2013 11:50:54 +0000 (UTC) Received: by mail-vb0-f71.google.com with SMTP id g17sf2079748vbg.2 for ; Sat, 03 Aug 2013 04:50:54 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=mime-version:x-beenthere:x-forwarded-to:x-forwarded-for :delivered-to:from:to:cc:subject:date:message-id:x-mailer :in-reply-to:references:in-reply-to:references:x-gm-message-state :x-removed-original-auth:x-original-sender :x-original-authentication-results:precedence:mailing-list:list-id :x-google-group-id:list-post:list-help:list-archive:list-unsubscribe; bh=gtaEPOnhNYnz7YJbqOuuF55IeZVmk/ohYUC6w5NdbBA=; b=aFGXxmH+F0PS3LCB/7Qi6VYFRTemBpTwDm1WeNh58acdfn4kwTr0HpEqLjBRzOBrSR mWveBnRWiK1X8OjSRyZ7hNQ26LxVkexPkjPLL8KDMaMr7cZm3ULD8OWo1FWhfIbnqtdR adKvhAb3RGvwcRvuyt4/yqai1WBjh4lUOrV5TLbdhFoeZn2011e7B1hSbKYniKs7Q7QR +BalOUM6KIRD1kj4gJ9Rd7yLS3IB6Av3atbCITmfm9r7fqVssvXZbzsdHfuCR7xzyGwE 8NqL0frddZlRsyANdp6RAba3fobY/NUcYgU7q0PrKCA5esUI6qosu6koH0E/WRuW/uT3 1D8A== X-Received: by 10.236.79.67 with SMTP id h43mr4660984yhe.46.1375530654297; Sat, 03 Aug 2013 04:50:54 -0700 (PDT) MIME-Version: 1.0 X-BeenThere: patchwork-forward@linaro.org Received: by 10.49.99.10 with SMTP id em10ls2033689qeb.82.gmail; Sat, 03 Aug 2013 04:50:54 -0700 (PDT) X-Received: by 10.58.197.5 with SMTP id iq5mr3490352vec.30.1375530654130; Sat, 03 Aug 2013 04:50:54 -0700 (PDT) Received: from mail-ve0-f179.google.com (mail-ve0-f179.google.com [209.85.128.179]) by mx.google.com with ESMTPS id r5si2968858vcr.139.2013.08.03.04.50.54 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Sat, 03 Aug 2013 04:50:54 -0700 (PDT) Received-SPF: neutral (google.com: 209.85.128.179 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) client-ip=209.85.128.179; Received: by mail-ve0-f179.google.com with SMTP id c13so1678482vea.24 for ; Sat, 03 Aug 2013 04:50:54 -0700 (PDT) X-Received: by 10.52.109.69 with SMTP id hq5mr2878356vdb.85.1375530653987; Sat, 03 Aug 2013 04:50:53 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patches@linaro.org Received: by 10.221.11.8 with SMTP id pc8csp479vcb; Sat, 3 Aug 2013 04:50:53 -0700 (PDT) X-Received: by 10.68.169.161 with SMTP id af1mr12651719pbc.22.1375530653014; Sat, 03 Aug 2013 04:50:53 -0700 (PDT) Received: from mail-pb0-f49.google.com (mail-pb0-f49.google.com [209.85.160.49]) by mx.google.com with ESMTPS id ua7si10570004pac.59.2013.08.03.04.50.52 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Sat, 03 Aug 2013 04:50:53 -0700 (PDT) Received-SPF: neutral (google.com: 209.85.160.49 is neither permitted nor denied by best guess record for domain of viresh.kumar@linaro.org) client-ip=209.85.160.49; Received: by mail-pb0-f49.google.com with SMTP id xb4so1625757pbc.22 for ; Sat, 03 Aug 2013 04:50:52 -0700 (PDT) X-Received: by 10.66.25.232 with SMTP id f8mr15552224pag.25.1375530652592; Sat, 03 Aug 2013 04:50:52 -0700 (PDT) Received: from localhost ([122.172.193.46]) by mx.google.com with ESMTPSA id w8sm16201654pab.12.2013.08.03.04.50.48 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Sat, 03 Aug 2013 04:50:52 -0700 (PDT) From: Viresh Kumar To: rjw@sisk.pl Cc: linaro-kernel@lists.linaro.org, patches@linaro.org, cpufreq@vger.kernel.org, linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, srivatsa.bhat@linux.vnet.ibm.com, l.majewski@samsung.com, Viresh Kumar Subject: [PATCH 09/10] cpufreq: Don't use cpufreq_driver->owner's refcount to protect critical sections Date: Sat, 3 Aug 2013 17:19:27 +0530 Message-Id: X-Mailer: git-send-email 1.7.12.rc2.18.g61b472e In-Reply-To: References: In-Reply-To: References: X-Gm-Message-State: ALoCoQn04Y2GCpZGAUdREi50NgThW2IT8QSMzHt2sZ77Kx2JbRxMB85jbXlyrwOdrMnk5JMxXwso X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: viresh.kumar@linaro.org X-Original-Authentication-Results: mx.google.com; spf=neutral (google.com: 209.85.128.179 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Precedence: list Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org List-ID: X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , We are protecting critical sections of cpufreq core with the help of owner's refcount, which isn't the correct approach. As rmmod returns error when some routine has updated the module's refcount. Lets use rwsem for this.. Only cpufreq_unregister_driver() will use write sem and everybody else will use read sem. Signed-off-by: Viresh Kumar --- drivers/cpufreq/cpufreq.c | 135 ++++++++++++++++++++-------------------------- 1 file changed, 57 insertions(+), 78 deletions(-) diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 74d4969..70399ea 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -90,6 +90,12 @@ static void unlock_policy_rwsem_##mode(int cpu) \ unlock_policy_rwsem(read, cpu); unlock_policy_rwsem(write, cpu); +/* + * rwsem to guarantee that cpufreq driver module doesn't unload during critical + * sections + */ +static DECLARE_RWSEM(cpufreq_rwsem); + /* internal prototypes */ static int __cpufreq_governor(struct cpufreq_policy *policy, unsigned int event); @@ -177,78 +183,46 @@ u64 get_cpu_idle_time(unsigned int cpu, u64 *wall, int io_busy) } EXPORT_SYMBOL_GPL(get_cpu_idle_time); -static struct cpufreq_policy *__cpufreq_cpu_get(unsigned int cpu, bool sysfs) +struct cpufreq_policy *cpufreq_cpu_get(unsigned int cpu) { - struct cpufreq_policy *policy; + struct cpufreq_policy *policy = NULL; unsigned long flags; - if (cpu >= nr_cpu_ids) - goto err_out; + if (cpufreq_disabled() || (cpu >= nr_cpu_ids)) + return NULL; + + if (!down_read_trylock(&cpufreq_rwsem)) + return NULL; /* get the cpufreq driver */ read_lock_irqsave(&cpufreq_driver_lock, flags); - if (!cpufreq_driver) - goto err_out_unlock; - - if (!try_module_get(cpufreq_driver->owner)) - goto err_out_unlock; + if (cpufreq_driver) { + /* get the CPU */ + policy = per_cpu(cpufreq_cpu_data, cpu); + if (policy) + kobject_get(&policy->kobj); + } - /* get the CPU */ - policy = per_cpu(cpufreq_cpu_data, cpu); + read_unlock_irqrestore(&cpufreq_driver_lock, flags); if (!policy) - goto err_out_put_module; - - if (!sysfs && !kobject_get(&policy->kobj)) - goto err_out_put_module; + up_read(&cpufreq_rwsem); - read_unlock_irqrestore(&cpufreq_driver_lock, flags); return policy; - -err_out_put_module: - module_put(cpufreq_driver->owner); -err_out_unlock: - read_unlock_irqrestore(&cpufreq_driver_lock, flags); -err_out: - return NULL; -} - -struct cpufreq_policy *cpufreq_cpu_get(unsigned int cpu) -{ - if (cpufreq_disabled()) - return NULL; - - return __cpufreq_cpu_get(cpu, false); } EXPORT_SYMBOL_GPL(cpufreq_cpu_get); -static struct cpufreq_policy *cpufreq_cpu_get_sysfs(unsigned int cpu) -{ - return __cpufreq_cpu_get(cpu, true); -} - -static void __cpufreq_cpu_put(struct cpufreq_policy *policy, bool sysfs) -{ - if (!sysfs) - kobject_put(&policy->kobj); - module_put(cpufreq_driver->owner); -} - void cpufreq_cpu_put(struct cpufreq_policy *policy) { if (cpufreq_disabled()) return; - __cpufreq_cpu_put(policy, false); + kobject_put(&policy->kobj); + up_read(&cpufreq_rwsem); } EXPORT_SYMBOL_GPL(cpufreq_cpu_put); -static void cpufreq_cpu_put_sysfs(struct cpufreq_policy *policy) -{ - __cpufreq_cpu_put(policy, true); -} - /********************************************************************* * EXTERNALLY AFFECTING FREQUENCY CHANGES * *********************************************************************/ @@ -693,12 +667,12 @@ static ssize_t show(struct kobject *kobj, struct attribute *attr, char *buf) struct cpufreq_policy *policy = to_policy(kobj); struct freq_attr *fattr = to_attr(attr); ssize_t ret = -EINVAL; - policy = cpufreq_cpu_get_sysfs(policy->cpu); - if (!policy) - goto no_policy; + + if (!down_read_trylock(&cpufreq_rwsem)) + goto exit; if (lock_policy_rwsem_read(policy->cpu) < 0) - goto fail; + goto up_read; if (fattr->show) ret = fattr->show(policy, buf); @@ -706,9 +680,10 @@ static ssize_t show(struct kobject *kobj, struct attribute *attr, char *buf) ret = -EIO; unlock_policy_rwsem_read(policy->cpu); -fail: - cpufreq_cpu_put_sysfs(policy); -no_policy: + +up_read: + up_read(&cpufreq_rwsem); +exit: return ret; } @@ -718,12 +693,12 @@ static ssize_t store(struct kobject *kobj, struct attribute *attr, struct cpufreq_policy *policy = to_policy(kobj); struct freq_attr *fattr = to_attr(attr); ssize_t ret = -EINVAL; - policy = cpufreq_cpu_get_sysfs(policy->cpu); - if (!policy) - goto no_policy; + + if (!down_read_trylock(&cpufreq_rwsem)) + goto exit; if (lock_policy_rwsem_write(policy->cpu) < 0) - goto fail; + goto up_read; if (fattr->store) ret = fattr->store(policy, buf, count); @@ -731,9 +706,10 @@ static ssize_t store(struct kobject *kobj, struct attribute *attr, ret = -EIO; unlock_policy_rwsem_write(policy->cpu); -fail: - cpufreq_cpu_put_sysfs(policy); -no_policy: + +up_read: + up_read(&cpufreq_rwsem); +exit: return ret; } @@ -1001,25 +977,24 @@ static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif, return 0; } + if (!down_read_trylock(&cpufreq_rwsem)) + return 0; + #ifdef CONFIG_HOTPLUG_CPU /* Check if this cpu was hot-unplugged earlier and has siblings */ read_lock_irqsave(&cpufreq_driver_lock, flags); list_for_each_entry(tpolicy, &cpufreq_policy_list, policy_list) { if (cpumask_test_cpu(cpu, tpolicy->related_cpus)) { read_unlock_irqrestore(&cpufreq_driver_lock, flags); - return cpufreq_add_policy_cpu(tpolicy, cpu, dev, - frozen); + ret = cpufreq_add_policy_cpu(tpolicy, cpu, dev, frozen); + up_read(&cpufreq_rwsem); + return ret; } } read_unlock_irqrestore(&cpufreq_driver_lock, flags); #endif #endif - if (!try_module_get(cpufreq_driver->owner)) { - ret = -EINVAL; - goto module_out; - } - if (frozen) /* Restore the saved policy when doing light-weight init */ policy = cpufreq_policy_restore(cpu); @@ -1092,7 +1067,8 @@ static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif, cpufreq_init_policy(policy); kobject_uevent(&policy->kobj, KOBJ_ADD); - module_put(cpufreq_driver->owner); + up_read(&cpufreq_rwsem); + pr_debug("initialization complete\n"); return 0; @@ -1110,8 +1086,8 @@ err_set_policy_cpu: per_cpu(cpufreq_policy_cpu, cpu) = -1; cpufreq_policy_free(policy); nomem_out: - module_put(cpufreq_driver->owner); -module_out: + up_read(&cpufreq_rwsem); + return ret; } @@ -1423,10 +1399,9 @@ static unsigned int __cpufreq_get(unsigned int cpu) unsigned int cpufreq_get(unsigned int cpu) { unsigned int ret_freq = 0; - struct cpufreq_policy *policy = cpufreq_cpu_get(cpu); - if (!policy) - goto out; + if (!down_read_trylock(&cpufreq_rwsem)) + return 0; if (unlikely(lock_policy_rwsem_read(cpu))) goto out_policy; @@ -1436,8 +1411,8 @@ unsigned int cpufreq_get(unsigned int cpu) unlock_policy_rwsem_read(cpu); out_policy: - cpufreq_cpu_put(policy); -out: + up_read(&cpufreq_rwsem); + return ret_freq; } EXPORT_SYMBOL(cpufreq_get); @@ -2130,9 +2105,13 @@ int cpufreq_unregister_driver(struct cpufreq_driver *driver) subsys_interface_unregister(&cpufreq_interface); unregister_hotcpu_notifier(&cpufreq_cpu_notifier); + down_write(&cpufreq_rwsem); write_lock_irqsave(&cpufreq_driver_lock, flags); + cpufreq_driver = NULL; + write_unlock_irqrestore(&cpufreq_driver_lock, flags); + up_write(&cpufreq_rwsem); return 0; }