From owner-freebsd-current@FreeBSD.ORG Wed Mar 5 19:43:31 2014 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 66421605; Wed, 5 Mar 2014 19:43:31 +0000 (UTC) Received: from bigwig.baldwin.cx (bigwig.baldwin.cx [IPv6:2001:470:1f11:75::1]) (using TLSv1 with cipher ADH-CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 3B1193D9; Wed, 5 Mar 2014 19:43:31 +0000 (UTC) Received: from jhbbsd.localnet (unknown [209.249.190.124]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 19A1DB97B; Wed, 5 Mar 2014 14:43:30 -0500 (EST) From: John Baldwin To: Konstantin Belousov Subject: Re: panic: lockmgr still held [tmpfs] [vm_map_remove()->vdropl()] (r262186: Thu Feb 20) Date: Wed, 5 Mar 2014 14:21:04 -0500 User-Agent: KMail/1.13.5 (FreeBSD/8.4-CBSD-20130906; KDE/4.5.5; amd64; ; ) References: <53109ACB.20102@FreeBSD.org> <5316F144.1000105@FreeBSD.org> <20140305110723.GB24664@kib.kiev.ua> In-Reply-To: <20140305110723.GB24664@kib.kiev.ua> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Message-Id: <201403051421.04381.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Wed, 05 Mar 2014 14:43:30 -0500 (EST) Cc: Alan Cox , freebsd-current@freebsd.org, Andriy Gapon , Bryan Drewery X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.17 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: Wed, 05 Mar 2014 19:43:31 -0000 On Wednesday, March 05, 2014 6:07:23 am Konstantin Belousov wrote: > On Wed, Mar 05, 2014 at 11:41:24AM +0200, Andriy Gapon wrote: > > on 04/03/2014 18:45 John Baldwin said the following: > > > So I'm not sure how to fix this. The crash is in this code in > > > vm_object_deallocate(): > > > > > > if (object->type == OBJT_SWAP && > > > (object->flags & OBJ_TMPFS) != 0) { > > > vp = object->un_pager.swp.swp_tmpfs; > > > vhold(vp); > > > VM_OBJECT_WUNLOCK(object); > > > vn_lock(vp, LK_EXCLUSIVE | LK_RETRY); > > > vdrop(vp); > > > VM_OBJECT_WLOCK(object); > > > if (object->type == OBJT_DEAD || > > > object->ref_count != 1) { > > > VM_OBJECT_WUNLOCK(object); > > > VOP_UNLOCK(vp, 0); > > > return; > > > } > > > if ((object->flags & OBJ_TMPFS) != 0) > > > VOP_UNSET_TEXT(vp); > > > VOP_UNLOCK(vp, 0); > > > } > > > > > > The vdrop() is dropping the count to zero and trying to free the vnode. The > > > real problem I think is that swp_tmpfs doesn't have an implicit vhold() on the > > > vnode, so in this case, the code is doing a vhold/vn_lock/vdrop of an already- > > > free vnode. For OBJT_VNODE objects, the reference from the object back to the > > > vnode holds a vref() that gets released by a vput() in > > > vm_object_vndeallocate(). > > > > > > One fix might be to chagne smp_tmpfs to hold a vhold reference. This is > > > untested but might work (but I'm also not sure that this is the right thing in > > > that I don't know what other effects it might have). > > > > I agree with your analysis, but I don't think that a filesystem holding its own > > vnode is a good idea. If I am not mistaken, that would prevent tmpfs vnodes > > from going to free list. > > I'd rather try to modify vm_object_deallocate() code. E.g. vdrop() could be > > called after VOP_UNLOCK(). Alternatively, the code could handle a doomed vnode > > in a different way. > > I agree with Andrey, it is just a bug to vdrop() before unlock. > Please try this. Ok, my only worry is in the case of Bryan's panic, the hold count on the vnode was already zero before vhold() was called, so is it possible that it is a stale pointer or is there some other implicit reference that prevents that? If it can't be stale, I think deferring the vdrop() is fine. -- John Baldwin