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.

Reported-by Paul E. McKenney <[email protected]>

Signed-off-by Li Zhong <[email protected]>

Tested-by Paul E. McKenney <[email protected]>

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

kernel/async.c +7 -0

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,

kernel/async.c +1 -3

 			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

Signature · 7 lines

1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Quoted 2 lines · Li Zhong

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.

This helps in my case[0], will this patch be included in -rc6?

Thanks,
Christian.

[0] https://lists.ozlabs.org/pipermail/linuxppc-dev/2012-July/098922.html

Quoted 82 lines

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.

Reported-by Paul E. McKenney <[email protected]>

Signed-off-by Li Zhong <[email protected]>

Tested-by Paul E. McKenney <[email protected]>

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

kernel/async.c +7 -0

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,

kernel/async.c +1 -3

 			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

Signature · 7 lines

1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Review

Quoted 7 lines · Li Zhong

void async_synchronize_full(void)
{

(quoted diff) +1 -3

-       do {
-               async_synchronize_cookie(next_cookie);
-       } while (!list_empty(&async_running) || !list_empty(&async_pending));
+       async_synchronize_cookie_domain(next_cookie, NULL);
 }

A nitpick:

async_synchronize_cookie_domain(next_cookie, NULL);

equals to

async_synchronize_full_domain(NULL)