From owner-freebsd-current@FreeBSD.ORG  Tue Jun 17 20:09:00 2003
Return-Path: <owner-freebsd-current@FreeBSD.ORG>
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 51C5037B401; Tue, 17 Jun 2003 20:09:00 -0700 (PDT)
Received: from gw.catspoiler.org (217-ip-163.nccn.net [209.79.217.163])
	by mx1.FreeBSD.org (Postfix) with ESMTP
	id 9FE1743FA3; Tue, 17 Jun 2003 20:08:59 -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 h5I38oM7053457;
	Tue, 17 Jun 2003 20:08:54 -0700 (PDT)
	(envelope-from truckman@FreeBSD.org)
Message-Id: <200306180308.h5I38oM7053457@gw.catspoiler.org>
Date: Tue, 17 Jun 2003 20:08:50 -0700 (PDT)
From: Don Lewis <truckman@FreeBSD.org>
To: alfred@FreeBSD.org
In-Reply-To: <20030617220231.GV62025@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
	<freebsd-current.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-current>,
	<mailto:freebsd-current-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-current>
List-Post: <mailto:freebsd-current@freebsd.org>
List-Help: <mailto:freebsd-current-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-current>,
	<mailto:freebsd-current-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Wed, 18 Jun 2003 03:09:00 -0000

On 17 Jun, Alfred Perlstein wrote:
> * Don Lewis <truckman@FreeBSD.org> [030617 13:06] wrote:
>> On 17 Jun, Alfred Perlstein wrote:
>> > * Don Lewis <truckman@FreeBSD.org> [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.
> 
> You can still deadlock because another file's mutex may hash to the same
> location.

... or some other user of the mutex pool that happens to hold Giant.

I'm in favor of committing your patch, though I think it should be
commented to indicate why it is safe to play with fp after the mutex has
been unlocked.