Skip to contentSkip to conversation

async: make sure independent async domains can't accidentally entangle.

PATCH In review James Bottomley 2 participants 3 messages May 24, 2009 SCSI

lore
Share & tools
lore

James Bottomley

author maintainer
#0 lore raw

PATCH 1/1 async: make sure independent async domains can't accidentally entangle.

The problem occurs when async_synchronize_full_domain() is called when the async_pending list is not empty. This will cause lowest_running() to return the cookie of the first entry on the async_pending list, which might be nothing at all to do with the domain being asked for and thus
cause the domain synchronization to wait for an unrelated domain. This
can cause a deadlock if domain synchronization is used from one domain to wait for another.

Fix by running over the async_pending list to see if any pending items actually belong to our domain (and return their cookies if they do).

Signed-off-by James Bottomley <[email protected]>

---

kernel/async.c +12 -7
index 968ef94..9b01c61 100644
@@ -92,19 +92,24 @@ extern int initcall_debug;
 static async_cookie_t  __lowest_in_progress(struct list_head *running)
 {
 	struct async_entry *entry;
+	unsigned long flags;
+	async_cookie_t ret = next_cookie; /* begin with "infinity" value */
+
 	if (!list_empty(running)) {
 		entry = list_first_entry(running,
 			struct async_entry, list);
-		return entry->cookie;
+		ret = entry->cookie;
 	} else if (!list_empty(&async_pending)) {
-		entry = list_first_entry(&async_pending,
-			struct async_entry, list);
-		return entry->cookie;
-	} else {
-		/* nothing in progress... next_cookie is "infinity" */
-		return next_cookie;
+		spin_lock_irqsave(&async_lock, flags);
+		list_for_each_entry(entry, &async_pending, list)
+			if (entry->running == running) {
+				ret = entry->cookie;
+				break;
+			}
+		spin_unlock_irqrestore(&async_lock, flags);
 	}
 
+	return ret;
 }
 
 static async_cookie_t  lowest_in_progress(struct list_head *running)
author maintainer
#1 lore raw
Quoted 8 lines · James Bottomley

The problem occurs when async_synchronize_full_domain() is called when the async_pending list is not empty. This will cause lowest_running() to return the cookie of the first entry on the async_pending list, which might be nothing at all to do with the domain being asked for and thus
cause the domain synchronization to wait for an unrelated domain. This
can cause a deadlock if domain synchronization is used from one domain to wait for another.

Quoted 4 lines

Fix by running over the async_pending list to see if any pending items actually belong to our domain (and return their cookies if they do).

Signed-off-by James Bottomley <[email protected]>

OK, so that version locked up under testing ... this version doesn't --- I think the phrase "MUST be called with the lock held!" was supposed to be some sort of clue ...

James

---

kernel/async.c +9 -7
index 968ef94..13ed571 100644
@@ -92,19 +92,21 @@ extern int initcall_debug;
 static async_cookie_t  __lowest_in_progress(struct list_head *running)
 {
 	struct async_entry *entry;
+	async_cookie_t ret = next_cookie; /* begin with "infinity" value */
+
 	if (!list_empty(running)) {
 		entry = list_first_entry(running,
 			struct async_entry, list);
-		return entry->cookie;
+		ret = entry->cookie;
 	} else if (!list_empty(&async_pending)) {
-		entry = list_first_entry(&async_pending,
-			struct async_entry, list);
-		return entry->cookie;
-	} else {
-		/* nothing in progress... next_cookie is "infinity" */
-		return next_cookie;
+		list_for_each_entry(entry, &async_pending, list)
+			if (entry->running == running) {
+				ret = entry->cookie;
+				break;
+			}
 	}
 
+	return ret;
 }
 
 static async_cookie_t  lowest_in_progress(struct list_head *running)
#2 lore raw
Quoted 13 lines · On Sun, 24 May 2009 13:50:43 -0500 James Bottomley…
Quoted 12 lines · James Bottomley

The problem occurs when async_synchronize_full_domain() is called when the async_pending list is not empty. This will cause lowest_running() to return the cookie of the first entry on the async_pending list, which might be nothing at all to do with the domain being asked for and thus cause the domain synchronization to
wait for an unrelated domain. This can cause a deadlock if domain
synchronization is used from one domain to wait for another.

Fix by running over the async_pending list to see if any pending items actually belong to our domain (and return their cookies if they do).

Quoted 6 lines
Quoted 2 lines

Signed-off-by: James Bottomley
<[email protected]>

OK, so that version locked up under testing ... this version doesn't --- I think the phrase "MUST be called with the lock held!" was supposed to be some sort of clue ...

there's not many comments there.. but the ones that exist do mean something ;-)

I'll craft this patch into your original description and send it to Linus to see if he'll take it for .30, otherwise it'll go for .31

Signature · 3 lines
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org