From patchwork Thu Aug 13 03:24:17 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Wang Limei-E12499 X-Patchwork-Id: 40971 X-Patchwork-Delegate: khilman@deeprootsystems.com Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n7D3OkHO013793 for ; Thu, 13 Aug 2009 03:24:46 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752638AbZHMDYn (ORCPT ); Wed, 12 Aug 2009 23:24:43 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752645AbZHMDYn (ORCPT ); Wed, 12 Aug 2009 23:24:43 -0400 Received: from mail128.messagelabs.com ([216.82.250.131]:49406 "EHLO mail128.messagelabs.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752638AbZHMDYm convert rfc822-to-8bit (ORCPT ); Wed, 12 Aug 2009 23:24:42 -0400 X-VirusChecked: Checked X-Env-Sender: E12499@motorola.com X-Msg-Ref: server-13.tower-128.messagelabs.com!1250133883!15029905!1 X-StarScan-Version: 6.1.3; banners=-,-,- X-Originating-IP: [136.182.1.12] Received: (qmail 31532 invoked from network); 13 Aug 2009 03:24:44 -0000 Received: from motgate2.mot.com (HELO motgate2.mot.com) (136.182.1.12) by server-13.tower-128.messagelabs.com with DHE-RSA-AES256-SHA encrypted SMTP; 13 Aug 2009 03:24:44 -0000 Received: from il27exr03.cig.mot.com (il27exr03.mot.com [10.17.196.72]) by motgate2.mot.com (8.14.3/8.14.3) with ESMTP id n7D3Ohqn015714 for ; Wed, 12 Aug 2009 20:24:43 -0700 (MST) Received: from zmy16exm62.ds.mot.com (zmy16exm62.ap.mot.com [10.179.4.33]) by il27exr03.cig.mot.com (8.13.1/8.13.0) with ESMTP id n7D3Ofef006584 for ; Wed, 12 Aug 2009 22:24:42 -0500 (CDT) X-MimeOLE: Produced By Microsoft Exchange V6.5 Content-class: urn:content-classes:message MIME-Version: 1.0 Subject: RE: Linux-omap PM: Fix dead lock condition in resource_release(vdd1_opp) Date: Thu, 13 Aug 2009 11:24:17 +0800 Message-ID: In-Reply-To: <87ws5bk58n.fsf@deeprootsystems.com> X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: Linux-omap PM: Fix dead lock condition in resource_release(vdd1_opp) thread-index: AcoZ1ufaXxUaQ8NUTzW26lKIECX0LwB7fXEg References: <87ws5bk58n.fsf@deeprootsystems.com> From: "Wang Limei-E12499" To: "Kevin Hilman" , "Romit Dasgupta" Cc: , "Wang Sawsd-A24013" , "Wang Limei-E12499" Sender: linux-omap-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-omap@vger.kernel.org Kevin and Romit, I agreed with you, thanks Kevin and Romit for the comments! Chunqiu Wang coded resource-based mutex, below is the patch. Pls review and let us know your feedback. From 31f87ffb8eb1f854a9adb7fd96011d490f4655fa Mon Sep 17 00:00:00 2001 From: Chunqiu Wang Date: Wed, 12 Aug 2009 16:22:09 +0800 Subject: [PATCH] Fix resource framework mutex lock issue when resource_get or resource_release are called nestedly. Signed-off-by: Chunqiu Wang --- arch/arm/plat-omap/include/mach/resource.h | 2 + arch/arm/plat-omap/resource.c | 38 +++++++++++++-------------- 2 files changed, 20 insertions(+), 20 deletions(-) if (resp->ops->validate_level) { ret = resp->ops->validate_level(resp, level); @@ -361,16 +362,12 @@ int resource_request(const char *name, struct device *dev, } user->level = level; + /* Recompute and set the current level for the resource */ + ret = update_resource_level(resp); res_unlock: - up(&res_mutex); - /* - * Recompute and set the current level for the resource. - * NOTE: update_resource level moved out of spin_lock, as it may call - * pm_qos_add_requirement, which does a kzmalloc. This won't be allowed - * in iterrupt context. The spin_lock still protects add/remove users. - */ - if (!ret) - ret = update_resource_level(resp); + up(&resp->resource_mutex); + +ret: return ret; } EXPORT_SYMBOL(resource_request); @@ -393,14 +390,14 @@ 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; + goto ret; } + down(&resp->resource_mutex); list_for_each_entry(user, &resp->users_list, node) { if (user->dev == dev) { found = 1; @@ -421,7 +418,9 @@ 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); + up(&resp->resource_mutex); + +ret: return ret; } EXPORT_SYMBOL(resource_release); @@ -438,15 +437,14 @@ int resource_get_level(const char *name) struct shared_resource *resp; u32 ret; - down(&res_mutex); - resp = _resource_lookup(name); + resp = resource_lookup(name); if (!resp) { printk(KERN_ERR "resource_release: Invalid resource name\n"); - up(&res_mutex); return -EINVAL; } + down(&resp->resource_mutex); ret = resp->curr_level; - up(&res_mutex); + up(&resp->resource_mutex); return ret; } EXPORT_SYMBOL(resource_get_level); diff --git a/arch/arm/plat-omap/include/mach/resource.h b/arch/arm/plat-omap/include/mach/resource.h index f91d8ce..389cb67 100644 --- a/arch/arm/plat-omap/include/mach/resource.h +++ b/arch/arm/plat-omap/include/mach/resource.h @@ -22,6 +22,7 @@ #include #include +#include #include #include @@ -46,6 +47,7 @@ struct shared_resource { /* Shared resource operations */ struct shared_resource_ops *ops; struct list_head node; + struct semaphore resource_mutex; }; struct shared_resource_ops { diff --git a/arch/arm/plat-omap/resource.c b/arch/arm/plat-omap/resource.c index ec31727..758a138 100644 --- a/arch/arm/plat-omap/resource.c +++ b/arch/arm/plat-omap/resource.c @@ -264,6 +264,7 @@ int resource_register(struct shared_resource *resp) return -EEXIST; INIT_LIST_HEAD(&resp->users_list); + sema_init(&resp->resource_mutex, 1); down(&res_mutex); /* Add the resource to the resource list */ @@ -326,14 +327,14 @@ 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; + goto ret; } + down(&resp->resource_mutex); /* Call the resource specific validate function */