From owner-svn-src-head@freebsd.org Tue Feb 20 02:18:31 2018 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 2ED3FF19365; Tue, 20 Feb 2018 02:18:31 +0000 (UTC) (envelope-from mjg@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id D595A79FD2; Tue, 20 Feb 2018 02:18:30 +0000 (UTC) (envelope-from mjg@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id D07DD22F17; Tue, 20 Feb 2018 02:18:30 +0000 (UTC) (envelope-from mjg@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id w1K2IU7m098043; Tue, 20 Feb 2018 02:18:30 GMT (envelope-from mjg@FreeBSD.org) Received: (from mjg@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id w1K2IU4r098041; Tue, 20 Feb 2018 02:18:30 GMT (envelope-from mjg@FreeBSD.org) Message-Id: <201802200218.w1K2IU4r098041@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: mjg set sender to mjg@FreeBSD.org using -f From: Mateusz Guzik Date: Tue, 20 Feb 2018 02:18:30 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r329615 - head/sys/kern X-SVN-Group: head X-SVN-Commit-Author: mjg X-SVN-Commit-Paths: head/sys/kern X-SVN-Commit-Revision: 329615 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 20 Feb 2018 02:18:31 -0000 Author: mjg Date: Tue Feb 20 02:18:30 2018 New Revision: 329615 URL: https://svnweb.freebsd.org/changeset/base/329615 Log: Reduce contention on the proctree lock during heavy package build. 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. Modified: head/sys/kern/kern_exit.c head/sys/kern/kern_fork.c Modified: head/sys/kern/kern_exit.c ============================================================================== --- head/sys/kern/kern_exit.c Tue Feb 20 02:03:29 2018 (r329614) +++ head/sys/kern/kern_exit.c Tue Feb 20 02:18:30 2018 (r329615) @@ -424,16 +424,6 @@ exit1(struct thread *td, int rval, int signo) tidhash_remove(td); /* - * Remove proc from allproc queue and pidhash chain. - * Place onto zombproc. Unlink from parent's child list. - */ - sx_xlock(&allproc_lock); - LIST_REMOVE(p, p_list); - LIST_INSERT_HEAD(&zombproc, p, p_list); - 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 @@ -443,12 +433,22 @@ exit1(struct thread *td, int rval, int signo) 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. + */ + sx_xlock(&allproc_lock); + LIST_REMOVE(p, p_list); + LIST_INSERT_HEAD(&zombproc, p, p_list); + LIST_REMOVE(p, p_hash); + sx_xunlock(&allproc_lock); + + /* * 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); Modified: head/sys/kern/kern_fork.c ============================================================================== --- head/sys/kern/kern_fork.c Tue Feb 20 02:03:29 2018 (r329614) +++ head/sys/kern/kern_fork.c Tue Feb 20 02:18:30 2018 (r329615) @@ -394,7 +394,7 @@ do_fork(struct thread *td, struct fork_req *fr, struct 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 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_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