From owner-freebsd-fs@FreeBSD.ORG Sun Oct 14 13:37:26 2012 Return-Path: Delivered-To: freebsd-fs@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 40242D72; Sun, 14 Oct 2012 13:37:26 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from citadel.icyb.net.ua (citadel.icyb.net.ua [212.40.38.140]) by mx1.freebsd.org (Postfix) with ESMTP id 57EE48FC14; Sun, 14 Oct 2012 13:37:24 +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 QAA29690; Sun, 14 Oct 2012 16:37:23 +0300 (EEST) (envelope-from avg@FreeBSD.org) Received: from localhost ([127.0.0.1]) by porto.starpoint.kiev.ua with esmtp (Exim 4.34 (FreeBSD)) id 1TNONe-000CrP-Cq; Sun, 14 Oct 2012 16:37:23 +0300 Message-ID: <507AC00E.7060407@FreeBSD.org> Date: Sun, 14 Oct 2012 16:37:18 +0300 From: Andriy Gapon User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:16.0) Gecko/20121013 Thunderbird/16.0.1 MIME-Version: 1.0 To: Pawel Jakub Dawidek Subject: Re: potential zfs/vfs trouble in force umount References: <507A8954.3000702@FreeBSD.org> <20121014112546.GH1383@garage.freebsd.pl> <507AAC38.3000709@FreeBSD.org> In-Reply-To: <507AAC38.3000709@FreeBSD.org> X-Enigmail-Version: 1.4.5 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.14 Precedence: list List-Id: Filesystems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 14 Oct 2012 13:37:26 -0000 on 14/10/2012 15:12 Andriy Gapon said the following: > on 14/10/2012 14:25 Pawel Jakub Dawidek said the following: >> On Sun, Oct 14, 2012 at 12:43:48PM +0300, Andriy Gapon wrote: >>> >>> I think that there is the following potentially troublesome scenario. One >>> thread does zil_commit and obtains a znode pointer using zfs_zget. At >>> this point the thread doesn't have any locks on either the znode or its >>> vnode. the only thing that is supposed to keep them around is a >>> reference on the vnode. If a force umount is going on in parallel, the >>> one of the first things it does is calling vflush(FORCECLOSE) (this >>> happens before closing down zil). vflush force-reclaims all vnodes in >>> this case (even when v_usecount > 0). So the znode in question gets >>> destroyed. Later, when the first thread tries to dereference the znode >>> pointer it would crash. >> >> The z_teardown_lock lock is held for reading for every VOP and zfs_umount() >> obtains this lock for writing before calling vflush(FORCECLOSE) and sets >> z_unmounted to true. This in turn will make every new VOP to return with >> EIO. This ensures that no VOP is in-progress when vflush() is called. >> > > What was/is not clear to me is whether zil operations are always called under > z_teardown_lock (aka ZFS_ENTER)... > OK, this appears to be true. The only special case is zil_commit being called from zil_close in umount. Because zil_close is called after vflush all the znodes would already be properly disposed of. If there is anything in zil at that point, then I am not sure what zfs_zget would do... Return an error and some the records would remain uncommitted/lost? Or create znodes (and vnodes) despite the unmount going on? Something else?.. Umm, looks like there would be an error in insmntque and so zfs_zget would return an error. Not sure if losing some zil entries (non-sync operations) is something to worry about for the forceful unmounting, though. -- Andriy Gapon