vfs: save on atomics on the root vnode for absolute lookups
ClosedPublic
Actions

Authored by mjg on Jan 30 2020, 10:35 AM.

Details

Reviewers

kib
jeff

Commits

rS357359: vfs: save on atomics on the root vnode for absolute lookups
rS357358: vfs: add vrefactn

Summary

There are 2 back-to-back atomics on the vnode, but we can check upfront if one is sufficient. Similarly we can handle relative lookups where current working directory == root directory.

Note lookup would be best off never unrefing the root vnode, but that's for later.

Diff Detail

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

mjg created this revision.Jan 30 2020, 10:35 AM

Herald added a subscriber: imp. · View Herald TranscriptJan 30 2020, 10:35 AM

Harbormaster completed remote builds in B29038: Diff 67502.Jan 30 2020, 10:36 AM

kib added inline comments.Jan 30 2020, 8:57 PM

sys/kern/vfs_lookup.c
427	So why not do this part in namei_handle_root() ? Because other places do not need v_usecount += 2, only += 1 ? Perhaps add a flag to namei_handle_root() then to select a variant, or pass 'n' to it directly.
sys/kern/vfs_subr.c
3024	You _must_ document all vref variants, even if only in the source code. VNASSERTs are not enough.

mjg added inline comments.Jan 30 2020, 9:06 PM

sys/kern/vfs_lookup.c
427	Yes, the other caller only takes one ref. How about renaming it to namei_get_root or similar instead?
sys/kern/vfs_subr.c
3024	ok, i'll create a separate review for this

kib added inline comments.Jan 30 2020, 9:38 PM

sys/kern/vfs_lookup.c
427	Instead of adding the arg ? My point is that removing vrefact() from name_handle/get_root is unfortunate.

mjg added inline comments.Jan 31 2020, 2:04 AM

sys/kern/vfs_lookup.c
427	well changing the name from 'handle' (which suggests everything is taken care of) to something which is more natural to only return the pointer was supposed to help here. if have other pending changes which imo will look better if refcount handling is moved away from that primitive, hence the patch in this form