Page MenuHomeFreeBSD

VOP_INACTIVE(9): clarify wording
ClosedPublic

Authored by asomers on Mar 15 2019, 4:27 PM.
Tags
None
Referenced Files
F103473587: D19596.id55118.diff
Mon, Nov 25, 11:58 AM
F103444112: D19596.id55115.diff
Mon, Nov 25, 2:50 AM
Unknown Object (File)
Sat, Nov 23, 12:57 PM
Unknown Object (File)
Fri, Nov 22, 7:36 AM
Unknown Object (File)
Tue, Nov 12, 2:01 PM
Unknown Object (File)
Tue, Oct 29, 9:41 PM
Unknown Object (File)
Sep 29 2024, 11:22 PM
Unknown Object (File)
Sep 24 2024, 9:08 AM
Subscribers

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

I believe a note that VOP_INACTIVE() call is not guaranteed is also due.

This revision is now accepted and ready to land.Mar 15 2019, 4:35 PM

Add note that there is no guarantee that VOP_INACTIVE will be called

This revision now requires review to proceed.Mar 15 2019, 4:38 PM
0mp added a subscriber: 0mp.

ok from manpages

This revision is now accepted and ready to land.Mar 15 2019, 5:25 PM
share/man/man9/VOP_INACTIVE.9
61 ↗(On Diff #55113)

I do not know what unclean unmounts are. It is not guaranteed to be called because e.g. it might be called while only owning a shared vnode lock, and non-sleeping upgrade failed due to other shared lockers.

Respond to kib's criticism

This revision now requires review to proceed.Mar 15 2019, 5:40 PM
share/man/man9/VOP_INACTIVE.9
53 ↗(On Diff #55113)

This seems a little less clear than before; it could be interpreted as "may be called sometimes when the kernel is still using the vnode."

I might leave this sentence alone, but bring up your "no guarantee" sentence to follow directly after it.

60–61 ↗(On Diff #55113)

Hm, can you clarify this sentence? It's called as part of vgonel() even during forced umount. So what is an unclean unmount in this context?

share/man/man9/VOP_INACTIVE.9
61 ↗(On Diff #55113)

@kib, which path would a VOP_INACTIVE() caller only hold a shared lock?

asomers added inline comments.
share/man/man9/VOP_INACTIVE.9
60–61 ↗(On Diff #55113)

I meant things like power loss, crash, etc. The filesystem musn't rely exclusively on VOP_INACTIVE to reclaim space in such cases.

share/man/man9/VOP_INACTIVE.9
61 ↗(On Diff #55113)

Oh, vput/vunref may have a failed lock upgrade. But they still set VI_OWEINACT, so eventually the last other holder should invoke VOP_INACTIVE (or vgonel will).

share/man/man9/VOP_INACTIVE.9
60–61 ↗(On Diff #55113)

We generally only document API guarantees with the understanding that they only apply if power is consistent and the system does not crash. If we added boilerplate about power loss/crash to every API it would bloat the manuals excessively and make it more difficult to read relevant information.

I don't think there's any particular reason these APIs are different in that respect.

share/man/man9/VOP_INACTIVE.9
61 ↗(On Diff #55115)

'while the vnode lock could not be upgraded to exclusive without sleeping'.

61 ↗(On Diff #55113)

The vnode could be vref-ed while VI_OWEINACT is set, and then if the lock mode is shared, it is still not inactivated. So no, there is no guarantee that vop_inactive necessary called before reclaim.

asomers marked an inline comment as done.

Use kib's preferred wording.

share/man/man9/VOP_INACTIVE.9
61 ↗(On Diff #55113)

The vnode could be vref-ed while VI_OWEINACT is set, and then if the lock mode is shared, it is still not inactivated. So no, there is no guarantee that vop_inactive necessary called before reclaim.

I don't follow. If it is vrefed, it gains a non-zero usecount and is marked VI_ACTIVE. In vgonel(), it will still be vinactive -> VOP_INACTIVE()ed before RECLAIM.

share/man/man9/VOP_INACTIVE.9
61 ↗(On Diff #55113)

VOP_INACTIVE() gerald is to be called on each v_usecount 1->0 transition. This is not guaranteed at least due to the shared locking.

The fact that we might be calling VOP_INACTIVE on next round of 1->0 transition or right before VOP_RECLAIM() call in vgonel() still does not satisfy the promise of calling VOP_INACTIVE on previous v_usecount zeroing.

This revision is now accepted and ready to land.Mar 15 2019, 6:46 PM
This revision was automatically updated to reflect the committed changes.