On Sun, Apr 24, 2011 at 11:36:27AM +0900, YONETANI Tomokazu wrote: > > With regards to getting rid of the timeout in the tsleep and using a > > proactive wakeup(), we have to avoid calling wakeup() for 1->0 > > transitions unless someone is known to be waiting on p_lock. This > > can be implementing by adding a WAITING flag to the field and using > > atomic_cmpset_int() to handle the (WAITING | 1) -> (0) transition and > > then calling wakeup() if WAITING was set. > > > > I will augment the sys/refcount.h API and add refcount_wait() and > > refcount_release_wakeup() which encapsulate the appropriate atomic > > ops. I will leave it up to you if you want to then use the new API > > functions for PHOLD/PRELE, which would give the tsleep case a > > proactive wakeup() instead of having to wait for it to timeout. > > So what I need to do is to change PHOLD/PRELE to use refcount_acquire/ > refcount_release_wakeup and replace p->p_lock loop with > refcount_release_wakeup? I'll give it a try. I've been running the kernel with patch(es) attached to this message and so far it's running fine under load. It reduced the number of non-zero p->p_lock just before calling proc_remove_zombie() even without holding proc_token around the first wait loop. However I've observed a panic when a signal is sent to a process group, and I need to determine if it's jail related or GNU screen related.
Best Regards, YONETANI Tomokazu.
>From ad2d4ff7a9d66008facfa171dad075c4a0397f13 Mon Sep 17 00:00:00 2001 From: YONETANI Tomokazu <[email protected]> Date: Mon, 25 Apr 2011 00:45:19 +0900 Subject: [PATCH 1/2] Remove double semi-colon --- sys/kern/kern_exit.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sys/kern/kern_exit.c b/sys/kern/kern_exit.c index 1e5a110..dda3a3e 100644 --- a/sys/kern/kern_exit.c +++ b/sys/kern/kern_exit.c @@ -686,7 +686,7 @@ lwp_exit(int masterexit) static int lwp_wait(struct lwp *lp) { - struct thread *td = lp->lwp_thread;; + struct thread *td = lp->lwp_thread; KKASSERT(lwkt_preempted_proc() != lp); @@ -724,7 +724,7 @@ lwp_wait(struct lwp *lp) void lwp_dispose(struct lwp *lp) { - struct thread *td = lp->lwp_thread;; + struct thread *td = lp->lwp_thread; KKASSERT(lwkt_preempted_proc() != lp); KKASSERT(td->td_refs == 0); -- 1.7.3.2
>From e44e7e865c219fcf101040bfb2a9894d3b6ad633 Mon Sep 17 00:00:00 2001 From: YONETANI Tomokazu <[email protected]> Date: Mon, 25 Apr 2011 06:22:16 +0900 Subject: [PATCH 2/2] kernel: Replace {,LW}P{HOLD,RELE} to use refcount APIs --- sys/kern/kern_exit.c | 14 +++++++------- sys/kern/kern_proc.c | 8 ++------ sys/platform/pc32/i386/pmap.c | 3 ++- sys/platform/pc64/x86_64/pmap.c | 3 ++- sys/platform/vkernel/platform/pmap.c | 3 ++- sys/platform/vkernel64/platform/pmap.c | 3 ++- sys/sys/proc.h | 11 +++++++---- 7 files changed, 24 insertions(+), 21 deletions(-) diff --git a/sys/kern/kern_exit.c b/sys/kern/kern_exit.c index dda3a3e..0d35a37 100644 --- a/sys/kern/kern_exit.c +++ b/sys/kern/kern_exit.c @@ -619,8 +619,7 @@ lwp_exit(int masterexit) * Nobody actually wakes us when the lock * count reaches zero, so just wait one tick. */ - while (lp->lwp_lock > 0) - tsleep(lp, 0, "lwpexit", 1); + LWPLOCKWAIT(lp, "lwpexit", 1); /* Hand down resource usage to our proc */ ruadd(&p->p_ru, &lp->lwp_ru); @@ -690,8 +689,7 @@ lwp_wait(struct lwp *lp) KKASSERT(lwkt_preempted_proc() != lp); - while (lp->lwp_lock > 0) - tsleep(lp, 0, "lwpwait1", 1); + LWPLOCKWAIT(lp, "lwpwait1", 1); lwkt_wait_free(td); @@ -867,8 +865,7 @@ loop: * put a hold on the process for short periods of * time. */ - while (p->p_lock) - tsleep(p, 0, "reap3", hz); + PLOCKWAIT(p, "reap3", hz); /* Take care of our return values. */ *res = p->p_pid; @@ -898,7 +895,10 @@ loop: * inconsistent state for processes running down * the zombie list. */ - KKASSERT(p->p_lock == 0); + if (p->p_lock) { + kprintf("%s: pid %d is still held (%x)\n", + __func__, p->p_pid, p->p_lock); + } proc_remove_zombie(p); leavepgrp(p); diff --git a/sys/kern/kern_proc.c b/sys/kern/kern_proc.c index cb067f0..6d760e2 100644 --- a/sys/kern/kern_proc.c +++ b/sys/kern/kern_proc.c @@ -638,9 +638,7 @@ void proc_move_allproc_zombie(struct proc *p) { lwkt_gettoken(&proc_token); - while (p->p_lock) { - tsleep(p, 0, "reap1", hz / 10); - } + PLOCKWAIT(p, "reap1", hz / 10); LIST_REMOVE(p, p_list); LIST_INSERT_HEAD(&zombproc, p, p_list); LIST_REMOVE(p, p_hash); @@ -660,9 +658,7 @@ void proc_remove_zombie(struct proc *p) { lwkt_gettoken(&proc_token); - while (p->p_lock) { - tsleep(p, 0, "reap1", hz / 10); - } + PLOCKWAIT(p, "reap1", hz / 10); LIST_REMOVE(p, p_list); /* off zombproc */ LIST_REMOVE(p, p_sibling); lwkt_reltoken(&proc_token); diff --git a/sys/platform/pc32/i386/pmap.c b/sys/platform/pc32/i386/pmap.c index 25816c5..8936a01 100644 --- a/sys/platform/pc32/i386/pmap.c +++ b/sys/platform/pc32/i386/pmap.c @@ -1020,7 +1020,8 @@ pmap_init_proc(struct proc *p) void pmap_dispose_proc(struct proc *p) { - KASSERT(p->p_lock == 0, ("attempt to dispose referenced proc! %p", p)); + KASSERT(p->p_lock == 0, + ("attempt to dispose referenced or waited for proc! %p", p)); } /*************************************************** diff --git a/sys/platform/pc64/x86_64/pmap.c b/sys/platform/pc64/x86_64/pmap.c index 3ab87c9..b119773 100644 --- a/sys/platform/pc64/x86_64/pmap.c +++ b/sys/platform/pc64/x86_64/pmap.c @@ -1151,7 +1151,8 @@ pmap_init_proc(struct proc *p) void pmap_dispose_proc(struct proc *p) { - KASSERT(p->p_lock == 0, ("attempt to dispose referenced proc! %p", p)); + KASSERT(p->p_lock == 0, + ("attempt to dispose referenced or waited for proc! %p", p)); } /*************************************************** diff --git a/sys/platform/vkernel/platform/pmap.c b/sys/platform/vkernel/platform/pmap.c index 125fcdc..6c03106 100644 --- a/sys/platform/vkernel/platform/pmap.c +++ b/sys/platform/vkernel/platform/pmap.c @@ -888,7 +888,8 @@ pmap_init_proc(struct proc *p) void pmap_dispose_proc(struct proc *p) { - KASSERT(p->p_lock == 0, ("attempt to dispose referenced proc! %p", p)); + KASSERT(p->p_lock == 0, + ("attempt to dispose referenced or waited for proc! %p", p)); } /* diff --git a/sys/platform/vkernel64/platform/pmap.c b/sys/platform/vkernel64/platform/pmap.c index 246d36a..ed3f6a3 100644 --- a/sys/platform/vkernel64/platform/pmap.c +++ b/sys/platform/vkernel64/platform/pmap.c @@ -905,7 +905,8 @@ pmap_init_proc(struct proc *p) void pmap_dispose_proc(struct proc *p) { - KASSERT(p->p_lock == 0, ("attempt to dispose referenced proc! %p", p)); + KASSERT(p->p_lock == 0, + ("attempt to dispose referenced or waited for proc! %p", p)); } /*************************************************** diff --git a/sys/sys/proc.h b/sys/sys/proc.h index be6ad41..29b2568 100644 --- a/sys/sys/proc.h +++ b/sys/sys/proc.h @@ -56,6 +56,7 @@ #include <sys/rtprio.h> /* For struct rtprio. */ #include <sys/signal.h> #include <sys/lock.h> +#include <sys/refcount.h> #ifndef _KERNEL #include <sys/time.h> /* For structs itimerval, timeval. */ #endif @@ -444,16 +445,18 @@ extern void stopevent(struct proc*, unsigned int, unsigned int); * * MPSAFE */ -#define PHOLD(p) atomic_add_int(&(p)->p_lock, 1) -#define PRELE(p) atomic_add_int(&(p)->p_lock, -1) +#define PHOLD(p) refcount_acquire(&(p)->p_lock) +#define PRELE(p) refcount_release_wakeup(&(p)->p_lock) +#define PLOCKWAIT(p, wmesg, timo) refcount_wait(&(p)->p_lock, wmesg) /* * Hold lwp in memory, don't destruct, normally for ptrace/procfs work * atomic ops because they can occur from an IPI. * MPSAFE */ -#define LWPHOLD(lp) atomic_add_int(&(lp)->lwp_lock, 1) -#define LWPRELE(lp) atomic_add_int(&(lp)->lwp_lock, -1) +#define LWPHOLD(lp) refcount_acquire(&(lp)->lwp_lock) +#define LWPRELE(lp) refcount_release_wakeup(&(lp)->lwp_lock) +#define LWPLOCKWAIT(lp, wmesg, timo) refcount_wait(&(lp)->lwp_lock, wmesg) #define PIDHASH(pid) (&pidhashtbl[(pid) & pidhash]) extern LIST_HEAD(pidhashhead, proc) *pidhashtbl; -- 1.7.3.2
