NetBSD Problem Report #39420

From dholland@eecs.harvard.edu  Wed Aug 27 20:43:13 2008
Return-Path: <dholland@eecs.harvard.edu>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by narn.NetBSD.org (Postfix) with ESMTP id 5CD0B63BC30
	for <gnats-bugs@gnats.NetBSD.org>; Wed, 27 Aug 2008 20:43:13 +0000 (UTC)
Message-Id: <20080827203944.83857F8CE@tanaqui.eecs.harvard.edu>
Date: Wed, 27 Aug 2008 16:39:44 -0400 (EDT)
From: dholland@eecs.harvard.edu
Reply-To: dholland@eecs.harvard.edu
To: gnats-bugs@gnats.NetBSD.org
Subject: stopped processes can hold locks
X-Send-Pr-Version: 3.95

>Number:         39420
>Category:       kern
>Synopsis:       stopped processes can hold locks
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Aug 27 20:45:01 +0000 2008
>Last-Modified:  Sat Aug 15 00:40:14 +0000 2009
>Originator:     David A. Holland <dholland@eecs.harvard.edu>
>Release:        NetBSD 4.99.54 (20080229)
>Organization:
>Environment:
System: NetBSD tanaqui 4.99.54 NetBSD 4.99.54 (TANAQUI) #21: Fri Feb 29 12:31:31 EST 2008 dholland@tanaqui:/usr/src/sys/arch/i386/compile/TANAQUI i386
Architecture: i386
Machine: i386
>Description:

Today we had a network hiccup that caused an nfs server to become
unreachable. After the network cleared, my browser remained stuck, and
I found it and also a leftover sync process were stuck in tstile:

32170  3994   602 2109  85  0 62176 96908 tstile   Dl   ttyp2  188:29.95 /usr/p
  163 18096 26111  513  85  0    20   476 tstile   D    ttyE2    0:00.01 sync 

Digging around, I discovered a second leftover sync process, this one
stopped by job control. (I think what happened was that I'd
absentmindedly typed sync, it hung, and I hit the usual ^Z^Z^C^C kind
of thing to try to get rid of it; then when the network came back, the
^Z got processed and it stopped.) Resuming this sync process cleared
the other two processes out of tstile.

This means that those processes must have been waiting on some lock or
other that was released when the stopped sync was continued and ran to
completion.

This in turn means that the stopped process was holding a lock.

This should not be allowed; stopped processes tend to remain stopped
for long periods of time, and it doesn't do for the system to grind to
a halt waiting for them.

>How-To-Repeat:

Not likely.

>Fix:

I've hit this before (it used to be easy to trigger it with vnode
locks) and dug around some, and I think the way processes stop may
need to be completely rewritten...

>Release-Note:

>Audit-Trail:
From: yamt@mwd.biglobe.ne.jp (YAMAMOTO Takashi)
To: gnats-bugs@NetBSD.org
Cc: kern-bug-people@netbsd.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Subject: Re: kern/39420: stopped processes can hold locks
Date: Thu,  2 Oct 2008 08:12:35 +0900 (JST)

 > >Number:         39420
 > >Category:       kern
 > >Synopsis:       stopped processes can hold locks

 are you using interruptible nfs mount?
 in that case, nfs uses PCATCH/cv_wait_sig for I/O and can be
 suspended by user.
 ups@ suggested introducing something like PCATCH|PNOSUSPEND while ago.
 i think that an alternative is not suspending LWPs in the middle of
 sleepq_block.  ie. suspend LWPs only on user-kernel boundaries.

 YAMAMOTO Takashi

From: David Holland <dholland-bugs@netbsd.org>
To: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
Cc: gnats-bugs@NetBSD.org, kern-bug-people@netbsd.org,
	gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Subject: Re: kern/39420: stopped processes can hold locks
Date: Sat, 11 Oct 2008 23:11:48 +0000

 On Thu, Oct 02, 2008 at 08:12:35AM +0900, YAMAMOTO Takashi wrote:
  > > >Number:         39420
  > > >Category:       kern
  > > >Synopsis:       stopped processes can hold locks
  > 
  > are you using interruptible nfs mount?
  > in that case, nfs uses PCATCH/cv_wait_sig for I/O and can be
  > suspended by user.

 Yes.

  > ups@ suggested introducing something like PCATCH|PNOSUSPEND while ago.
  > i think that an alternative is not suspending LWPs in the middle of
  > sleepq_block.  ie. suspend LWPs only on user-kernel boundaries.

 I think that's a much better alternative. I don't think it's safe to
 suspend anything except at the user-kernel boundary. It's easy enough
 to use EINTR/ERESTARTSYS to get the LWPs there when necessary.

 When I looked into this before it appeared like it would require
 nontrivial changes, but I think I was probably missing something.

 -- 
 David A. Holland
 dholland@netbsd.org

>Unformatted:


 Here is an ugly patch that minimizes the line of diffs to implement PNOSTOP.
 Untested but compiles.

 christos

 Index: kern/kern_condvar.c
 ===================================================================
 RCS file: /cvsroot/src/sys/kern/kern_condvar.c,v
 retrieving revision 1.26
 diff -u -r1.26 kern_condvar.c
 --- kern/kern_condvar.c	19 Dec 2008 07:57:28 -0000	1.26
 +++ kern/kern_condvar.c	14 Aug 2009 22:48:50 -0000
 @@ -228,7 +228,7 @@
  *	call is restartable, or EINTR otherwise.
  */
 int
 -cv_wait_sig(kcondvar_t *cv, kmutex_t *mtx)
 +cv_wait_sig1(kcondvar_t *cv, kmutex_t *mtx, const sigset_t *set)
 {
 	lwp_t *l = curlwp;
 	int error;
 @@ -236,7 +236,7 @@
 	KASSERT(mutex_owned(mtx));

 	cv_enter(cv, mtx, l);
 -	error = sleepq_block(0, true);
 +	error = sleepq_block1(0, true, set);
 	return cv_exit(cv, mtx, l, error);
 }

 @@ -270,7 +270,7 @@
  *	call is restartable, or EINTR otherwise.
  */
 int
 -cv_timedwait_sig(kcondvar_t *cv, kmutex_t *mtx, int timo)
 +cv_timedwait_sig1(kcondvar_t *cv, kmutex_t *mtx, int timo, const sigset_t *set)
 {
 	lwp_t *l = curlwp;
 	int error;
 @@ -278,7 +278,7 @@
 	KASSERT(mutex_owned(mtx));

 	cv_enter(cv, mtx, l);
 -	error = sleepq_block(timo, true);
 +	error = sleepq_block1(timo, true, set);
 	return cv_exit(cv, mtx, l, error);
 }

 Index: kern/kern_sig.c
 ===================================================================
 RCS file: /cvsroot/src/sys/kern/kern_sig.c,v
 retrieving revision 1.298
 diff -u -r1.298 kern_sig.c
 --- kern/kern_sig.c	24 May 2009 21:41:26 -0000	1.298
 +++ kern/kern_sig.c	14 Aug 2009 22:48:51 -0000
 @@ -640,7 +640,7 @@
  *	set.
  */ 
 int
 -sigispending(struct lwp *l, int signo)
 +sigispending1(struct lwp *l, int signo, const sigset_t *set)
 {
 	struct proc *p = l->l_proc;
 	sigset_t tset;
 @@ -652,6 +652,9 @@
 	sigminusset(&p->p_sigctx.ps_sigignore, &tset);
 	sigminusset(&l->l_sigmask, &tset);

 +	if (set)
 +		sigminusset(set, &tset);
 +
 	if (signo == 0) {
 		if (firstsig(&tset) != 0)
 			return EINTR;
 Index: kern/kern_sleepq.c
 ===================================================================
 RCS file: /cvsroot/src/sys/kern/kern_sleepq.c,v
 retrieving revision 1.36
 diff -u -r1.36 kern_sleepq.c
 --- kern/kern_sleepq.c	21 Mar 2009 13:11:14 -0000	1.36
 +++ kern/kern_sleepq.c	14 Aug 2009 22:48:51 -0000
 @@ -230,7 +230,7 @@
  * 	example if the LWP's containing process is exiting.
  */
 int
 -sleepq_block(int timo, bool catch)
 +sleepq_block1(int timo, bool catch, const sigset_t *set)
 {
 	int error = 0, sig;
 	struct proc *p;
 @@ -250,7 +250,8 @@
 			l->l_flag &= ~LW_CANCELLED;
 			error = EINTR;
 			early = true;
 -		} else if ((l->l_flag & LW_PENDSIG) != 0 && sigispending(l, 0))
 +		} else if ((l->l_flag & LW_PENDSIG) != 0 && sigispending1(l, 0,
 +		    set))
 			early = true;
 	}

 Index: kern/vfs_bio.c
 ===================================================================
 RCS file: /cvsroot/src/sys/kern/vfs_bio.c,v
 retrieving revision 1.218
 diff -u -r1.218 vfs_bio.c
 --- kern/vfs_bio.c	11 Mar 2009 05:55:22 -0000	1.218
 +++ kern/vfs_bio.c	14 Aug 2009 22:48:52 -0000
 @@ -1359,8 +1359,9 @@
 		if (!from_bufq || curlwp != uvm.pagedaemon_lwp) {
 			/* wait for a free buffer of any kind */
 			if ((slpflag & PCATCH) != 0)
 -				(void)cv_timedwait_sig(&needbuffer_cv,
 -				    &bufcache_lock, slptimeo);
 +				(void)cv_timedwait_sig1(&needbuffer_cv,
 +				    &bufcache_lock, slptimeo, 
 +				    (slpflag & PNOSTOP) ? &stopsigmask : NULL);
 			else
 				(void)cv_timedwait(&needbuffer_cv,
 				    &bufcache_lock, slptimeo);
 Index: sys/condvar.h
 ===================================================================
 RCS file: /cvsroot/src/sys/sys/condvar.h,v
 retrieving revision 1.11
 diff -u -r1.11 condvar.h
 --- sys/condvar.h	19 Dec 2008 07:57:28 -0000	1.11
 +++ sys/condvar.h	14 Aug 2009 22:48:52 -0000
 @@ -33,6 +33,7 @@
 #define	_SYS_CONDVAR_H_

 #include <sys/mutex.h>
 +#include <sys/signal.h>

 typedef struct kcondvar {
 	void		*cv_opaque[3];
 @@ -44,9 +45,12 @@
 void	cv_destroy(kcondvar_t *);

 void	cv_wait(kcondvar_t *, kmutex_t *);
 -int	cv_wait_sig(kcondvar_t *, kmutex_t *);
 +int	cv_wait_sig1(kcondvar_t *, kmutex_t *, const sigset_t *);
 +#define cv_wait_sig(_cv, _km) cv_wait_sig1((_cv), (_km), NULL)
 int	cv_timedwait(kcondvar_t *, kmutex_t *, int);
 -int	cv_timedwait_sig(kcondvar_t *, kmutex_t *, int);
 +int	cv_timedwait_sig1(kcondvar_t *, kmutex_t *, int, const sigset_t *);
 +#define cv_timedwait_sig(_cv, _km, _t) \
 +    cv_timedwait_sig1((_cv), (_km), (_t), NULL)

 void	cv_signal(kcondvar_t *);
 void	cv_broadcast(kcondvar_t *);
 Index: sys/param.h
 ===================================================================
 RCS file: /cvsroot/src/sys/sys/param.h,v
 retrieving revision 1.350
 diff -u -r1.350 param.h
 --- sys/param.h	29 Jun 2009 05:47:35 -0000	1.350
 +++ sys/param.h	14 Aug 2009 22:48:52 -0000
 @@ -239,6 +239,8 @@

 #define	PCATCH		0x100	/* OR'd with pri for tsleep to check signals */
 #define	PNORELOCK	0x200	/* OR'd with pri for tsleep to not relock */
 +#define PNOSTOP		0x400	/* OR'd with pri for tsleep to not check stop
 +				 * signals */

 /*
  * New priority levels.
 Index: sys/signalvar.h
 ===================================================================
 RCS file: /cvsroot/src/sys/sys/signalvar.h,v
 retrieving revision 1.75
 diff -u -r1.75 signalvar.h
 --- sys/signalvar.h	11 Jan 2009 02:45:55 -0000	1.75
 +++ sys/signalvar.h	14 Aug 2009 22:48:52 -0000
 @@ -183,7 +183,8 @@
     register_t *, copyout_t, copyin_t, copyout_t);

 void	signotify(struct lwp *);
 -int	sigispending(struct lwp *, int);
 +int	sigispending1(struct lwp *, int, const sigset_t *);
 +#define	sigispending(_lw, _si) sigispending1((_lw), (_si), NULL)

 /*
  * Machine-dependent functions:
 Index: sys/sleepq.h
 ===================================================================
 RCS file: /cvsroot/src/sys/sys/sleepq.h,v
 retrieving revision 1.16
 diff -u -r1.16 sleepq.h
 --- sys/sleepq.h	21 Mar 2009 13:11:14 -0000	1.16
 +++ sys/sleepq.h	14 Aug 2009 22:48:52 -0000
 @@ -38,6 +38,7 @@
 #include <sys/queue.h>
 #include <sys/sched.h>
 #include <sys/syncobj.h>
 +#include <sys/signal.h>

 /*
  * Generic sleep queues.
 @@ -68,7 +69,8 @@
 int	sleepq_abort(kmutex_t *, int);
 void	sleepq_changepri(lwp_t *, pri_t);
 void	sleepq_lendpri(lwp_t *, pri_t);
 -int	sleepq_block(int, bool);
 +int	sleepq_block1(int, bool, const sigset_t *);
 +#define	sleepq_block(_t, _c) sleepq_block1((_t), (_c), NULL)
 void	sleepq_insert(sleepq_t *, lwp_t *, syncobj_t *);

 void	sleeptab_init(sleeptab_t *);
 Index: nfs/nfs_bio.c
 ===================================================================
 RCS file: /cvsroot/src/sys/nfs/nfs_bio.c,v
 retrieving revision 1.183
 diff -u -r1.183 nfs_bio.c
 --- nfs/nfs_bio.c	14 Mar 2009 14:46:11 -0000	1.183
 +++ nfs/nfs_bio.c	14 Aug 2009 22:48:52 -0000
 @@ -592,7 +592,7 @@
 	struct nfsmount *nmp = VFSTONFS(vp->v_mount);

 	if (nmp->nm_flag & NFSMNT_INT) {
 -		bp = getblk(vp, bn, size, PCATCH, 0);
 +		bp = getblk(vp, bn, size, PCATCH|PNOSTOP, 0);
 		while (bp == NULL) {
 			if (nfs_sigintr(nmp, NULL, l))
 				return (NULL);

NetBSD Home
NetBSD PR Database Search

(Contact us) $NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2007 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.