From owner-freebsd-current@FreeBSD.ORG Tue Jun 17 13:06:57 2003 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 7638337B401; Tue, 17 Jun 2003 13:06:57 -0700 (PDT) Received: from gw.catspoiler.org (217-ip-163.nccn.net [209.79.217.163]) by mx1.FreeBSD.org (Postfix) with ESMTP id C1D9343FDD; Tue, 17 Jun 2003 13:06:56 -0700 (PDT) (envelope-from truckman@FreeBSD.org) Received: from FreeBSD.org (mousie.catspoiler.org [192.168.101.2]) by gw.catspoiler.org (8.12.9/8.12.9) with ESMTP id h5HK6lM7052632; Tue, 17 Jun 2003 13:06:51 -0700 (PDT) (envelope-from truckman@FreeBSD.org) Message-Id: <200306172006.h5HK6lM7052632@gw.catspoiler.org> Date: Tue, 17 Jun 2003 13:06:47 -0700 (PDT) From: Don Lewis To: alfred@FreeBSD.org In-Reply-To: <20030617190735.GU62025@elvis.mu.org> MIME-Version: 1.0 Content-Type: TEXT/plain; charset=us-ascii cc: current@FreeBSD.org Subject: Re: Giant pushdown in kern_descrip.c rev 1.128 X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 17 Jun 2003 20:06:57 -0000 On 17 Jun, Alfred Perlstein wrote: > * Don Lewis [030617 12:00] wrote: >> It's not legal to attempt to aquire Giant in fdrop_locked(), while >> FILE_LOCK() is held. The problem is that FILE_LOCK uses the mutex pool, >> which should only be used for leaf mutexes. >> >> It also looks like there is a potential for a lock order reversal if >> some callers aquire Giant before FILE_LOCK() and fdrop_locked() does the >> opposite. >> >> It also appears that witness ignores the mutex pool ... > > Yes, but I think the fix is as simple as just dropping the FILE_LOCK > after the decrement as we're the last holders of it, can you try > this: I like simple fixes, especially when the code shrinks ;-) Unfortunately, I think your point about this only happening because this process is the last holder of the file means that this doesn't explain Peter's deadlock. > Index: kern_descrip.c > =================================================================== > RCS file: /home/ncvs/src/sys/kern/kern_descrip.c,v > retrieving revision 1.199 > diff -u -r1.199 kern_descrip.c > --- kern_descrip.c 11 Jun 2003 00:56:55 -0000 1.199 > +++ kern_descrip.c 17 Jun 2003 19:07:01 -0000 > @@ -2003,6 +2003,7 @@ > FILE_UNLOCK(fp); > return (0); > } > + FILE_UNLOCK(fp); > mtx_lock(&Giant); > if (fp->f_count < 0) > panic("fdrop: count < 0"); > @@ -2012,10 +2013,8 @@ > lf.l_len = 0; > lf.l_type = F_UNLCK; > vp = fp->f_data; > - FILE_UNLOCK(fp); > (void) VOP_ADVLOCK(vp, (caddr_t)fp, F_UNLCK, &lf, F_FLOCK); > - } else > - FILE_UNLOCK(fp); > + } > if (fp->f_ops != &badfileops) > error = fo_close(fp, td); > else > >