--W/nzBZO5zC0uMSeA
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
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.
--W/nzBZO5zC0uMSeA
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="0001-Remove-double-semi-colon.patch"
From ad2d4ff7a9d66008facfa171dad075c4a0397f13 Mon Sep 17 00:00:00 2001
From: YONETANI Tomokazu <y0netan1@dragonflybsd.org>
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
--W/nzBZO5zC0uMSeA
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="0002-kernel-Replace-LW-P-HOLD-RELE-to-use-refcount-APIs.patch"
From e44e7e865c219fcf101040bfb2a9894d3b6ad633 Mon Sep 17 00:00:00 2001
From: YONETANI Tomokazu <y0netan1@dragonflybsd.org>
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
--W/nzBZO5zC0uMSeA--