Skip to contentSkip to conversation

change current_is_single_threaded() to use for_each_thread()

PATCH In review Oleg Nesterov 2 participants 4 messages Oct 5, 2015 – Oct 7, 2015 Library code

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 change current_is_single_threaded() to use for_each_thread()

Change current_is_single_threaded() to use for_each_thread() rather than deprecated while_each_thread().

---
 lib/is_single_threaded.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)
lib/is_single_threaded.c +2 -3
index bd2bea9..391fd23 100644
@@ -36,8 +36,7 @@ bool current_is_single_threaded(void)
 		if (unlikely(p == task->group_leader))
 			continue;
 
-		t = p;
-		do {
+		for_each_thread(p, t) {
 			if (unlikely(t->mm == mm))
 				goto found;
 			if (likely(t->mm))
@@ -48,7 +47,7 @@ bool current_is_single_threaded(void)
 			 * forked before exiting.
 			 */
 			smp_rmb();
-		} while_each_thread(p, t);
+		}
 	}
 	ret = true;
 found:
Signature · 1 line
1.5.5.1
#1 lore raw
Quoted 2 lines · Oleg Nesterov

Change current_is_single_threaded() to use for_each_thread() rather than deprecated while_each_thread().

Wouldn't the old way be more efficient, given there's always going to be at least one thread? Granted, it's not much of an efficiency boost...

David

#2 lore raw
Quoted 8 lines · David Howells
Quoted 2 lines · Oleg Nesterov

Change current_is_single_threaded() to use for_each_thread() rather than deprecated while_each_thread().

Wouldn't the old way be more efficient, given there's always going to be at least one thread?

Yes, unfortunately. Before this change (in the likely case) we never do next_thread(), after this patch we always do list_first_entry().

Quoted 1 line

Granted, it's not much of an efficiency boost...

Yes, and while_each_thread() is buggy. See the changelog in 0c740d0afc "introduce for_each_thread() to replace the buggy while_each_thread().

Yes, the race is almost purely theoretical, but still.

To clarify, we had some "real" bugs fixed by s/while_each_thread/ for_each_thread/, but only because while_each_thread() was used wrongly. for_each_thread() is more safe, it only needs the stable task_struct.

Oleg.

#3 lore raw
Quoted 3 lines · Oleg Nesterov

To clarify, we had some "real" bugs fixed by s/while_each_thread/ for_each_thread/, but only because while_each_thread() was used wrongly. for_each_thread() is more safe, it only needs the stable task_struct.

Fair enough.

David