Page MenuHomeFreeBSD

More careful handling of the mount failure.
ClosedPublic

Authored by kib on Nov 23 2020, 8:13 AM.

Details

Summary
  • VFS_UNMOUNT() requires vn_start_write() around it [*].
  • call VFS_PURGE() before unmount.
  • do not destroy mp if cleanup unmount did not succeed.
  • set MNTK_UNMOUNT, and indicate forced unmount with MNTK_UNMOUNTF for VFS_UNMOUNT() in cleanup.

PR: 251320 [*]

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

kib requested review of this revision.Nov 23 2020, 8:13 AM

rmacklem has nfs tests which trigger this codepath, would be useful to ask him.

I think the thing to do is to fold all this work into mount routines so that there is no need to backpedal from actual mount, but that's quite a bit of work.

This revision is now accepted and ready to land.Nov 25 2020, 1:37 AM
sys/kern/vfs_mount.c
980 ↗(On Diff #79879)

Extra ws after (void).

sys/kern/vfs_mount.c
985 ↗(On Diff #79879)

We are in a VFS op section here and the mp is busy, so mnt_lockref >= 1, so other busying threads cannot be drained using the existing mechanism. I am not sure if it is possible to have mnt_lockref > 1 here.

kib marked 2 inline comments as done.Nov 25 2020, 4:12 PM
kib added inline comments.
sys/kern/vfs_mount.c
980 ↗(On Diff #79879)

At least two other instances of ignoring return value use space, in this file.

985 ↗(On Diff #79879)

VFS op section only causes slow/precise version of references use. That said, I moved the comment right before vfs_mount_destroy().

kib marked 2 inline comments as done.

Move comment.
Remove space.
Improve diagnostic message.

This revision now requires review to proceed.Nov 25 2020, 4:21 PM
This revision is now accepted and ready to land.Nov 25 2020, 4:25 PM

Looks ok to me. I should be able to test this
someday soon by making the NFS VFS_STATFS()
fail.
Others know a lot more about the VFS side and
have already clicked reviewed, so I've left that to them.

This revision was automatically updated to reflect the committed changes.