Date: Thu, 29 Aug 2013 18:21:41 -0400 (EDT) From: Rick Macklem <rmacklem@uoguelph.ca> To: Konstantin Belousov <kostikbel@gmail.com> Cc: freebsd-fs <freebsd-fs@freebsd.org> Subject: Re: fixing "umount -f" for the NFS client Message-ID: <2075428996.15437999.1377814901677.JavaMail.root@uoguelph.ca> In-Reply-To: <20130829005616.GH4972@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
------=_Part_15437997_1305582011.1377814901674 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit 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(). > > > > 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. > > > > The problem seems to be that dounmount() msleep()s while > > mnt_lockref != 0 before calling VFS_UNMOUNT(). > > > > 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. > > > > 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. > > > > Anyone have comments on this? > > > Yes, I do. I agree with adding the pre-unmount vfs method. > This seems to be the cleanest solution possible. > 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. I assume I would also need to bump __FreeBSD_version (and maybe VFS_VERSION?). Please review it. If anyone would like to test it, please do so. Thanks, rick ------=_Part_15437997_1305582011.1377814901674 Content-Type: text/x-patch; name=forced-dism.patch Content-Disposition: attachment; filename=forced-dism.patch Content-Transfer-Encoding: base64 LS0tIHN5cy9tb3VudC5oLnNhdgkyMDEzLTA4LTI5IDE1OjE5OjQwLjAwMDAwMDAwMCAtMDQwMAor Kysgc3lzL21vdW50LmgJMjAxMy0wOC0yOSAxNToyMzo1OS4wMDAwMDAwMDAgLTA0MDAKQEAgLTYw OSw2ICs2MDksNyBAQCB0eXBlZGVmIGludCB2ZnNfc3lzY3RsX3Qoc3RydWN0IG1vdW50ICptCiAJ CSAgICBzdHJ1Y3Qgc3lzY3RsX3JlcSAqcmVxKTsKIHR5cGVkZWYgdm9pZCB2ZnNfc3VzcF9jbGVh bl90KHN0cnVjdCBtb3VudCAqbXApOwogdHlwZWRlZiB2b2lkIHZmc19ub3RpZnlfbG93ZXJ2cF90 KHN0cnVjdCBtb3VudCAqbXAsIHN0cnVjdCB2bm9kZSAqbG93ZXJ2cCk7Cit0eXBlZGVmIHZvaWQg dmZzX2tpbGxpb190KHN0cnVjdCBtb3VudCAqbXApOwogCiBzdHJ1Y3QgdmZzb3BzIHsKIAl2ZnNf bW91bnRfdAkJKnZmc19tb3VudDsKQEAgLTYyOCw2ICs2MjksNyBAQCBzdHJ1Y3QgdmZzb3BzIHsK IAl2ZnNfc3VzcF9jbGVhbl90CSp2ZnNfc3VzcF9jbGVhbjsKIAl2ZnNfbm90aWZ5X2xvd2VydnBf dAkqdmZzX3JlY2xhaW1fbG93ZXJ2cDsKIAl2ZnNfbm90aWZ5X2xvd2VydnBfdAkqdmZzX3VubGlu a19sb3dlcnZwOworCXZmc19raWxsaW9fdAkJKnZmc19raWxsaW87CiB9OwogCiB2ZnNfc3RhdGZz X3QJX192ZnNfc3RhdGZzOwpAQCAtNzU2LDYgKzc1OCwxNCBAQCB2ZnNfc3RhdGZzX3QJX192ZnNf c3RhdGZzOwogCX0JCQkJCQkJCVwKIH0gd2hpbGUgKDApCiAKKyNkZWZpbmUJVkZTX0tJTExJTyhN UCkgZG8gewkJCQkJCVwKKwlpZiAoKihNUCktPm1udF9vcC0+dmZzX2tpbGxpbyAhPSBOVUxMKSB7 CQkJXAorCQlWRlNfUFJPTE9HVUUoTVApOwkJCQkJXAorCQkoKihNUCktPm1udF9vcC0+dmZzX2tp bGxpbykoTVApOwkJCVwKKwkJVkZTX0VQSUxPR1VFKE1QKTsJCQkJCVwKKwl9CQkJCQkJCQlcCit9 IHdoaWxlICgwKQorCiAjZGVmaW5lIFZGU19LTk9URV9MT0NLRUQodnAsIGhpbnQpIGRvCQkJCQlc CiB7CQkJCQkJCQkJXAogCWlmICgoKHZwKS0+dl92ZmxhZyAmIFZWX05PS05PVEUpID09IDApCQkJ CVwKLS0tIGtlcm4vdmZzX21vdW50LmMuc2F2CTIwMTMtMDgtMjkgMTU6MjQ6MzYuMDAwMDAwMDAw IC0wNDAwCisrKyBrZXJuL3Zmc19tb3VudC5jCTIwMTMtMDgtMjkgMTc6MjI6NTEuMDAwMDAwMDAw IC0wNDAwCkBAIC0xMjY5LDggKzEyNjksMTYgQEAgZG91bm1vdW50KG1wLCBmbGFncywgdGQpCiAJ fQogCW1wLT5tbnRfa2Vybl9mbGFnIHw9IE1OVEtfVU5NT1VOVCB8IE1OVEtfTk9JTlNNTlRROwog CS8qIEFsbG93IGZpbGVzeXN0ZW1zIHRvIGRldGVjdCB0aGF0IGEgZm9yY2VkIHVubW91bnQgaXMg aW4gcHJvZ3Jlc3MuICovCi0JaWYgKGZsYWdzICYgTU5UX0ZPUkNFKQorCWlmIChmbGFncyAmIE1O VF9GT1JDRSkgewogCQltcC0+bW50X2tlcm5fZmxhZyB8PSBNTlRLX1VOTU9VTlRGOworCQlNTlRf SVVOTE9DSyhtcCk7CisJCS8qCisJCSAqIE11c3QgYmUgZG9uZSBhZnRlciBzZXR0aW5nIE1OVEtf VU5NT1VOVEYgYW5kIGJlZm9yZQorCQkgKiB3YWl0aW5nIGZvciBtbnRfbG9ja3JlZiB0byBiZWNv bWUgMC4KKwkJICovCisJCVZGU19LSUxMSU8obXApOworCQlNTlRfSUxPQ0sobXApOworCX0KIAll cnJvciA9IDA7CiAJaWYgKG1wLT5tbnRfbG9ja3JlZikgewogCQltcC0+bW50X2tlcm5fZmxhZyB8 PSBNTlRLX0RSQUlOSU5HOwotLS0gZnMvbmZzY2xpZW50L25mc19jbHZmc29wcy5jLnNhdgkyMDEz LTA4LTI5IDE1OjMwOjUwLjAwMDAwMDAwMCAtMDQwMAorKysgZnMvbmZzY2xpZW50L25mc19jbHZm c29wcy5jCTIwMTMtMDgtMjkgMTY6NTI6NTQuMDAwMDAwMDAwIC0wNDAwCkBAIC0xMjAsNiArMTIw LDcgQEAgc3RhdGljIHZmc19yb290X3QgbmZzX3Jvb3Q7CiBzdGF0aWMgdmZzX3N0YXRmc190IG5m c19zdGF0ZnM7CiBzdGF0aWMgdmZzX3N5bmNfdCBuZnNfc3luYzsKIHN0YXRpYyB2ZnNfc3lzY3Rs X3QgbmZzX3N5c2N0bDsKK3N0YXRpYyB2ZnNfa2lsbGlvX3QgbmZzX2tpbGxpbzsKIAogLyoKICAq IG5mcyB2ZnMgb3BlcmF0aW9ucy4KQEAgLTEzNCw2ICsxMzUsNyBAQCBzdGF0aWMgc3RydWN0IHZm c29wcyBuZnNfdmZzb3BzID0gewogCS52ZnNfdW5pbml0ID0JCW5jbF91bmluaXQsCiAJLnZmc191 bm1vdW50ID0JCW5mc191bm1vdW50LAogCS52ZnNfc3lzY3RsID0JCW5mc19zeXNjdGwsCisJLnZm c19raWxsaW8gPQkJbmZzX2tpbGxpbywKIH07CiBWRlNfU0VUKG5mc192ZnNvcHMsIG5mcywgVkZD Rl9ORVRXT1JLIHwgVkZDRl9TQkRSWSk7CiAKQEAgLTE2NzYsNiArMTY3OCwxOSBAQCBuZnNfc3lz Y3RsKHN0cnVjdCBtb3VudCAqbXAsIGZzY3Rsb3BfdCBvCiB9CiAKIC8qCisgKiBLaWxsIG9mZiBh bnkgUlBDcyBpbiBwcm9ncmVzcywgc28gdGhhdCB0aGV5IHdpbGwgYWxsIHJldHVybiBlcnJvcnMu CisgKiBUaGlzIGFsbG93cyBkb3VubW91bnQoKSB0byBjb250aW51ZSBhcyBmYXIgYXMgVkZTX1VO TU9VTlQoKSBmb3IgYQorICogZm9yY2VkIGRpc21vdW50LgorICovCitzdGF0aWMgdm9pZAorbmZz X2tpbGxpbyhzdHJ1Y3QgbW91bnQgKm1wKQoreworCXN0cnVjdCBuZnNtb3VudCAqbm1wID0gVkZT VE9ORlMobXApOworCisJbmV3bmZzX25tY2FuY2VscmVxcyhubXApOworfQorCisvKgogICogRXh0 cmFjdCB0aGUgaW5mb3JtYXRpb24gbmVlZGVkIGJ5IHRoZSBubG0gZnJvbSB0aGUgbmZzIHZub2Rl LgogICovCiBzdGF0aWMgdm9pZAo= ------=_Part_15437997_1305582011.1377814901674--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?2075428996.15437999.1377814901677.JavaMail.root>