Summary: | [nlm] Fatal trap 9 when printing out KASSERT trying to run umount -f on an NFS share while it's trying to print out "lockd not responding" in nlm(4) | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | Base System | Reporter: | Enji Cooper <ngie> | ||||||
Component: | kern | Assignee: | freebsd-fs (Nobody) <fs> | ||||||
Status: | Closed FIXED | ||||||||
Severity: | Affects Some People | CC: | benno, delphij, gonzo, kib, pho, rmacklem | ||||||
Priority: | --- | ||||||||
Version: | CURRENT | ||||||||
Hardware: | Any | ||||||||
OS: | Any | ||||||||
Attachments: |
|
Description
Enji Cooper
2015-06-02 04:32:09 UTC
Created attachment 157364 [details]
This LOR might be interesting...
I think the LOR may be unrelated. This happens because umount -f (nfs_unmount in sys/fs/nfsclient/nfs_clvfsops.c:1522) free's the mountpoint structure (1579) but nlm have kept a reference to it. In order to fix this I think we would need a way similar to newnfs_nmcancelreqs() that cancells all lockd RPCs that are associated with the mountpoint (i.e. abort all lock requests that are related to the unmounting mountpoint) which is probably non-trivial with the current structure. Looks like td_ucred problem: https://people.freebsd.org/~pho/stress/log/svn.txt (In reply to Peter Holm from comment #3) Indeed, the unmount which drops the last reference on the nfs mount credentials is the culprit. This sole issue can be easily handled by taking additional reference on the mnt_cred, before the vnode is unlocked. If there are further problems from the mnt disappearing, we could try to busy the filesystem after the vnode is unlocked. But this would have bad consequences for the unmount -f. Lets try the minimal patch first. Created attachment 157390 [details]
Hold the mnt credentials before vnode unlock.
I see spinning threads with this patch: https://people.freebsd.org/~pho/stress/log/kostik813.txt It's been a while since I looked at it, but I remember that there is a known issue with the client side NLM for "umount -f". Basically it unlocks and later relocks the vnode without holding a refcnt on the mount point. At the time, I didn't know enough about the mount point refcnt stuff to fix it. I'll try and take a look later to-day. I don't know if this is the cause of the crash (I can't remember what credentials are used), but it does need to be fixed for "umount -f" to be safe when running rpc.lockd. (I keep hoping people will stop using rpc.lockd and rpc.statd, but it's just a dream;-) Although adding a ref count to the cred struct may help avoid some crashes, the code that prints out the message uses both the mount structure and the NFS specific one hanging off of mnt_data. Unfortunately adding a ref count to the mount structure via vfs_ref(mp) only delays freeing of *mp and not *mnt_data. Also, vfs_destroy_mount() waits for the ref count on the mount structure to go to 0 and, as such, "umount -f" won't complete until the lock RPC completes. If the ref count on the mount structure only free'd the structure and substructure hanging off of mnt_data, then adding a ref count to the mount structure before the VOP_UNLOCK() could avoid the crashes. Maybe this VFS change should be considered? Also, doing "umount -f" on a file system using locking could leave "dangling locks" on files. In theory, rpc.statd should eventually notice that the host isn't responding and get rid of these file locks, but I wouldn't bet on it. I suppose the dangers of using "umount -f" while running rpc.lockd should be documented. I'll come up with a man page patch for that. A commit references this bug: Author: rmacklem Date: Wed Jun 17 23:24:47 UTC 2015 New revision: 284531 URL: https://svnweb.freebsd.org/changeset/base/284531 Log: Document that a forced dismount of an NFSv3 mount when the NLM (rpc.lockd) is running can crash the system. Unfortunately this is not easy to fix, but I have left PR#200585 open. PR: 200585 MFC after: 3 days Changes: head/sbin/umount/umount.8 There is a commit referencing this PR, but it's still not closed and has been inactive for some time. Closing the PR as fixed but feel free to re-open it if the issue hasn't been completely resolved. Thanks |