| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <8bb80c381003091611l7211439ief40c5cf31e782a0@mail.gmail.com>
Date: Tue, 9 Mar 2010 16:11:52 -0800
From: Mike Chan <mike@...roid.com>
To: unlisted-recipients:; (no To-header on input)
Cc: android-kernel@...glegroups.com,
Chunqiu Wang <cqwang@...orola.com>,
Tony Lindgren <tony@...mide.com>,
Russell King <linux@....linux.org.uk>,
Rajendra Nayak <rnayak@...com>,
Tero Kristo <tero.kristo@...ia.com>,
Kalle Jokiniemi <ext-kalle.jokiniemi@...ia.com>,
Jouni Hogander <jouni.hogander@...ia.com>,
Kevin Hilman <khilman@...prootsystems.com>,
linux-omap@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] omap: resource: Add per-resource mutex for OMAP resource
framework
On Tue, Mar 9, 2010 at 4:09 PM, <y@...gle.com> wrote:
> From: Chunqiu Wang <cqwang@...orola.com>
>
> Current OMAP resource fwk uses a global res_mutex
> for resource_request and resource_release calls
> for all the available resources.It may cause dead
> lock if resource_request/resource_release is called
> recursively.
>
> For current OMAP3 VDD1/VDD2 resource, the change_level
> implementation is mach-omap2/resource34xx.c/set_opp(),
> when using resource_release to remove vdd1 constraint,
> this function may call resource_release again to release
> Vdd2 constrait if target vdd1 level is less than OPP3.
> in this scenario, the global res_mutex down operation
> will be called again, this will cause the second
> down operation hang there.
>
> To fix the problem, per-resource mutex is added
> to avoid hangup when resource_request/resource_release
> is called recursively.
>
> Signed-off-by: Chunqiu Wang <cqwang@...orola.com>
> Signed-off-by: Mike Chan <mike@...roid.com>
> ---
> arch/arm/plat-omap/include/plat/resource.h | 2 ++
> arch/arm/plat-omap/resource.c | 21 ++++++++++-----------
> 2 files changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm/plat-omap/include/plat/resource.h b/arch/arm/plat-omap/include/plat/resource.h
> index 9acebcc..b5aff1f 100644
> --- a/arch/arm/plat-omap/include/plat/resource.h
> +++ b/arch/arm/plat-omap/include/plat/resource.h
> @@ -54,6 +54,8 @@ struct shared_resource {
> /* Shared resource operations */
> struct shared_resource_ops *ops;
> struct list_head node;
> + /* Protect each resource */
> + struct mutex resource_mutex;
> };
>
> struct shared_resource_ops {
> diff --git a/arch/arm/plat-omap/resource.c b/arch/arm/plat-omap/resource.c
> index f1cdecf..0a7b79b 100644
> --- a/arch/arm/plat-omap/resource.c
> +++ b/arch/arm/plat-omap/resource.c
> @@ -277,6 +277,7 @@ int resource_register(struct shared_resource *resp)
> }
>
> INIT_LIST_HEAD(&resp->users_list);
> + mutex_init(&resp->resource_mutex);
>
> /* Add the resource to the resource list */
> list_add(&resp->node, &res_list);
> @@ -339,14 +340,13 @@ int resource_request(const char *name, struct device *dev,
> struct users_list *user;
> int found = 0, ret = 0;
>
> - down(&res_mutex);
> - resp = _resource_lookup(name);
> + resp = resource_lookup(name);
> if (!resp) {
> printk(KERN_ERR "resource_request: Invalid resource name\n");
> - ret = -EINVAL;
> - goto res_unlock;
> + return -EINVAL;
> }
>
> + mutex_lock(&resp->resource_mutex);
> /* Call the resource specific validate function */
> if (resp->ops->validate_level) {
> ret = resp->ops->validate_level(resp, level);
> @@ -375,7 +375,7 @@ int resource_request(const char *name, struct device *dev,
> user->level = level;
>
> res_unlock:
> - up(&res_mutex);
> + mutex_unlock(&resp->resource_mutex);
> /*
> * Recompute and set the current level for the resource.
> * NOTE: update_resource level moved out of spin_lock, as it may call
> @@ -406,14 +406,13 @@ int resource_release(const char *name, struct device *dev)
> struct users_list *user;
> int found = 0, ret = 0;
>
> - down(&res_mutex);
> - resp = _resource_lookup(name);
> + resp = resource_lookup(name);
> if (!resp) {
> printk(KERN_ERR "resource_release: Invalid resource name\n");
> - ret = -EINVAL;
> - goto res_unlock;
> + return -EINVAL;
> }
>
> + mutex_lock(&resp->resource_mutex);
> list_for_each_entry(user, &resp->users_list, node) {
> if (user->dev == dev) {
> found = 1;
> @@ -434,7 +433,7 @@ int resource_release(const char *name, struct device *dev)
> /* Recompute and set the current level for the resource */
> ret = update_resource_level(resp);
> res_unlock:
> - up(&res_mutex);
> + mutex_unlock(&resp->resource_mutex);
> return ret;
> }
> EXPORT_SYMBOL(resource_release);
> @@ -458,7 +457,7 @@ int resource_get_level(const char *name)
> up(&res_mutex);
> return -EINVAL;
> }
> - ret = resp->curr_level;
> + ret = resp->curr_level;
> up(&res_mutex);
> return ret;
> }
> --
> 1.6.6.2
>
>
Oops, my git client was slightly screwed up, apologies, I meant to
send this from mike@...roid.com, the patch is still good though :)
-- Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists