Skip to contentSkip to conversation

dca: missing unlock in unregister_dca_providers()

PATCH In review Dan Carpenter 3 participants 4 messages Nov 17, 2010 – Nov 22, 2010 Kernel janitors

lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 dca: missing unlock in unregister_dca_providers()

We return here with the lock held and IRQs disabled by mistake.

Signed-off-by Dan Carpenter <[email protected]>
drivers/dca/dca-core.c +3 -1
index b98c676..b4c95be 100644
@@ -110,8 +110,10 @@ static void unregister_dca_providers(void)
 
 	/* at this point only one domain in the list is expected */
 	domain = list_first_entry(&dca_domains, struct dca_domain, node);
-	if (!domain)
+	if (!domain) {
+		spin_unlock_irqrestore(&dca_lock, flags);
 		return;
+	}
 
 	list_for_each_entry_safe(dca, _dca, &domain->dca_providers, node) {
 		list_del(&dca->node);
#1 lore raw
Quoted 10 lines · On Wed, 17 Nov 2010 08:10:32 +0300 Dan Carpenter <[email protected]> wrote:

We return here with the lock held and IRQs disabled by mistake.

Signed-off-by Dan Carpenter <[email protected]>
drivers/dca/dca-core.c +0 -0
index b98c676..b4c95be 100644
@@ -110,8 +110,10 @@ static void unregister_dca_providers(void)

drivers/dca/dca-core.c static void unregister_dca_providers(void) · line 119

Quoted 10 lines
drivers/dca/dca-core.c +3 -1
 	/* at this point only one domain in the list is expected */
 	domain = list_first_entry(&dca_domains, struct dca_domain, node);
-	if (!domain)
+	if (!domain) {
+		spin_unlock_irqrestore(&dca_lock, flags);
 		return;
+	}
 
 	list_for_each_entry_safe(dca, _dca, &domain->dca_providers, node) {
 		list_del(&dca->node);

I think the code's just bogus, actually.
list_first_entry(&dca_domains) can't return NULL.

author
#2 lore raw

PATCH v2 1/1 dca: remove unneeded NULL check

The return here doesn't release the locks or re-enable IRQs. But as Andrew Morton points out, domain is never NULL. list_first_entry() essentially never returns NULL and also we already verified that the list is not empty.

Signed-off-by Dan Carpenter <[email protected]>
drivers/dca/dca-core.c +0 -2
index b98c676..c461eda 100644
@@ -110,8 +110,6 @@ static void unregister_dca_providers(void)
 
 	/* at this point only one domain in the list is expected */
 	domain = list_first_entry(&dca_domains, struct dca_domain, node);
-	if (!domain)
-		return;
 
 	list_for_each_entry_safe(dca, _dca, &domain->dca_providers, node) {
 		list_del(&dca->node);
Acked-by on v2 1/1
#3 lore raw

RE: [patch v2] dca: remove unneeded NULL check

Quoted 12 lines · Dan Carpenter

The return here doesn't release the locks or re-enable IRQs. But as Andrew Morton points out, domain is never NULL. list_first_entry() essentially never returns NULL and also we already verified that the list is not empty.

Signed-off-by Dan Carpenter <[email protected]>
drivers/dca/dca-core.c index renamed from drivers/dca/dca-core.c +0 -0

b98c676..c461eda 100644

drivers/dca/dca-core.c static void unregister_dca_providers(void) · line 115

Quoted 8 lines
drivers/dca/dca-core.c +0 -2
@@ -110,8 +110,6 @@ static void unregister_dca_providers(void)
 
 	/* at this point only one domain in the list is expected */
 	domain = list_first_entry(&dca_domains, struct dca_domain, node);
-	if (!domain)
-		return;
 
 	list_for_each_entry_safe(dca, _dca, &domain->dca_providers, node) {
 		list_del(&dca->node);
Acked-by Maciej Sosnowski <[email protected]>