Page MenuHomeFreeBSD

superio: Use a bus_child_deleted method to free ivars for children
ClosedPublic

Authored by jhb on Oct 31 2024, 8:36 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Jan 12, 10:35 PM
Unknown Object (File)
Dec 9 2024, 6:26 AM
Unknown Object (File)
Dec 5 2024, 10:01 AM
Unknown Object (File)
Dec 5 2024, 3:55 AM
Unknown Object (File)
Nov 29 2024, 3:10 AM
Unknown Object (File)
Nov 20 2024, 1:51 AM
Unknown Object (File)
Nov 15 2024, 11:20 AM
Unknown Object (File)
Nov 2 2024, 2:16 PM
Subscribers
None

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Owners added a reviewer: Restricted Owners Package.Oct 31 2024, 8:36 PM
jhb requested review of this revision.Oct 31 2024, 8:36 PM

(more a comment in passing than a request for change)

sys/dev/superio/superio.c
780

After seeing a bunch of these, I kinda wonder if there's some way we can refactor this into newbus.
Though given that, at best, the ivar might have a resource list and there's no 'baseclass' requirement, that might be hard.

This revision is now accepted and ready to land.Oct 31 2024, 9:04 PM
sys/dev/superio/superio.c
780

Maybe the actual free could be pulled up into new-bus similar how softc allocation is managed, but the resource_list bit means you'd end up with a child_deleted method anyway, so I'm not sure it buys much.

There are a couple of related things it might be nice to fix to be more consistent in this area:

  1. Bus drivers that allocate ivars should probably always be using a custom bus_add_child method where these are initialized. Right now some buses setup ivars in the caller of BUS_ADD_CHILD. OTOH, I think I would actually prefer to remove BUS_ADD_CHILD and switch to a BUS_CHILD_ADDED that is always called from device_add_child(). I think the latter approach would be more robust, and it would also be a bit more obvious that bus_child_deleted should be paired with bus_child_added.
  1. bus_child_deleted methods should probably be using resource_list_purge() instead of resource_list_free(). Arguably resource_list_free() should just be resource_list_purge()?