On Thu, Jan 18, 2024 at 03:54:16PM +0100, Greg KH wrote: > On Thu, Jan 18, 2024 at 02:38:58PM +0200, Heikki Krogerus wrote: > > It is "being used" when something (user) acquires reference to the > > role switch that it provides. The "user" is in most cases USB Type-C > > port driver - what ever knows the role the port needs to be in. > > > > USB role switch is meant to act as a "resource" like any other. So > > when you acquire for example a GPIO, the gpiodev driver is pinned down > > by calling module_get() just like this driver is doing to the switch > > provider. > > So again, if the hardware is present in the system, the module reference > count will always be increased and can never be removed no matter what a > user does? That feels wrong if so. It's not quite that bad. Even if the USB role-switch hardware is present in the system, the module controlling it can be removed if the role-switch resource isn't being used (which will cause the module count will go back down to 0). This may require the user first to remove the module that's using the resource. > > > Any module should be able to be removed any time, unless there is a > > > "code" requirement of it being present. The driver model structures > > > should make this possible if used properly. Trying to much around in > > > various parent devices and the drivers bound to parents should NEVER be > > > done as happens here, sorry I missed that in the review process. > > > > If this is not something that this kind of device class should be > > doing, then let's remove the whole thing. But if that is the case, > > then shouldn't we remove that from all the other bus and device class > > drivers as well? > > I started to remove it many years ago, but then something prevented that > as there was actually some valid uses, but I can't remember at the > moment. Try taking it out and see what happens! :) The problem here is that a device is being used by module B that isn't a descendant of the device's driver module A. If B didn't take a reference to A then A could be unloaded while B was still using the device. When B finally drops its reference to the device, the driver-model release call would get a NULL-pointer violation, since the code for the release method was part of A and has been unloaded from memory. In the current code, B takes a module reference to A and drops that reference after dropping its reference to the device. The bug that Xu Yang wants to fix (the original reason for this discussion) has to do with the way B drops its module reference to A. It gets the module handle of A by looking up the owner of the device's driver -- and that approach doesn't work if the device has been unbound from its driver, for the obvious reason. The proper solution to this problem is for B to cache A's handle at the time when it first acquires the module reference. Alan Stern
