Date: Fri, 7 Dec 2018 12:22:32 +0000 (UTC) From: Mateusz Guzik <mjg@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r341684 - in head/sys: kern sys Message-ID: <201812071222.wB7CMWh3048198@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: mjg Date: Fri Dec 7 12:22:32 2018 New Revision: 341684 URL: https://svnweb.freebsd.org/changeset/base/341684 Log: Manage process-related IDs with bitmaps Currently unique pid allocation on fork often requires a full walk of process, group, session lists to make sure it is not used by anything. This has a side effect of requiring proctree to be held along with allproc, which adds more contention in poudriere -j 128. The patch below implements trivial bitmaps which gets rid of the problem. Dedicated lock is introduced to manage IDs. While here a bug was discovered: all processes would inherit reap id from the first process spawned by init. This had a side effect of keeping the ID used and when allocation rolls over to the beginning it keeps being skipped. The patch is loosely based on initial work by mjoras@. Reviewed by: kib Sponsored by: The FreeBSD Foundation Modified: head/sys/kern/kern_exit.c head/sys/kern/kern_fork.c head/sys/kern/kern_proc.c head/sys/sys/proc.h Modified: head/sys/kern/kern_exit.c ============================================================================== --- head/sys/kern/kern_exit.c Fri Dec 7 12:06:03 2018 (r341683) +++ head/sys/kern/kern_exit.c Fri Dec 7 12:22:32 2018 (r341684) @@ -148,6 +148,27 @@ reaper_abandon_children(struct proc *p, bool exiting) } static void +reaper_clear(struct proc *p) +{ + struct proc *p1; + bool clear; + + sx_assert(&proctree_lock, SX_LOCKED); + LIST_REMOVE(p, p_reapsibling); + if (p->p_reapsubtree == 1) + return; + clear = true; + LIST_FOREACH(p1, &p->p_reaper->p_reaplist, p_reapsibling) { + if (p1->p_reapsubtree == p->p_reapsubtree) { + clear = false; + break; + } + } + if (clear) + proc_id_clear(PROC_ID_REAP, p->p_reapsubtree); +} + +static void clear_orphan(struct proc *p) { struct proc *p1; @@ -881,7 +902,8 @@ proc_reap(struct thread *td, struct proc *p, int *stat sx_xunlock(PIDHASHLOCK(p->p_pid)); LIST_REMOVE(p, p_sibling); reaper_abandon_children(p, true); - LIST_REMOVE(p, p_reapsibling); + reaper_clear(p); + proc_id_clear(PROC_ID_PID, p->p_pid); PROC_LOCK(p); clear_orphan(p); PROC_UNLOCK(p); Modified: head/sys/kern/kern_fork.c ============================================================================== --- head/sys/kern/kern_fork.c Fri Dec 7 12:06:03 2018 (r341683) +++ head/sys/kern/kern_fork.c Fri Dec 7 12:22:32 2018 (r341684) @@ -44,6 +44,7 @@ __FBSDID("$FreeBSD$"); #include <sys/param.h> #include <sys/systm.h> +#include <sys/bitstring.h> #include <sys/sysproto.h> #include <sys/eventhandler.h> #include <sys/fcntl.h> @@ -232,22 +233,18 @@ sysctl_kern_randompid(SYSCTL_HANDLER_ARGS) SYSCTL_PROC(_kern, OID_AUTO, randompid, CTLTYPE_INT|CTLFLAG_RW, 0, 0, sysctl_kern_randompid, "I", "Random PID modulus. Special values: 0: disable, 1: choose random value"); +extern bitstr_t proc_id_pidmap; +extern bitstr_t proc_id_grpidmap; +extern bitstr_t proc_id_sessidmap; +extern bitstr_t proc_id_reapmap; + static int fork_findpid(int flags) { - struct proc *p; + pid_t result; int trypid; - static int pidchecked = 0; - bool locked_zomb = false; /* - * Requires allproc_lock in order to iterate over the list - * of processes, and proctree_lock to access p_pgrp. - */ - sx_assert(&allproc_lock, SX_LOCKED); - sx_assert(&proctree_lock, SX_LOCKED); - - /* * Find an unused process ID. We remember a range of unused IDs * ready to use (from lastpid+1 through pidchecked-1). * @@ -262,6 +259,7 @@ fork_findpid(int flags) if (randompid) trypid += arc4random() % randompid; } + mtx_lock(&procid_lock); retry: /* * If the process ID prototype has wrapped around, @@ -272,74 +270,28 @@ retry: trypid = trypid % pid_max; if (trypid < 100) trypid += 100; - pidchecked = 0; } - if (trypid >= pidchecked) { - int doingzomb = 0; - pidchecked = PID_MAX; - /* - * Scan the active and zombie procs to check whether this pid - * is in use. Remember the lowest pid that's greater - * than trypid, so we can avoid checking for a while. - * - * Avoid reuse of the process group id, session id or - * the reaper subtree id. Note that for process group - * and sessions, the amount of reserved pids is - * limited by process limit. For the subtree ids, the - * id is kept reserved only while there is a - * non-reaped process in the subtree, so amount of - * reserved pids is limited by process limit times - * two. - */ - p = LIST_FIRST(&allproc); -again: - for (; p != NULL; p = LIST_NEXT(p, p_list)) { - while (p->p_pid == trypid || - p->p_reapsubtree == trypid || - (p->p_pgrp != NULL && - (p->p_pgrp->pg_id == trypid || - (p->p_session != NULL && - p->p_session->s_sid == trypid)))) { - trypid++; - if (trypid >= pidchecked) - goto retry; - } - if (p->p_pid > trypid && pidchecked > p->p_pid) - pidchecked = p->p_pid; - if (p->p_pgrp != NULL) { - if (p->p_pgrp->pg_id > trypid && - pidchecked > p->p_pgrp->pg_id) - pidchecked = p->p_pgrp->pg_id; - if (p->p_session != NULL && - p->p_session->s_sid > trypid && - pidchecked > p->p_session->s_sid) - pidchecked = p->p_session->s_sid; - } - } - if (!doingzomb) { - doingzomb = 1; - if (!locked_zomb) { - sx_slock(&zombproc_lock); - locked_zomb = true; - } - p = LIST_FIRST(&zombproc); - goto again; - } + bit_ffc_at(&proc_id_pidmap, trypid, pid_max, &result); + if (result == -1) + goto retry; + if (bit_test(&proc_id_grpidmap, result) || + bit_test(&proc_id_sessidmap, result) || + bit_test(&proc_id_reapmap, result)) { + trypid++; + goto retry; } /* * RFHIGHPID does not mess with the lastpid counter during boot. */ - if (flags & RFHIGHPID) - pidchecked = 0; - else - lastpid = trypid; + if ((flags & RFHIGHPID) == 0) + lastpid = result; - if (locked_zomb) - sx_sunlock(&zombproc_lock); + bit_set(&proc_id_pidmap, result); + mtx_unlock(&procid_lock); - return (trypid); + return (result); } static int @@ -402,13 +354,11 @@ do_fork(struct thread *td, struct fork_req *fr, struct struct filedesc_to_leader *fdtol; struct sigacts *newsigacts; - sx_assert(&proctree_lock, SX_LOCKED); sx_assert(&allproc_lock, SX_XLOCKED); p1 = td->td_proc; trypid = fork_findpid(fr->fr_flags); - p2->p_state = PRS_NEW; /* protect against others */ p2->p_pid = trypid; AUDIT_ARG_PID(p2->p_pid); @@ -421,7 +371,6 @@ do_fork(struct thread *td, struct fork_req *fr, struct PROC_LOCK(p1); sx_xunlock(&allproc_lock); - sx_xunlock(&proctree_lock); bcopy(&p1->p_startcopy, &p2->p_startcopy, __rangeof(struct proc, p_startcopy, p_endcopy)); @@ -657,8 +606,10 @@ do_fork(struct thread *td, struct fork_req *fr, struct LIST_INSERT_HEAD(&pptr->p_children, p2, p_sibling); LIST_INIT(&p2->p_reaplist); LIST_INSERT_HEAD(&p2->p_reaper->p_reaplist, p2, p_reapsibling); - if (p2->p_reaper == p1) + if (p2->p_reaper == p1 && p1 != initproc) { p2->p_reapsubtree = p2->p_pid; + proc_id_set_cond(PROC_ID_REAP, p2->p_pid); + } sx_xunlock(&proctree_lock); /* Inform accounting that we have forked. */ @@ -975,8 +926,6 @@ fork1(struct thread *td, struct fork_req *fr) newproc->p_klist = knlist_alloc(&newproc->p_mtx); STAILQ_INIT(&newproc->p_ktr); - /* We have to lock the process tree while we look for a pid. */ - sx_xlock(&proctree_lock); sx_xlock(&allproc_lock); /* @@ -999,7 +948,6 @@ fork1(struct thread *td, struct fork_req *fr) error = EAGAIN; sx_xunlock(&allproc_lock); - sx_xunlock(&proctree_lock); #ifdef MAC mac_proc_destroy(newproc); #endif Modified: head/sys/kern/kern_proc.c ============================================================================== --- head/sys/kern/kern_proc.c Fri Dec 7 12:06:03 2018 (r341683) +++ head/sys/kern/kern_proc.c Fri Dec 7 12:22:32 2018 (r341684) @@ -41,6 +41,7 @@ __FBSDID("$FreeBSD$"); #include <sys/param.h> #include <sys/systm.h> +#include <sys/bitstring.h> #include <sys/elf.h> #include <sys/eventhandler.h> #include <sys/exec.h> @@ -128,6 +129,7 @@ struct sx __exclusive_cache_line allproc_lock; struct sx __exclusive_cache_line zombproc_lock; struct sx __exclusive_cache_line proctree_lock; struct mtx __exclusive_cache_line ppeers_lock; +struct mtx __exclusive_cache_line procid_lock; uma_zone_t proc_zone; /* @@ -181,6 +183,7 @@ procinit(void) sx_init(&zombproc_lock, "zombproc"); sx_init(&proctree_lock, "proctree"); mtx_init(&ppeers_lock, "p_peers", NULL, MTX_DEF); + mtx_init(&procid_lock, "procid", NULL, MTX_DEF); LIST_INIT(&allproc); LIST_INIT(&zombproc); pidhashtbl = hashinit(maxproc / 4, M_PROC, &pidhash); @@ -293,6 +296,62 @@ proc_fini(void *mem, int size) } /* + * PID space management. + * + * These bitmaps are used by fork_findpid. + */ +bitstr_t bit_decl(proc_id_pidmap, PID_MAX); +bitstr_t bit_decl(proc_id_grpidmap, PID_MAX); +bitstr_t bit_decl(proc_id_sessidmap, PID_MAX); +bitstr_t bit_decl(proc_id_reapmap, PID_MAX); + +static bitstr_t *proc_id_array[] = { + proc_id_pidmap, + proc_id_grpidmap, + proc_id_sessidmap, + proc_id_reapmap, +}; + +void +proc_id_set(int type, pid_t id) +{ + + KASSERT(type >= 0 && type < nitems(proc_id_array), + ("invalid type %d\n", type)); + mtx_lock(&procid_lock); + KASSERT(bit_test(proc_id_array[type], id) == 0, + ("bit %d already set in %d\n", id, type)); + bit_set(proc_id_array[type], id); + mtx_unlock(&procid_lock); +} + +void +proc_id_set_cond(int type, pid_t id) +{ + + KASSERT(type >= 0 && type < nitems(proc_id_array), + ("invalid type %d\n", type)); + if (bit_test(proc_id_array[type], id)) + return; + mtx_lock(&procid_lock); + bit_set(proc_id_array[type], id); + mtx_unlock(&procid_lock); +} + +void +proc_id_clear(int type, pid_t id) +{ + + KASSERT(type >= 0 && type < nitems(proc_id_array), + ("invalid type %d\n", type)); + mtx_lock(&procid_lock); + KASSERT(bit_test(proc_id_array[type], id) != 0, + ("bit %d not set in %d\n", id, type)); + bit_clear(proc_id_array[type], id); + mtx_unlock(&procid_lock); +} + +/* * Is p an inferior of the current process? */ int @@ -495,6 +554,7 @@ enterpgrp(struct proc *p, pid_t pgid, struct pgrp *pgr PGRP_LOCK(pgrp); sess->s_leader = p; sess->s_sid = p->p_pid; + proc_id_set(PROC_ID_SESSION, p->p_pid); refcount_init(&sess->s_count, 1); sess->s_ttyvp = NULL; sess->s_ttydp = NULL; @@ -510,6 +570,7 @@ enterpgrp(struct proc *p, pid_t pgid, struct pgrp *pgr PGRP_LOCK(pgrp); } pgrp->pg_id = pgid; + proc_id_set(PROC_ID_GROUP, p->p_pid); LIST_INIT(&pgrp->pg_members); /* @@ -640,6 +701,7 @@ pgdelete(struct pgrp *pgrp) tty_rel_pgrp(tp, pgrp); } + proc_id_clear(PROC_ID_GROUP, pgrp->pg_id); mtx_destroy(&pgrp->pg_mtx); free(pgrp, M_PGRP); sess_release(savesess); @@ -824,6 +886,7 @@ sess_release(struct session *s) tty_lock(s->s_ttyp); tty_rel_sess(s->s_ttyp, s); } + proc_id_clear(PROC_ID_SESSION, s->s_sid); mtx_destroy(&s->s_mtx); free(s, M_SESSION); } Modified: head/sys/sys/proc.h ============================================================================== --- head/sys/sys/proc.h Fri Dec 7 12:06:03 2018 (r341683) +++ head/sys/sys/proc.h Fri Dec 7 12:22:32 2018 (r341684) @@ -962,6 +962,7 @@ extern int allproc_gen; extern struct sx zombproc_lock; extern struct sx proctree_lock; extern struct mtx ppeers_lock; +extern struct mtx procid_lock; extern struct proc proc0; /* Process slot for swapper. */ extern struct thread0_storage thread0_st; /* Primary thread in proc0. */ #define thread0 (thread0_st.t0st_thread) @@ -1164,6 +1165,15 @@ td_softdep_cleanup(struct thread *td) if (td->td_su != NULL && softdep_ast_cleanup != NULL) softdep_ast_cleanup(td); } + +#define PROC_ID_PID 0 +#define PROC_ID_GROUP 1 +#define PROC_ID_SESSION 2 +#define PROC_ID_REAP 3 + +void proc_id_set(int type, pid_t id); +void proc_id_set_cond(int type, pid_t id); +void proc_id_clear(int type, pid_t id); #endif /* _KERNEL */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201812071222.wB7CMWh3048198>