More careful handling of the mount failure.
ClosedPublic
Actions

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

Details

Reviewers

markj
mjg
rmacklem

Commits

rS368077: nullfs: provide custom bypass for VOP_READ_PGCACHE().
rS368076: vn_read_from_obj(): fix handling of doomed vnodes.
rS368075: More careful handling of the mount failure.

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

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

kib created this revision.Nov 23 2020, 8:13 AM

Herald added a subscriber: imp. · View Herald TranscriptNov 23 2020, 8:13 AM

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

Harbormaster completed remote builds in B34969: Diff 79879.Nov 23 2020, 8:13 AM

kib added reviewers: markj, mjg.Nov 24 2020, 3:34 PM

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

kib added a reviewer: rmacklem.Nov 25 2020, 8:02 AM

markj added inline comments.Nov 25 2020, 3:56 PM

sys/kern/vfs_mount.c
980 ↗	(On Diff #79879)	Extra ws after `(void)`.

markj added inline comments.Nov 25 2020, 3:58 PM

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().

Move comment.
Remove space.
Improve diagnostic message.

This revision now requires review to proceed.Nov 25 2020, 4:21 PM

Harbormaster completed remote builds in B35034: Diff 79992.Nov 25 2020, 4:21 PM

markj accepted this revision.Nov 25 2020, 4:25 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.