From patchwork Tue Aug 6 17:23:11 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Viresh Kumar X-Patchwork-Id: 18812 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-ye0-f199.google.com (mail-ye0-f199.google.com [209.85.213.199]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id D20A125DF6 for ; Tue, 6 Aug 2013 17:24:44 +0000 (UTC) Received: by mail-ye0-f199.google.com with SMTP id l12sf707553yen.6 for ; Tue, 06 Aug 2013 10:24:44 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=mime-version:x-gm-message-state:delivered-to:from:to:cc:subject :date:message-id:in-reply-to:references:in-reply-to:references :x-original-sender:x-original-authentication-results:precedence :mailing-list:list-id:list-post:list-help:list-archive :list-unsubscribe; bh=NIwHRl80vrO9Ha5y/Aw3wwUvppIBnRdXZevpf8kO+hQ=; b=ec/XkieBx27lnmf+6WWcIjLeULz+NXNfrh2rmUvU4qpxyNDl8kTiA3z06nAfkVTyjS O58KctMNaASHaj0ng4LvYCoYXqb5vAFndDJHo90pe5Y7pIIYzWlumYooeOi/rre7dtMf xPS84AwL+YhexhEdJ2cw9gRwFsS83nP/4VAuH7syLepbaRHwVTcyPJmqyzBKvksBLuMT WUvL/yfXa9CAPl5FDh18qT3XmGgOMXwHMb4TjWFKnbi4i5B/q5cgCHgkg0aOFDrhTJsY 255OXXXlZBBNuvhO0HoBGX7sozVA4QlqmoEcxxL9+ICzlQspdz+IRQWUSExLUC3nUtuf JioA== X-Received: by 10.236.110.168 with SMTP id u28mr841236yhg.32.1375809884434; Tue, 06 Aug 2013 10:24:44 -0700 (PDT) MIME-Version: 1.0 X-BeenThere: patchwork-forward@linaro.org Received: by 10.49.30.35 with SMTP id p3ls351595qeh.97.gmail; Tue, 06 Aug 2013 10:24:44 -0700 (PDT) X-Received: by 10.58.196.132 with SMTP id im4mr696366vec.28.1375809884260; Tue, 06 Aug 2013 10:24:44 -0700 (PDT) Received: from mail-ve0-f170.google.com (mail-ve0-f170.google.com [209.85.128.170]) by mx.google.com with ESMTPS id bq2si533612vcb.115.2013.08.06.10.24.44 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Tue, 06 Aug 2013 10:24:44 -0700 (PDT) Received-SPF: neutral (google.com: 209.85.128.170 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.170; Received: by mail-ve0-f170.google.com with SMTP id 15so719473vea.29 for ; Tue, 06 Aug 2013 10:24:44 -0700 (PDT) X-Gm-Message-State: ALoCoQmKibeVoLK6wqafNfsUkbMjR8zX1SP214odjw2vzjOzisp2Mk4fexz8jitOmp3T8eZQrnih X-Received: by 10.52.117.208 with SMTP id kg16mr610366vdb.48.1375809884127; Tue, 06 Aug 2013 10:24:44 -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 pc8csp157181vcb; Tue, 6 Aug 2013 10:24:43 -0700 (PDT) X-Received: by 10.66.157.68 with SMTP id wk4mr4506439pab.72.1375809883064; Tue, 06 Aug 2013 10:24:43 -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 ql10si1944719pbb.190.2013.08.06.10.24.42 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Tue, 06 Aug 2013 10:24:43 -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 xb4so727338pbc.8 for ; Tue, 06 Aug 2013 10:24:42 -0700 (PDT) X-Received: by 10.66.163.229 with SMTP id yl5mr4496554pab.69.1375809882617; Tue, 06 Aug 2013 10:24:42 -0700 (PDT) Received: from localhost ([122.172.193.46]) by mx.google.com with ESMTPSA id aj3sm5256773pad.8.2013.08.06.10.24.38 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Tue, 06 Aug 2013 10:24:42 -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, Viresh Kumar Subject: [PATCH V2 09/11] cpufreq: Don't use cpufreq_driver->owner's refcount to protect critical sections Date: Tue, 6 Aug 2013 22:53:11 +0530 Message-Id: <678fa712ad13aa39a92568003fe0a9f7d2452cb3.1375809311.git.viresh.kumar@linaro.org> X-Mailer: git-send-email 1.7.12.rc2.18.g61b472e In-Reply-To: References: In-Reply-To: References: 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.170 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 62eddb6..ccaf025 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -91,6 +91,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); @@ -178,78 +184,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 * *********************************************************************/ @@ -694,12 +668,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); @@ -707,9 +681,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; } @@ -719,12 +694,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); @@ -732,9 +707,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; } @@ -1002,25 +978,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); @@ -1093,7 +1068,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; @@ -1111,8 +1087,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; } @@ -1424,10 +1400,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; @@ -1437,8 +1412,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); @@ -2131,9 +2106,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; }