Make getfsstat(2) not skip filesystems being unmounted if passed MNT_WAIT flag.

[trasz]

Summary

As it is now, the getfsstat(2) syscall will skip filesystems that are
in the process of being unmounted, even if the unmount would eventually
fail due to eg the filesystem being busy.

This behaviour breaks autounmountd(8) - if you try to manually unmount
a mounted filesystem, using 'automount -u', and the autounmountd attempts
to refresh the filesystem list in that very moment, it would conclude that
the filesystem got unmounted and not try to unmount it afterwards.

This behaviour depends on MNT_WAIT flag being passed to getfsstat(2).
This makes getfsstat(2) "hang" in case of hung mount - eg NFS with the server
being offline - and it's the same thing that would happen with MNT_WAIT
anyway.

[kib]

Erm, this code seems to have stupid bugs. Namely, MNT_WAIT and other constants are not flags. So the test for MNT_LAZY always implies MNT_WAIT etc. I think this must be untangles in kern_getfsstat() before your fix can be elaborated more.

Did you reviewed the callers ?

[trasz]

Fixed version.

[kib]

sys/kern/vfs_syscalls.c

485 ↗

(On Diff #21791)

If MNT_WAIT was specified to the vfs_busy() call, and error is returned, then mp might become unmounted and removed from the list. In this case, calculation of nmp below is wrong.

[trasz]

Is it enough to just replace the loop with TAILQ_FOREACH_SAFE()?

[kib]

In D8030#174387, @trasz wrote:

Is it enough to just replace the loop with TAILQ_FOREACH_SAFE()?

Of course no. What would prevent the struct mount referenced by the temp pointer from being unmounted ?

[trasz]

Make sure the mp doesn't get freed.

[kib]

The mount point structure is never freed since the uma zone is type-stable. Also vfs_busy() manages the references itself.

Please re-read the description of the issue, it has nothing to do with the free of the mp. It is about mp being removed from the global list on umount, and the iterator both loosing the position and TAILQ_NEXT() dereferencing wrong pointer.

[trasz]

Okay, I think I get it now. How about this one?

[kib]

sys/kern/vfs_syscalls.c

469 ↗

(On Diff #21809)

Please use style. Fill the line up to column 72 at least, before splitting.

485 ↗

(On Diff #21809)

vfs_busy() returns error, and not a boolean. use if (vfs_busy() != 0) {

487 ↗

(On Diff #21809)

Sometimes you free random memory.
A comment should be added explaining why restart is needed.

[trasz]

Fixes suggested by kib@.

[kib]

I suggest asking Peter to test this with a load where parallel unmounts and fstatfs(2) (e.g. df(1)) are used.