NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

kern/55781: rump_init() does differentiate when all CPUs are initialized



>Number:         55781
>Category:       kern
>Synopsis:       rump_init() does differentiate when all CPUs are initialized
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Nov 04 00:35:00 +0000 2020
>Originator:     Ruslan Nikolaev
>Release:        master
>Organization:
Virginia Tech
>Environment:
>Description:
While working on rumprun-smp (github.com/ssrg-vt/rumprun-smp), we found one particular problem with CPU initialization.

The first (bootstrap) CPU should call rump_init(). However, it is not
clear when for other CPUs it is already safe to schedule threads that are placed to the run queue (e.g., due to blocking, etc., possibly also due to events happening in rump_init() itself).
If we allow the scheduler to use other CPUs prematurely, they may not yet be initialized properly causing various bugs and race conditions.

One approach is to let all other CPUs sleep (or busy wait) until the bootsrap CPU fully initializes all these CPUs in rump_init(). Only after that, allow the scheduler to proceed.

Note that the scheduler cannot simply freeze all other CPUs until after rump_init() completes on the bootstrap CPU. There are several issues, e.g., per-CPU initialization of cprng_init_cpu() which should be scheduled on each CPU and/or similar issues.
>How-To-Repeat:

>Fix:
The way we solved it is just by passing an additional callback function to rump_init(). May not necessarily be the best solution but it is not intrusive and does not require significant changes in rump_init():

diff --git a/sys/rump/include/rump/rump.h b/sys/rump/include/rump/rump.h
index 8da2737f6f77..2ff19adab38a 100644
--- a/sys/rump/include/rump/rump.h
+++ b/sys/rump/include/rump/rump.h
@@ -118,6 +118,7 @@ void	rump_unschedule(void);
 void	rump_printevcnts(void);
 
 int	rump_daemonize_begin(void);
+int	rump_init_callback(void (*cpuinit_callback) (void));
 int	rump_init(void);
 int	rump_init_server(const char *);
 int	rump_daemonize_done(int);
diff --git a/sys/rump/librump/rumpkern/rump.c b/sys/rump/librump/rumpkern/rump.c
index 91f8462f4da4..6397dc6c8952 100644
--- a/sys/rump/librump/rumpkern/rump.c
+++ b/sys/rump/librump/rumpkern/rump.c
@@ -218,7 +218,7 @@ RUMP_COMPONENT(RUMP_COMPONENT_POSTINIT)
 #endif /* RUMP_USE_CTOR */
 
 int
-rump_init(void)
+rump_init_callback(void (*cpuinit_callback) (void))
 {
 	char buf[256];
 	struct timespec bts;
@@ -231,7 +231,7 @@ rump_init(void)
 	if (rump_inited)
 		return 0;
 	else if (rump_inited == -1)
-		panic("rump_init: host process restart required");
+		panic("rump_init_callback: host process restart required");
 	else
 		rump_inited = 1;
 
@@ -393,6 +393,9 @@ rump_init(void)
 
 	mp_online = true;
 
+	if (cpuinit_callback)
+		cpuinit_callback();
+
 	/* CPUs are up.  allow kernel threads to run */
 	rump_thread_allow(NULL);
 
@@ -496,6 +499,12 @@ rump_init(void)
 
 	return 0;
 }
+
+int
+rump_init(void)
+{
+	return rump_init_callback(NULL);
+}
 /* historic compat */
 __strong_alias(rump__init,rump_init);
 



Home | Main Index | Thread Index | Old Index