Date: Fri, 30 Aug 2013 01:31:28 +0300 From: Konstantin Belousov <kostikbel@gmail.com> To: Rick Macklem <rmacklem@uoguelph.ca> Cc: freebsd-fs <freebsd-fs@freebsd.org> Subject: Re: fixing "umount -f" for the NFS client Message-ID: <20130829223128.GP4972@kib.kiev.ua> In-Reply-To: <2075428996.15437999.1377814901677.JavaMail.root@uoguelph.ca> References: <20130829005616.GH4972@kib.kiev.ua> <2075428996.15437999.1377814901677.JavaMail.root@uoguelph.ca>
next in thread | previous in thread | raw e-mail | index | archive | help
--uFc9xn1y3ioKq896 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Aug 29, 2013 at 06:21:41PM -0400, Rick Macklem wrote: > Kostik wrote: > > On Wed, Aug 28, 2013 at 08:15:27PM -0400, Rick Macklem wrote: > > > I've been doing a little more testing of "umount -f" for NFS > > > mounts and they seem to be working unless some other process/thread > > > has busied the file system via vfs_busy(). > > >=20 > > > Unfortunately, it is pretty easy to vfs_busy() the file system > > > by using a command like "df" that is stuck on the unresponsive > > > NFS server. > > >=20 > > > The problem seems to be that dounmount() msleep()s while > > > mnt_lockref !=3D 0 before calling VFS_UNMOUNT(). > > >=20 > > > If some call into the NFS client was done before this > > > while (mp->mnt_lockref) loop with msleep() in it, it > > > can easily kill off RPCs in progress. (It currently > > > does this in nfs_unmount() using the newnfs_nmcancelreqs() > > > call. > > >=20 > > > In summary: > > > - Would it be appropriate to add a new vfs_XXX method that > > > dounmount() would call before the while() loop for the > > > forced dismount case? > > > (The default would be a no-op and I have no idea if any > > > file system other than NFS would have a use for it?) > > > Alternately, there could be a function pointer set non-NULL > > > that would specifically be used by the NFS client for this. > > > This would avoid adding a vfs_XXX() method, but would mean > > > an NFS specific call ends up in the generic dounmount() code. > > >=20 > > > Anyone have comments on this? > > >=20 > > Yes, I do. I agree with adding the pre-unmount vfs method. > > This seems to be the cleanest solution possible. > >=20 > I've attached a patch. It is also at > http://people.freebsd.org/~rmacklem/forced-dism.patch > in case the attachment gets lost. > I don't really like doing the MNT_IUNLOCK(), MNT_ILOCK() before/after > the VFS_KILLIO() call, but I couldn't see any better way to do it and > it looks safe to do so, at least for the forced case. Might be, call it VFS_PURGE() ? I suggest to move the call to the VFS_KILLIO after the MNTK_DRAINING is set, to avoid getting new references after the current i/o transactions are stopped. You would need to set MNTK_DRAINING unconditionally. Also, it probably makes sense to replace the if (mnt_lockref) with while (). >=20 > I assume I would also need to bump __FreeBSD_version (and maybe VFS_VERSI= ON?). I think you could avoid it. --uFc9xn1y3ioKq896 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.21 (FreeBSD) iQIcBAEBAgAGBQJSH8u/AAoJEJDCuSvBvK1B1dsQAKlRITSVql/vHTUinsxv6hq5 Qaq8EdsVyTTwoUi2fqp1Vf91B+K0prFiWEwJkJHfLIMzKCGT47wM30pWQvDRZ+Xl OcrOLilWj3WgdolfQRubNH35xlO5WhQOSxuPslhzyIvQOUninRUBXWQZy5ptq4tG nAHHLmlfviAN5gfu3gKoSizfDoB0aJwC0MoLZMD72ta4CVArmkK/qQMxNbZn+w7O U7/Owm1HCZKNyAukdLIpmqEViusCDpvwtfNR3wi5JaJLE+leT8K/vcmCivLH9QqP Lx/SppNsW9a/temHpSs4mBK7pR+awqF2k0cVYIoquSyDk1xaMsqYQOwWhYDbzm51 CSsm89c0TNZJyuQxfCouNVg7p3pzrRjyC5EE+Azu9T+gkTNq+PQaVr6G3DOQdEdm 1DbxAiBK5EC2yfdQREf9twRcXVB2Qor8IEEXeuyzX3P+s+9NI2RqrpXyKRC0c4We KkNdwHkakhA8PQZl64x8lP8ugvye7JewYa0gTORehBwR/fuAfMqhYfWVgoAYKH0b AfW1Zx3tnSN3gX0FVM761Q+hZRJ3Ov76obsSS1pr+WmNEOwtdzdCC4bS0xqx21+B WCR/Fqf+HMd8L6KJLXbG7zDeuZyYddR5Pg4o3f9lQ8SMRCxEZFRyrnGByP8RfHaK XwuZaen7vIO4wF30Wjtq =+HoH -----END PGP SIGNATURE----- --uFc9xn1y3ioKq896--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130829223128.GP4972>