vn_lock_pair(): reasonably handle vp1 == vp2 case
ClosedPublic
Actions

Authored by kib on Nov 16 2023, 10:52 PM.

Details

Reviewers

jah
markj

Commits

rG22bac49b09a9: vn_lock_pair(): reasonably handle vp1 == vp2 case

Summary

Lock the vnode in the most exclusive lock mode requested, once.
All callers already ensure that vp1 != vp2 or are careful enough to only
unlock once otherwise.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

kib created this revision.Nov 16 2023, 10:52 PM

Herald added a subscriber: imp. · View Herald TranscriptNov 16 2023, 10:52 PM

kib requested review of this revision.Nov 16 2023, 10:52 PM

kib added a subscriber: pho.

markj added inline comments.Nov 16 2023, 11:11 PM

sys/kern/vfs_vnops.c
4048	This comment should mention that they may be equal.
4079	Don't we need to check `LK_NOSHARE` too?

kib marked 2 inline comments as done.Nov 16 2023, 11:21 PM

kib added inline comments.

sys/kern/vfs_vnops.c
4079	This is micro-optimization, not strictly needed for correctness. Added.

Update herald comment.
Optimize for LK_NOSHARE lock mode.

markj added inline comments.Nov 16 2023, 11:32 PM

sys/kern/vfs_vnops.c
4079	LK_NOSHARE seems to mean, "silently upgrade share lock requests to exclusive lock requests." I don't see how that's just an optimization. In particular, if the caller requests LK_SHARE, we should honour LK_NOSHARE and change the request to LK_EXCLUSIVE.

kib added inline comments.Nov 16 2023, 11:46 PM

sys/kern/vfs_vnops.c
4079	The change happens inside lockmgr. If passed vnode is lk_noshare and locked, it is locked exlusive. Optimization then consists in avoiding unlock.