From owner-freebsd-fs@FreeBSD.ORG Mon Apr 28 13:24:25 2008 Return-Path: Delivered-To: fs@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id CA7971065677; Mon, 28 Apr 2008 13:24:25 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from anti-4.kiev.sovam.com (anti-4.kiev.sovam.com [62.64.120.202]) by mx1.freebsd.org (Postfix) with ESMTP id 625968FC1C; Mon, 28 Apr 2008 13:24:25 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from [212.82.216.226] (helo=skuns.kiev.zoral.com.ua) by anti-4.kiev.sovam.com with esmtps (TLSv1:AES256-SHA:256) (Exim 4.67) (envelope-from ) id 1JqTLD-0003Wq-8T; Mon, 28 Apr 2008 16:24:23 +0300 Received: from deviant.kiev.zoral.com.ua (root@deviant.kiev.zoral.com.ua [10.1.1.148]) by skuns.kiev.zoral.com.ua (8.14.2/8.14.2) with ESMTP id m3SDOJZh040737 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 28 Apr 2008 16:24:19 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: from deviant.kiev.zoral.com.ua (kostik@localhost [127.0.0.1]) by deviant.kiev.zoral.com.ua (8.14.2/8.14.2) with ESMTP id m3SDODBx052241; Mon, 28 Apr 2008 16:24:13 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: (from kostik@localhost) by deviant.kiev.zoral.com.ua (8.14.2/8.14.2/Submit) id m3SDODTT052240; Mon, 28 Apr 2008 16:24:13 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: deviant.kiev.zoral.com.ua: kostik set sender to kostikbel@gmail.com using -f Date: Mon, 28 Apr 2008 16:24:13 +0300 From: Kostik Belousov To: Daichi GOTO Message-ID: <20080428132413.GS18958@deviant.kiev.zoral.com.ua> References: <4811B0A0.8040702@freebsd.org> <20080426100116.GL18958@deviant.kiev.zoral.com.ua> <48159AC5.3030000@freebsd.org> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="A61Eau4L8twGtri1" Content-Disposition: inline In-Reply-To: <48159AC5.3030000@freebsd.org> User-Agent: Mutt/1.4.2.3i X-Virus-Scanned: ClamAV version 0.91.2, clamav-milter version 0.91.2 on skuns.kiev.zoral.com.ua X-Virus-Status: Clean X-Spam-Status: No, score=-4.4 required=5.0 tests=ALL_TRUSTED,AWL,BAYES_00 autolearn=ham version=3.2.4 X-Spam-Checker-Version: SpamAssassin 3.2.4 (2008-01-01) on skuns.kiev.zoral.com.ua X-Scanner-Signature: e7d6a74e6a41bfc4865bf8c76b21c35c X-DrWeb-checked: yes X-SpamTest-Envelope-From: kostikbel@gmail.com X-SpamTest-Group-ID: 00000000 X-SpamTest-Info: Profiles 2733 [Apr 28 2008] X-SpamTest-Info: helo_type=3 X-SpamTest-Info: {received from trusted relay: not dialup} X-SpamTest-Method: none X-SpamTest-Method: Local Lists X-SpamTest-Rate: 0 X-SpamTest-Status: Not detected X-SpamTest-Status-Extended: not_detected X-SpamTest-Version: SMTP-Filter Version 3.0.0 [0255], KAS30/Release Cc: fs@freebsd.org Subject: Re: Approval request of some additions to src/sys/kern/vfs_subr.c and src/sys/sys/vnode.h X-BeenThere: freebsd-fs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Filesystems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 28 Apr 2008 13:24:25 -0000 --A61Eau4L8twGtri1 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Apr 28, 2008 at 06:37:09PM +0900, Daichi GOTO wrote: > Kostik Belousov wrote: > >On Fri, Apr 25, 2008 at 07:21:20PM +0900, Daichi GOTO wrote: > >>Hi Konstantin :) > >> > >>To fix a unionfs issue of=20 > >>http://www.freebsd.org/cgi/query-pr.cgi?pr=3D109377, > >>we need to add new functions > >> > >> void vkernrele(struct vnode *vp); > >> void vkernref(struct vnode *vp); > >> > >>and one value > >> > >> int v_kernusecount; /* i ref count of kernel */ > >> > >>to src/sys/sys/vnode.h and rc/sys/kern/vfs_subr.c. > >> > >>Unionfs will be panic when lower fs layer is forced umounted by > >>"umount -f". So to avoid this issue, we've added > >>"v_kernusecount" value that means "a vnode count that kernel are > >>using". vkernrele() and vkernref() are functions that manage > >>"v_kernusecount" value. > >> > >>Please check those and give us an approve or some comments! > > > >There is already the vnode reference count. From your description, I can= not > >understand how the kernusecount would prevent the panic when forced unmo= unt > >is performed. Could you, please, show the actual code ? PR mentioned > >does not contain any patch. >=20 > Our patch realizes avoiding kernel panic by "umount -f" operation using w= ith > EBUSY process. >=20 > On current implementation (not applied our patch), "umount -f" tries to > release vnode at any vnode reference count value. Since that, unionfs > and nullfs access invalid vnode and lead kernel panic. To prevent this > issue, we need a some kind of not-umount-accept-mechanism in invalid case > (e.x. fs in unionfsed stack, it must be umounted in correct order) and > to realize that, current vnode reference count is not enough we are=20 > thinking. >=20 > If you have any ideas to realize the same solution with current vnode > reference, would you please tell us your idea :) >=20 > >The problem you described is common for the kernel code, and right way > >to handle it, for now, is to keep refcount _and_ check for the forced > >reclaim. Your patch in essence disables the forced unmount. I would object against such decision. Even if taking this direction, I believe more cleaner solution would be to introduce a counter that disables the (forced) unmount into the struct mount, instead of the struct vnode. Having the counter in the vnode, the unmount -f behaviour is non-deterministic and depended on the presence of the cached vnodes of the upper layer. The mount counter would be incremented by unionfs cover mount. But, as I said above, this looks like a wrong solution. The right way to handle the forced reclaim with the current VFS is to add the explicit checks for the reclaimed vnodes where it is needed. The vnode cannot be reclaimed while the vnode lock is held. When obtaining the vnode lock, the reclamation can be detected. For instance, the vget() without LK_RETRY shall be checked for ENOENT. You said that that nullfs is vulnerable to the problem. Could you, please, point me to the corresponding stack trace ? At least, the nullfs vop_lock() seems to carefully check the possible problems. --A61Eau4L8twGtri1 Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (FreeBSD) iEYEARECAAYFAkgVz/wACgkQC3+MBN1Mb4isvwCfbECmYEu6lJ2FXIqaU3zYPTZs 5I0AoNzrqhXvT5XHDQs+l65owxM8rfp3 =eTfF -----END PGP SIGNATURE----- --A61Eau4L8twGtri1--