From owner-freebsd-current  Thu Mar  4 23: 8:30 1999
Delivered-To: freebsd-current@freebsd.org
Received: from apollo.backplane.com (apollo.backplane.com [209.157.86.2])
	by hub.freebsd.org (Postfix) with ESMTP id 8D7B2150F3
	for <current@FreeBSD.ORG>; Thu,  4 Mar 1999 23:06:50 -0800 (PST)
	(envelope-from dillon@apollo.backplane.com)
Received: (from dillon@localhost)
	by apollo.backplane.com (8.9.3/8.9.1) id XAA38141;
	Thu, 4 Mar 1999 23:06:28 -0800 (PST)
	(envelope-from dillon)
Date: Thu, 4 Mar 1999 23:06:28 -0800 (PST)
From: Matthew Dillon <dillon@apollo.backplane.com>
Message-Id: <199903050706.XAA38141@apollo.backplane.com>
To: Peter Jeremy <peter.jeremy@auss2.alcatel.com.au>
Cc: current@FreeBSD.ORG
Subject: Re: Simple DOS against 3.x locks box solid
Sender: owner-freebsd-current@FreeBSD.ORG
Precedence: bulk
X-Loop: FreeBSD.ORG

:Hi Matt,
:
:You wrote:
:>    We really need to fix the kernel, I think.  This sort of deadlock
:>    situation has been a problem for too long and it is only going to
:>    get worse as more programs use mmap().
:
:I agree.  Since you suggest that the kernel fix is going to be a
:while, I was suggesting an interim fix which would make processes
:die (unnecessarily) rather than deadlock the system.
:
:Peter

    I don't think killing processes is a good idea.  I have an interim 
    solution that will work for the situation that grep creates.

    The precise deadlock situation that grep creates is actually due
    to a pending exclusive lock blocking additional shared locks from
    occuring on a vnode that is already shared-locked.   Grep does a
    combination mmap()/read().  The deadlock was occuring due to an
    open() waiting to get an exclusive lock and preventing a vm_fault()
    within a read() from being able to obtain a second shared lock
    reference.

    vm_fault() always obtain shared locks.
    read() syscalls obtain shared locks.
    write() syscalls obtain exclusiuve locks.
    open() syscalls obtain exclusive locks.

    There are still deadlock situations possible, especially in combinations
    that use write(), but this tentitive patch should get many of the more 
    common deadlocks.

    *** THIS IS EXPERIMENTAL ***
    *** DO NOT APPLY UNLESS YOU HAVE THIS EXPLICIT PROBLEM UNDER NORMAL
	OPERATING CONDITIONS ***

					-Matt
					Matthew Dillon 
					<dillon@backplane.com>

Add to sys/proc.h:

#define P_DEADLKTREAT   0x800000 /* lock aquisition - deadlock treatment */

Then patch:

Index: kern/kern_lock.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/kern_lock.c,v
retrieving revision 1.23
diff -u -r1.23 kern_lock.c
--- kern_lock.c	1999/01/20 14:49:11	1.23
+++ kern_lock.c	1999/03/05 03:39:51
@@ -205,9 +205,30 @@
 	switch (flags & LK_TYPE_MASK) {
 
 	case LK_SHARED:
+		/*
+		 * If we are not the exclusive lock holder, we have to block
+		 * while there is an exclusive lock holder or while an
+		 * exclusive lock request or upgrade request is in progress.
+		 *
+		 * However, if P_DEADLKTREAT is set, we override exclusive
+		 * lock requests or upgrade requests ( but not the exclusive
+		 * lock itself ).
+		 */
 		if (lkp->lk_lockholder != pid) {
-			error = acquire(lkp, extflags,
-				LK_HAVE_EXCL | LK_WANT_EXCL | LK_WANT_UPGRADE);
+			if (p->p_flag & P_DEADLKTREAT) {
+				error = acquire(
+					    lkp,
+					    extflags,
+					    LK_HAVE_EXCL
+					);
+			} else {
+				error = acquire(
+					    lkp, 
+					    extflags,
+					    LK_HAVE_EXCL | LK_WANT_EXCL | 
+					     LK_WANT_UPGRADE
+					);
+			}
 			if (error)
 				break;
 			sharelock(lkp, 1);
Index: kern/kern_subr.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/kern_subr.c,v
retrieving revision 1.27
diff -u -r1.27 kern_subr.c
--- kern_subr.c	1999/02/22 18:39:49	1.27
+++ kern_subr.c	1999/03/05 03:39:52
@@ -63,13 +63,19 @@
 {
 	register struct iovec *iov;
 	u_int cnt;
-	int error;
+	int error = 0;
+	int save = 0;
 
 	KASSERT(uio->uio_rw == UIO_READ || uio->uio_rw == UIO_WRITE,
 	    ("uiomove: mode"));
 	KASSERT(uio->uio_segflg != UIO_USERSPACE || uio->uio_procp == curproc,
 	    ("uiomove proc"));
 
+	if (curproc) {
+		save = curproc->p_flag & P_DEADLKTREAT;
+		curproc->p_flag |= P_DEADLKTREAT;
+	}
+
 	while (n > 0 && uio->uio_resid) {
 		iov = uio->uio_iov;
 		cnt = iov->iov_len;
@@ -92,7 +98,7 @@
 			else
 				error = copyin(iov->iov_base, cp, cnt);
 			if (error)
-				return (error);
+				break;
 			break;
 
 		case UIO_SYSSPACE:
@@ -111,7 +117,9 @@
 		cp += cnt;
 		n -= cnt;
 	}
-	return (0);
+	if (curproc)
+		curproc->p_flag = (curproc->p_flag & ~P_DEADLKTREAT) | save;
+	return (error);
 }
 
 int


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message