On 3/31/11 11:37 AM, John Baldwin wrote:
On Thursday, March 31, 2011 2:20:11 pm Attilio Rao wrote:
2011/3/31 John Baldwin<j...@freebsd.org>:
On Thursday, March 31, 2011 12:34:31 pm Attilio Rao wrote:
2011/3/31 John Baldwin<j...@freebsd.org>:
On Thursday, March 31, 2011 7:32:26 am Svatopluk Kraus wrote:
Hi,
I've got a page fault (because of NULL td_lock) in
thread_lock_flags() called from schedcpu() in /sys/kern/sched_4bsd.c
file. During process fork, new thread is linked to new process which
is linked to allproc list and both allproc_lock and new process lock
are unlocked before sched_fork() is called, where new thread td_lock
is initialized. Only PRS_NEW process status is on sentry but not
checked in schedcpu().
I think this should fix it:
Index: sched_4bsd.c
===================================================================
--- sched_4bsd.c (revision 220190)
+++ sched_4bsd.c (working copy)
@@ -463,6 +463,10 @@ schedcpu(void)
sx_slock(&allproc_lock);
FOREACH_PROC_IN_SYSTEM(p) {
PROC_LOCK(p);
+ if (p->p_state == PRS_NEW) {
+ PROC_UNLOCK(p);
+ continue;
+ }
FOREACH_THREAD_IN_PROC(p, td) {
awake = 0;
thread_lock(td);
I don't really think this fix is right because otherwise, when using
sched_4bsd anytime we are going to scan the thread list within a proc
we need to check for PRS_NEW.
We likely need to change the init scheme for the td_lock by having a
scheduler primitive setting it and doing that on thread_init() UMA
constructor, or similar approach.
But the thread state isn't valid anyway. 4BSD shouldn't be touching the
thread since it is in an incomplete / undefined state.
Yep, in this case I'd then want to just add the threads to proc once
they are fully initialized.
It is pointless (and dangerous) to replicate this check all over,
besides we want scheduler agnostic code, which means every iterations
of p_threads will need to check for a valid state of threads.
Yes, we do have to check for PRS_NEW in many places with the current approach,
but we need some way to reserve the PID to avoid duplicates and unless we
expand the scope of allproc in fork by a whole lot or stop using the allproc
list to track "pids in use", we will be stuck with some sort of "process
is still being built" sentry.
the pid used to be reserved in the pid hash
it was not put into the proc list until it was set up.
I know you don't believe me but that's how it was around 2000 I'm
pretty sure of it.
_______________________________________________
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
