| Message ID | 1252006638-1697-1-git-send-email-mike@android.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Kevin Hilman |
| Headers | show |
Mike Chan <mike@android.com> writes: > Checking if the resource is already registered and adding to the list > must be atomic or bad things can happen. > > Signed-off-by: Mike Chan <mike@android.com> Functionally, this looks fine. Some procedural details still to work out: Are you the original author of this? or was it Chunqiu or Limei? The changelog should be prefixed with a From: line of the original author (handled by git-format-patch) if it was not you. Also add the other's signoffs if appropriate. What tree does this apply to? It's not specified, and does not apply to either current PM branch or pm-2.6.29. Kevin > --- > arch/arm/plat-omap/resource.c | 14 +++++++++----- > 1 files changed, 9 insertions(+), 5 deletions(-) > > diff --git a/arch/arm/plat-omap/resource.c b/arch/arm/plat-omap/resource.c > index e9ace13..9d20249 100644 > --- a/arch/arm/plat-omap/resource.c > +++ b/arch/arm/plat-omap/resource.c > @@ -253,6 +253,7 @@ int resource_refresh(void) > */ > int resource_register(struct shared_resource *resp) > { > + int ret = 0; > if (!resp) > return -EINVAL; > > @@ -260,13 +261,15 @@ int resource_register(struct shared_resource *resp) > return -EINVAL; > > /* Verify that the resource is not already registered */ > - if (resource_lookup(resp->name)) > - return -EEXIST; > + down(&res_mutex); > + if (_resource_lookup(resp->name)) { > + ret = -EEXIST; > + goto out; > + } > > INIT_LIST_HEAD(&resp->users_list); > mutex_init(&resp->resource_mutex); > > - down(&res_mutex); > /* Add the resource to the resource list */ > list_add(&resp->node, &res_list); > > @@ -274,10 +277,11 @@ int resource_register(struct shared_resource *resp) > if (resp->ops->init) > resp->ops->init(resp); > > - up(&res_mutex); > pr_debug("resource: registered %s\n", resp->name); > > - return 0; > +out: > + up(&res_mutex); > + return ret; > } > EXPORT_SYMBOL(resource_register); > > -- > 1.5.4.5 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Sep 8, 2009 at 3:39 PM, Kevin Hilman<khilman@deeprootsystems.com> wrote: > Mike Chan <mike@android.com> writes: > >> Checking if the resource is already registered and adding to the list >> must be atomic or bad things can happen. >> >> Signed-off-by: Mike Chan <mike@android.com> > > Functionally, this looks fine. Â Some procedural details still to work > out: > > Are you the original author of this? Â or was it Chunqiu or Limei? Â The > changelog should be prefixed with a From: line of the original author > (handled by git-format-patch) if it was not you. Â Also add the other's > signoffs if appropriate. > I am the original author of this patch. Chunqiu sent two patches out 1) a per-resource level mutex (vs global). 2) Make update_resource_level() thread safe. I've been meaning to send them out to l-o on behalf of Chunqiu. > What tree does this apply to? Â It's not specified, and does not apply > to either current PM branch or pm-2.6.29. > I thought I rebased this off of pm but I may have goofed. I can send a new one. -- Mike > Kevin > >> --- >> Â arch/arm/plat-omap/resource.c | Â 14 +++++++++----- >> Â 1 files changed, 9 insertions(+), 5 deletions(-) >> >> diff --git a/arch/arm/plat-omap/resource.c b/arch/arm/plat-omap/resource.c >> index e9ace13..9d20249 100644 >> --- a/arch/arm/plat-omap/resource.c >> +++ b/arch/arm/plat-omap/resource.c >> @@ -253,6 +253,7 @@ int resource_refresh(void) >> Â */ >> Â int resource_register(struct shared_resource *resp) >> Â { >> + Â Â int ret = 0; >> Â Â Â if (!resp) >> Â Â Â Â Â Â Â return -EINVAL; >> >> @@ -260,13 +261,15 @@ int resource_register(struct shared_resource *resp) >> Â Â Â Â Â Â Â return -EINVAL; >> >> Â Â Â /* Verify that the resource is not already registered */ >> - Â Â if (resource_lookup(resp->name)) >> - Â Â Â Â Â Â return -EEXIST; >> + Â Â down(&res_mutex); >> + Â Â if (_resource_lookup(resp->name)) { >> + Â Â Â Â Â Â ret = -EEXIST; >> + Â Â Â Â Â Â goto out; >> + Â Â } >> >> Â Â Â INIT_LIST_HEAD(&resp->users_list); >> Â Â Â mutex_init(&resp->resource_mutex); >> >> - Â Â down(&res_mutex); >> Â Â Â /* Add the resource to the resource list */ >> Â Â Â list_add(&resp->node, &res_list); >> >> @@ -274,10 +277,11 @@ int resource_register(struct shared_resource *resp) >> Â Â Â if (resp->ops->init) >> Â Â Â Â Â Â Â resp->ops->init(resp); >> >> - Â Â up(&res_mutex); >> Â Â Â pr_debug("resource: registered %s\n", resp->name); >> >> - Â Â return 0; >> +out: >> + Â Â up(&res_mutex); >> + Â Â return ret; >> Â } >> Â EXPORT_SYMBOL(resource_register); >> >> -- >> 1.5.4.5 > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" 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/arch/arm/plat-omap/resource.c b/arch/arm/plat-omap/resource.c index e9ace13..9d20249 100644 --- a/arch/arm/plat-omap/resource.c +++ b/arch/arm/plat-omap/resource.c @@ -253,6 +253,7 @@ int resource_refresh(void) */ int resource_register(struct shared_resource *resp) { + int ret = 0; if (!resp) return -EINVAL; @@ -260,13 +261,15 @@ int resource_register(struct shared_resource *resp) return -EINVAL; /* Verify that the resource is not already registered */ - if (resource_lookup(resp->name)) - return -EEXIST; + down(&res_mutex); + if (_resource_lookup(resp->name)) { + ret = -EEXIST; + goto out; + } INIT_LIST_HEAD(&resp->users_list); mutex_init(&resp->resource_mutex); - down(&res_mutex); /* Add the resource to the resource list */ list_add(&resp->node, &res_list); @@ -274,10 +277,11 @@ int resource_register(struct shared_resource *resp) if (resp->ops->init) resp->ops->init(resp); - up(&res_mutex); pr_debug("resource: registered %s\n", resp->name); - return 0; +out: + up(&res_mutex); + return ret; } EXPORT_SYMBOL(resource_register);
Checking if the resource is already registered and adding to the list must be atomic or bad things can happen. Signed-off-by: Mike Chan <mike@android.com> --- arch/arm/plat-omap/resource.c | 14 +++++++++----- 1 files changed, 9 insertions(+), 5 deletions(-)