Page MenuHomeFreeBSD

bus: Take the topolock in driver_module_handler()
Needs ReviewPublic

Authored by imp on Tue, Oct 28, 7:49 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Nov 4, 9:20 AM
Unknown Object (File)
Mon, Nov 3, 7:51 AM
Unknown Object (File)
Fri, Oct 31, 12:57 AM
Unknown Object (File)
Fri, Oct 31, 12:57 AM
Unknown Object (File)
Thu, Oct 30, 7:58 PM
Unknown Object (File)
Wed, Oct 29, 2:01 AM
Unknown Object (File)
Wed, Oct 29, 1:46 AM
Unknown Object (File)
Tue, Oct 28, 10:17 PM
Subscribers

Details

Reviewers
jhb
markj
Summary

And sprinkler some asserts. Right now all module handlers are called
with Giant, but I'd like to drop that so push the newbus locking one
step further.

Sponsored by: Netflix

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 68188
Build 65071: arc lint + arc unit

Event Timeline

imp requested review of this revision.Tue, Oct 28, 7:49 PM

I haven't yet looked at Mark's changes to see how they conflict.

sys/kern/subr_bus.c
5273

Arguably, we should only create the devclass for MOD_LOAD (and pass false instead of true for MOD_UNLOAD and MOD_QUIESCE), and we should perhaps run the chained handlers not under the topo_lock (requiring the chained handlers to manage their own locking). You could fix both of those by pushing the devclass_find_internal() down into each case below, but that that's kind of tedious. This is probably fine for now.

imp added inline comments.
sys/kern/subr_bus.c
5273

Agreed. I'll do that as new patches in the series.

This change depends on D53451, not the other way around. If you commit just this one, the devclass_find_internal() assertion will fail in some cases.