From owner-freebsd-fs@FreeBSD.ORG Tue Jan 14 14:49:07 2014 Return-Path: Delivered-To: freebsd-fs@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 75654AD3 for ; Tue, 14 Jan 2014 14:49:07 +0000 (UTC) Received: from citadel.icyb.net.ua (citadel.icyb.net.ua [212.40.38.140]) by mx1.freebsd.org (Postfix) with ESMTP id D4A621D05 for ; Tue, 14 Jan 2014 14:49:06 +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 QAA22508; Tue, 14 Jan 2014 16:49:04 +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 1W35Ie-000Ayx-7G; Tue, 14 Jan 2014 16:49:04 +0200 Message-ID: <52D54E28.5020208@FreeBSD.org> Date: Tue, 14 Jan 2014 16:48:08 +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> <52D5239B.6000203@FreeBSD.org> 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 14:49:07 -0000 on 14/01/2014 14:25 krichy@tvnetwork.hu said the following: > Dear Andriy, > > I've also sent a detailed explanation of my changes to Will Andrews, I could > also forward that to you. This would be nice. > You are right, these areas are critical areas, though > the patch is very small and solves some of the issues. And that is also true > that I am not a really FreeBSD developer, just I compared illumos and freebsd > code, debugged these locking issues for a while. Then at the end I think the > result patch is reasonably small, and I also tried to follow some freebsd > recommendations. I hope to be able to examine your proposed patch soon-ish. > If I can help somehow I am willing to do. Also I would accept any critics on my > work. It is for a month or two now when we discovered these bugs, and > unfortunately it happened on our production servers, and with these patches our > system works since then. > > But of course, that is not a proof of concept. Just in case, does the patched code pass my concurrent ls -l test? :-) The "test" is here: http://thread.gmane.org/gmane.os.freebsd.devel.file-systems/18924/focus=19056 > > Kojedzinszky Richard > Euronet Magyarorszag Informatikai Zrt. > > On Tue, 14 Jan 2014, Andriy Gapon wrote: > >> Date: Tue, 14 Jan 2014 13:46:35 +0200 >> From: Andriy Gapon >> To: krichy@tvnetwork.hu >> Cc: freebsd-fs@FreeBSD.org >> Subject: Re: kern/184677 / ZFS snapshot handling deadlocks >> >> 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 >> -- Andriy Gapon