d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Reduce contention on the proctree lock during heavy package build.

Browse Source 
There is a proctree -> allproc ordering established.

Most of the time it is either xlock -> xlock or slock -> slock.

On fork however there is a slock -> xlock pair which results in
pathological wait times due to threads keeping proctree held for
reading and all waiting on allproc. Switch this to xlock -> xlock.
Longer term fix would get rid of proctree in this place to begin with.
Right now it is necessary to walk the session/process group lists to
determine which id is free. The walk can be avoided e.g. with bitmaps.

The exit path used to have one place which dealt with allproc and
then with proctree. Move the allproc acquire into the section protected
by proctree. This reduces contention against threads waiting on proctree
in the fork codepath - the fork proctree holder does not have to wait
for allproc as often.

Finally, move tidhash manipulation outside of the area protected by
either of these locks. The removal from the hash was already unprotected.
There is no legitimate reason to look up thread ids for a process still
under construction.

This results in about 50% wait time reduction during -j 128 package build.
This commit is contained in:
mjg 2018-02-20 02:18:30 +00:00
parent 40d79c8768
commit 25e9d331dc
2 changed files with 17 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
sys/kern/kern_exit.c
View File

@ -423,6 +423,17 @@ exit1(struct thread *td, int rval, int signo)
tidhash_remove(td);
/*
* Call machine-dependent code to release any
* machine-dependent resources other than the address space.
* The address space is released by "vmspace_exitfree(p)" in
* vm_waitproc().
*/
cpu_exit(td);
WITNESS_WARN(WARN_PANIC, NULL, "process (pid %d) exiting", p->p_pid);
sx_xlock(&proctree_lock);
/*
* Remove proc from allproc queue and pidhash chain.
* Place onto zombproc. Unlink from parent's child list.
@ -433,22 +444,11 @@ exit1(struct thread *td, int rval, int signo)
LIST_REMOVE(p, p_hash);
sx_xunlock(&allproc_lock);
/*
* Call machine-dependent code to release any
* machine-dependent resources other than the address space.
* The address space is released by "vmspace_exitfree(p)" in
* vm_waitproc().
*/
cpu_exit(td);
WITNESS_WARN(WARN_PANIC, NULL, "process (pid %d) exiting", p->p_pid);
/*
* Reparent all children processes:
* - traced ones to the original parent (or init if we are that parent)
* - the rest to init
*/
sx_xlock(&proctree_lock);
q = LIST_FIRST(&p->p_children);
if (q != NULL) /* only need this if any child is S_ZOMB */
wakeup(q->p_reaper);

11
sys/kern/kern_fork.c
View File

@ -394,7 +394,7 @@ do_fork(struct thread *td, struct fork_req *fr, struct proc *p2, struct thread *
struct filedesc_to_leader *fdtol;
struct sigacts *newsigacts;
sx_assert(&proctree_lock, SX_SLOCKED);
sx_assert(&proctree_lock, SX_LOCKED);
sx_assert(&allproc_lock, SX_XLOCKED);
p1 = td->td_proc;
@ -407,12 +407,11 @@ do_fork(struct thread *td, struct fork_req *fr, struct proc *p2, struct thread *
LIST_INSERT_HEAD(&allproc, p2, p_list);
allproc_gen++;
LIST_INSERT_HEAD(PIDHASH(p2->p_pid), p2, p_hash);
tidhash_add(td2);
PROC_LOCK(p2);
PROC_LOCK(p1);
sx_xunlock(&allproc_lock);
sx_sunlock(&proctree_lock);
sx_xunlock(&proctree_lock);
bcopy(&p1->p_startcopy, &p2->p_startcopy,
__rangeof(struct proc, p_startcopy, p_endcopy));
@ -428,6 +427,8 @@ do_fork(struct thread *td, struct fork_req *fr, struct proc *p2, struct thread *
PROC_UNLOCK(p2);
tidhash_add(td2);
/*
* Malloc things while we don't hold any locks.
*/
@ -954,7 +955,7 @@ fork1(struct thread *td, struct fork_req *fr)
STAILQ_INIT(&newproc->p_ktr);
/* We have to lock the process tree while we look for a pid. */
sx_slock(&proctree_lock);
sx_xlock(&proctree_lock);
sx_xlock(&allproc_lock);
/*
@ -977,7 +978,7 @@ fork1(struct thread *td, struct fork_req *fr)
error = EAGAIN;
sx_xunlock(&allproc_lock);
sx_sunlock(&proctree_lock);
sx_xunlock(&proctree_lock);
#ifdef MAC
mac_proc_destroy(newproc);
#endif