Re: [PATCH 0/1] stop the unbound recursion in preempt_schedule_context()

[Oleg Nesterov]

And, Frederic, this is off-topic but I am just curious...

I can't understand why exception_enter() calls context_tracking_user_exit()
unconditionally (unlike exception_exit), and why enter/exit need to check
context_tracking.state with irqs disabled.

IOW, any reason why the patch below is wrong?

Of course, context_tracking is per-cpu, so

	if (context_tracking_in_user()) {
		local_irq_save(flags);
		...

can be preempted and change the state on another CPU. But this should
be fine because the task must see the same .state on all CPU's or the
whole preempt context-tracking logic is broken?

Oleg.


--- x/include/linux/context_tracking.h
+++ x/include/linux/context_tracking.h
@@ -29,15 +29,14 @@ static inline void user_exit(void)
 
 static inline enum ctx_state exception_enter(void)
 {
-	enum ctx_state prev_ctx;
-
-	if (!context_tracking_is_enabled())
-		return 0;
-
-	prev_ctx = this_cpu_read(context_tracking.state);
-	context_tracking_user_exit();
+	if (context_tracking_is_enabled()) {
+		if (context_tracking_in_user()) {
+			context_tracking_user_exit();
+			return IN_USER;
+		}
+	}
 
-	return prev_ctx;
+	return IN_KERNEL;
 }
 
 static inline void exception_exit(enum ctx_state prev_ctx)
--- x/kernel/context_tracking.c
+++ x/kernel/context_tracking.c
@@ -74,8 +74,8 @@ void context_tracking_user_enter(void)
 	/* Kernel threads aren't supposed to go to userspace */
 	WARN_ON_ONCE(!current->mm);
 
-	local_irq_save(flags);
-	if ( __this_cpu_read(context_tracking.state) != IN_USER) {
+	if (!context_tracking_in_user()) {
+		local_irq_save(flags);
 		if (__this_cpu_read(context_tracking.active)) {
 			trace_user_enter(0);
 			/*
@@ -102,8 +102,8 @@ void context_tracking_user_enter(void)
 		 * is false because we know that CPU is not tickless.
 		 */
 		__this_cpu_write(context_tracking.state, IN_USER);
+		local_irq_restore(flags);
 	}
-	local_irq_restore(flags);
 }
 NOKPROBE_SYMBOL(context_tracking_user_enter);
 
@@ -128,8 +128,8 @@ void context_tracking_user_exit(void)
 	if (in_interrupt())
 		return;
 
-	local_irq_save(flags);
-	if (__this_cpu_read(context_tracking.state) == IN_USER) {
+	if (context_tracking_in_user()) {
+		local_irq_save(flags);
 		if (__this_cpu_read(context_tracking.active)) {
 			/*
 			 * We are going to run code that may use RCU. Inform
@@ -140,8 +140,8 @@ void context_tracking_user_exit(void)
 			trace_user_exit(0);
 		}
 		__this_cpu_write(context_tracking.state, IN_KERNEL);
+		local_irq_restore(flags);
 	}
-	local_irq_restore(flags);
 }
 NOKPROBE_SYMBOL(context_tracking_user_exit);
 

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