Page MenuHomeFreeBSD

bus: Cleanup device_probe_child()
ClosedPublic

Authored by mav on Sep 24 2021, 7:34 PM.

Details

Summary

When device driver probe method returns 0, i.e. absolute priority, do not remove its class from the device just to set it back few lines later, that may change the device unit number, etc. and after which we'd better call the probe again.

If during search we found some driver with absolute priority, we do not need to set device driver and class since we haven't removed them before.

It should not happen, but if second probe method call failed, remove the driver and possibly the class from the device as it was when we started.

Diff Detail

Repository
rG 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

mav requested review of this revision.Sep 24 2021, 7:34 PM
mav created this revision.

This looks good... I have one question though...

sys/kern/subr_bus.c
2215

Why is the hasclass test needed here? If we've probed the device and this second probe fails, shouldn't we always set devclass to NULL?

sys/kern/subr_bus.c
2215

There can be other drivers of the same class. And if we haven't set the class for the device (it can be set by the bus) we should not remove it.

OK I'm happy. A suggested place to add a comment, but if you don't want to, I can do it as a followup.

sys/kern/subr_bus.c
2078

We should add a comment here that says we try to preserve the devclass of an object that the bus has set maybe?

2215

Makes sense... I'd forgotten that was why this was here...

This revision is now accepted and ready to land.Sep 24 2021, 8:46 PM
jhb added inline comments.
sys/kern/subr_bus.c
2187

Unindenting all of this with the early failure is a nice cleanup.

2195

Why did you move this earlier? I guess it matches the main probe loop above? Either order is probably fine though.

2215

This is what lets hinted devices work for example. I think that's the main case of pre-set device classes. devctl set driver is another instance.

sys/kern/subr_bus.c
2195

Yes, just to match the above code. Also the previous code missed error handling, so would need to be touched any way.

This revision was automatically updated to reflect the committed changes.
sys/kern/subr_bus.c
2089

I still think this should be >= DS_ALIVE... But I don't know if it is possible to get here for >.

2215

Yea, hinted devices are set by the bus and probe just approves / rejects the device.
Disabled devices will also be similar, but will be in DS_ALIVE.