From owner-freebsd-fs@FreeBSD.ORG Tue Jan 14 11:47:15 2014 Return-Path: Delivered-To: freebsd-fs@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 A8D30E96; Tue, 14 Jan 2014 11:47:15 +0000 (UTC) Received: from citadel.icyb.net.ua (citadel.icyb.net.ua [212.40.38.140]) by mx1.freebsd.org (Postfix) with ESMTP id A3BBB1CE7; Tue, 14 Jan 2014 11:47:14 +0000 (UTC) Received: from porto.starpoint.kiev.ua (porto-e.starpoint.kiev.ua [212.40.38.100]) by citadel.icyb.net.ua (8.8.8p3/ICyb-2.3exp) with ESMTP id NAA18720; Tue, 14 Jan 2014 13:47:12 +0200 (EET) (envelope-from avg@FreeBSD.org) Received: from localhost ([127.0.0.1]) by porto.starpoint.kiev.ua with esmtp (Exim 4.34 (FreeBSD)) id 1W32Se-000Ai8-41; Tue, 14 Jan 2014 13:47:12 +0200 Message-ID: <52D5239B.6000203@FreeBSD.org> Date: Tue, 14 Jan 2014 13:46:35 +0200 From: Andriy Gapon User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:24.0) Gecko/20100101 Thunderbird/24.2.0 MIME-Version: 1.0 To: krichy@tvnetwork.hu Subject: Re: kern/184677 / ZFS snapshot handling deadlocks References: <20131220134405.GE1658@garage.freebsd.pl> In-Reply-To: X-Enigmail-Version: 1.6 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: freebsd-fs@FreeBSD.org X-BeenThere: freebsd-fs@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: Filesystems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 14 Jan 2014 11:47:15 -0000 on 25/12/2013 07:22 krichy@tvnetwork.hu said the following: > I've made a new patch again, which fixes most of my earlier issues. Mainly, I've > enabled shared vnode locks for GFS vnodes - is it acceptable? And that way, > deadlock cases reduced much, and also the patch is more clear (at least to me). > One thing still remains, the spa_namespace_lock race I mentioned before, I dont > know how to handle that. > > Waiting for comments on this. Richard, first of all, apologies for the delay with a reply and for not having any comment on the essence of your patch... I do have the following meta-comment. - working with FreeBSD VFS is hard - porting code that was written for a very different VFS model to FreeBSD VFS is even harder - doing the same for the code that plays various tricks, like .zfs support code does, is harder again - reviewing somebody else's changes to that kind of code is harder still than making such changes This is quite an unfortunate situation. I am not much surprised by the lack of followups to your analysis and patches. I am saying this as someone who spent some time analyzing and trying to fix the .zfs code and ZFS<->VFS interaction in general. See e.g. http://thread.gmane.org/gmane.os.freebsd.devel.file-systems/18924/focus=19056 My opinion is that .zfs code breaks several fundamental FreeBSD VFS contracts. The most glaring violation is that VOP_INACTIVE makes a vnode invalid. I think that trying to fix .zfs code by patching individual problems here and there is an uphill battle. I think that the same also applies to ZFS ZPL code but in a less obvious way. The code in many cases just pretends to play by VFS rules by satisfying most obvious assertions, but it does not really try to obey VFS contracts. For example, almost all locks in znode are mostly redundant given VFS vnode locking. But in some cases they are not sufficient precisely because VFS expects its locking to be used rather than ZFS internal locking. The most obvious example is interaction of VOP_RENAME with other VOPs. In any case, thanks for your work! I am trying to find some time to review it. -- Andriy Gapon