vfs: introduce v_irflag and make v_type smaller
ClosedPublic
Actions

Authored by mjg on Dec 6 2019, 11:29 PM.

Details

Reviewers

jeff
kib

Group Reviewers

manpages

Commits

rS355537: vfs: introduce v_irflag and make v_type smaller

Summary

The current vnode layout is not smp-friendly by having frequently read data avoidably sharing cachelines with very frequently modified fields. In particular v_iflag inspected for VI_DOOMED can be found in the same line with v_usecount. Instead make it available in the same cacheline as the v_op, v_data and v_type which all get read all the time.

v_type is avoidably 4 bytes while the necessary data will easily fit in 1. Shrinking it frees up 3 bytes, 2 of which get used here to introduce a new flag: VIRF_DOOMED.

The patch below is the material part of the change + some fix ups. Tree-wide sweep was generated with coccinelle and the combined patch can be found here:
https://people.freebsd.org/~mjg/vfs-v_irflag-complete.diff

With this out of the way + the lockmgr changes we will be able to do further reshuffling.

Test Plan

will ask for a ports exp run
stress2

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 28021

Event Timeline

mjg created this revision.Dec 6 2019, 11:29 PM

Herald added a reviewer: manpages. · View Herald TranscriptDec 6 2019, 11:29 PM

Herald added subscribers: delphij, imp. · View Herald Transcript

Harbormaster completed remote builds in B28021: Diff 65339.Dec 6 2019, 11:30 PM

mjg mentioned this in D22664: vfs: eliminate v_iflag read from vn_lock.Dec 6 2019, 11:31 PM

mjg edited the summary of this revision. (Show Details)

rpokala added a subscriber: rpokala.Dec 6 2019, 11:59 PM

rpokala added inline comments.

sys/fs/nfsclient/nfs_clport.c
158	`VN_IS_DOOMED(nvp)`?
sys/kern/vfs_subr.c
3588	`VN_IS_DOOMED(vp)`?
3788	`VN_IS_DOOMED(vp)`?
sys/sys/vnode.h
106	If all `enum vtype` values couldn't fit in 8 bits, what would happen? Compile error? truncation?

mjg added inline comments.Dec 7 2019, 12:05 AM

sys/fs/nfsclient/nfs_clport.c
158	This and almost all other VI_DOOMED instances are patched in the sweep.
sys/kern/vfs_subr.c
3588	I think it's cleaner given the way the flag is set below to not use the macro in here.
3788	This dumps flags so it should not.
sys/sys/vnode.h
106	It fails stating truncation would occur.

kib accepted this revision.Dec 7 2019, 5:37 PM

kib added inline comments.

sys/sys/vnode.h
107	BTW I am quite sure that on Intels byte ops are faster then shorts. Bytes instructions have direct encoding while shorts require 16bit data prefix.

This revision is now accepted and ready to land.Dec 7 2019, 5:37 PM

mjg mentioned this in D22689: vfs: stop error checking vget(..., LK_RETRY, ...) in vfs_msync.Dec 7 2019, 5:37 PM

mjg added inline comments.Dec 7 2019, 6:46 PM

sys/sys/vnode.h
107	I did not check before hand, so I took a look now out of curiosity. For a bit test clang compiles int and short as testb with an offset, while bool with cmpb. Either way, the size is the same.