Page MenuHomeFreeBSD

devfs: introduce a per-dev lock to protect ->si_devsw
ClosedPublic

Authored by mjg on Mon, Nov 25, 4:52 PM.

Details

Summary

This allows bumping threadcount without taking the global devmtx lock.

markj reports it eliminates contention on said lock on a sample system with 32 instances of bhyve. Also it went went through stress testing while hosting syzkaller. No issues found.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

mjg created this revision.Mon, Nov 25, 4:52 PM
kib added a comment.Mon, Nov 25, 4:58 PM

This absolutely requires a bump for D_VERSION.
May be add explicit order between devmtx and si_threadlock to the witness static table.

mjg updated this revision to Diff 64857.Mon, Nov 25, 5:15 PM
  • bump D_VERSION
  • annotate the lock in witness
kib added inline comments.Mon, Nov 25, 9:03 PM
sys/sys/conf.h
174 ↗(On Diff #64857)

After closer look, can we avoid this bump ? si_threadlock is used (should be used) only be devfs internals, so if you move the lock into cdev_priv it should still work.

mjg updated this revision to Diff 64923.Wed, Nov 27, 1:31 AM
  • move the lock to cdev_privdata
kib accepted this revision.Wed, Nov 27, 11:25 AM

Except for the headers pollution, I am fine with the change.

sys/fs/devfs/devfs_int.h
43 ↗(On Diff #64923)

Is that needed ? Even if it is, devfs_int.h is included in 5 or 6 files at all, so it is easy to arrange sys/mutex.h before it in .c files.

This revision is now accepted and ready to land.Wed, Nov 27, 11:25 AM
mjg updated this revision to Diff 64939.Wed, Nov 27, 3:34 PM
  • drop _lock.h and _mtx.h

I included them towards self-contianment of the header file. universe MAKE_JUST_KERNELS builds just fine (except for powerpc* which were note tested)

This revision now requires review to proceed.Wed, Nov 27, 3:34 PM
kib accepted this revision.Wed, Nov 27, 4:00 PM
This revision is now accepted and ready to land.Wed, Nov 27, 4:00 PM
This revision was automatically updated to reflect the committed changes.