Fix a dead loop in async_synchronize_full()

This patch tries to fix a dead loop in async_synchronize_full(), which could be seen when preemption is disabled on a single cpu machine.

void async_synchronize_full(void)

{
        do {
                async_synchronize_cookie(next_cookie);
        } while (!list_empty(&async_running) || !

list_empty(&async_pending));
}

async_synchronize_cookie() calls async_synchronize_cookie_domain() with &async_running as the default domain to synchronize.

However, there might be some works in the async_pending list from other domains. On a single cpu system, without preemption, there is no chance for the other works to finish, so async_synchronize_full() enters a dead loop.

It seems async_synchronize_full() wants to synchronize all entries in all running lists(domains), so maybe we could just check the entry_count to know whether all works are finished.

Currently, async_synchronize_cookie_domain() expects a non-NULL running list ( if NULL, there would be NULL pointer dereference ), so maybe a NULL pointer could be used as an indication for the functions to synchronize all works in all domains.

---
 kernel/async.c |   13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

index bd0c168..32d8dc9 100644
@@ -86,6 +86,13 @@ static async_cookie_t  __lowest_in_progress(struct

list_head *running)
{
struct async_entry *entry;

+ if (!running) { /* just check the entry count */
+ if (atomic_read(&entry_count))
+ return 0; /* smaller than any cookie */
+ else
+ return next_cookie;
+ }
+
if (!list_empty(running)) {
entry = list_first_entry(running,
struct async_entry, list);
@@ -236,9 +243,7 @@ EXPORT_SYMBOL_GPL(async_schedule_domain);
*/
void async_synchronize_full(void)
{
- do {
- async_synchronize_cookie(next_cookie);
- } while (!list_empty(&async_running) || !list_empty(&async_pending));
+ async_synchronize_cookie_domain(next_cookie, NULL);
}
EXPORT_SYMBOL_GPL(async_synchronize_full);

@@ -258,7 +263,7 @@ EXPORT_SYMBOL_GPL(async_synchronize_full_domain);
/**
* async_synchronize_cookie_domain - synchronize asynchronous function
calls within a certain domain with cookie checkpointing
* @cookie: async_cookie_t to use as checkpoint
- * @running: running list to synchronize on
+ * @running: running list to synchronize on, NULL indicates all lists *
* This function waits until all asynchronous function calls for the
* synchronization domain specified by the running list @list submitted

1.7.9.5

The patch is fairly wordwrapped - please fix up your email client.

More seriously, it does not apply to linux-next due to some fairly significant changes which have been sitting in Dan's tree since May. What's going on?

Those changes missed the 3.5 merge window, but now that they have Arjan's ack they should head upstream via James for 3.6. Right now they are on his pending [1] branch.

As far as the comment:

...at first glance this is what the new async patches achieve. async_synchronize_full should now sync work across all domains, but if you can reproduce this bug it would be nice to confirm that the pending changes fix it.

--
Dan

[1]: http://git.kernel.org/?p=linux/kernel/git/jejb/scsi.git;a=shortlog;h=refs/heads/pending

I have tested your pending patches, they fix the problem here.

But with ASYNC_DOMAIN_EXCLUSIVE added for the domains defined on the stack, I think we lack a function that could wait for all the works in all domains (however, maybe actually we don't need such an interface).

Also, I think it's not good to exclude them from async_synchronize_full() just because they are defined on the stack.

[ adding James ]

Thanks!

James, if you get the chance please add:

...to the pending set, or I can just resend. Let me know.

ASYNC_DOMAIN can be used to allow on-stack domains to be flushed via async_synchronize_full(). However, if you know ahead of time that your work items do not need to be anonymously flushed and you know the lifetime of your domain ASYNC_DOMAIN_EXCLUSIVE + async_synchronize_full_domain() are there to prevent unnecessary entanglements.

If for some reason you want to have temporary on stack domains be globally visible I included an async_unregister_domain() routine to make the api complete. It has a comment that using ASYNC_DOMAIN_EXCLUSIVE for such domains is preferred.

--
Dan

Is this still scheduled to go into 3.5? I'm asking because -rc7 has been released and does not contain this fix. W/o this fix, my powerpc system won't boot[0] :-\

Thanks,
Christian.

[0] http://nerdbynature.de/bits/3.5.0-rc5/soft_lockup/

BOFH excuse #28:

CPU radiator broken

I don't expect James is going to push my async changes for 3.5. So maybe we should go with Zhong's smaller fix for 3.5 and then replace with the ASYNC_DOMAIN patches in 3.6?

--
Dan

Makes sense to me! I have been using Zhong's fix for my testing on PowerPC for some weeks now with no trouble.

Thanx, Paul

resend it again with the email client fixed... in case it is needed

This patch tries to fix a dead loop in async_synchronize_full(), which could be seen when preemption is disabled on a single cpu machine.

void async_synchronize_full(void)

{
        do {
                async_synchronize_cookie(next_cookie);
        } while (!list_empty(&async_running) || !

list_empty(&async_pending));
}

async_synchronize_cookie() calls async_synchronize_cookie_domain() with &async_running as the default domain to synchronize.

However, there might be some works in the async_pending list from other domains. On a single cpu system, without preemption, there is no chance for the other works to finish, so async_synchronize_full() enters a dead loop.

It seems async_synchronize_full() wants to synchronize all entries in all running lists(domains), so maybe we could just check the entry_count to know whether all works are finished.

Currently, async_synchronize_cookie_domain() expects a non-NULL running list ( if NULL, there would be NULL pointer dereference ), so maybe a NULL pointer could be used as an indication for the functions to synchronize all works in all domains.

---
 kernel/async.c |   13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

index bd0c168..32d8dc9 100644
@@ -86,6 +86,13 @@ static async_cookie_t  __lowest_in_progress(struct list_head *running)
 {
 	struct async_entry *entry;
 
+	if (!running) { /* just check the entry count */
+		if (atomic_read(&entry_count))
+			return 0; /* smaller than any cookie */
+		else
+			return next_cookie;
+	}
+
 	if (!list_empty(running)) {
 		entry = list_first_entry(running,
 			struct async_entry, list);

@@ -236,9 +243,7 @@ EXPORT_SYMBOL_GPL(async_schedule_domain);
  */
 void async_synchronize_full(void)
 {
-	do {
-		async_synchronize_cookie(next_cookie);
-	} while (!list_empty(&async_running) || !list_empty(&async_pending));
+	async_synchronize_cookie_domain(next_cookie, NULL);
 }
 EXPORT_SYMBOL_GPL(async_synchronize_full);
 

@@ -258,7 +263,7 @@ EXPORT_SYMBOL_GPL(async_synchronize_full_domain);
 /**
  * async_synchronize_cookie_domain - synchronize asynchronous function calls within a certain domain with cookie checkpointing
  * @cookie: async_cookie_t to use as checkpoint
- * @running: running list to synchronize on
+ * @running: running list to synchronize on, NULL indicates all lists
  *
  * This function waits until all asynchronous function calls for the
  * synchronization domain specified by the running list @list submitted

1.7.9.5

Ah, sorry for that, I will check it.

Just went through Dan's patches, it seems that they also had async_synchronize_full() to sync all domains. I will test/check those patches, and drop this one if the result is good.