On 2023/03/29 1:18, Jason Gunthorpe wrote: > On Tue, Mar 28, 2023 at 11:59:48PM +0900, Tetsuo Handa wrote: >> Without this patch, __ib_unregister_device(device) is not called because >> enable_device_and_get() returns 0 because add_client_context() returns 0 >> because add_client_context() ignores client->add() failures. As a result, >> device's refcount remains 7, which later prevents unregister_netdevice() >> from unregistering this device. > > That is completely correct, the device was successfully registered > without one of the clients. ib_register_device() is responsible for unregistering that device (by calling __ib_unregister_device(device)) if ib_register_device() failed to initialize a device, isn't it? > > It seems to me all this has done done is make it so the device doesn't > register and that will hide the refcount bug because the bug is > actually something that happens during operation - and we never get to > operation now. If ib_register_device() were not responsible for unregistering that device when ib_register_device() failed to initialize a device, who is responsible for unregistering that device? The caller of ib_register_device() (i.e. siw_device_register() from siw_newlink()) is assuming that somebody will call __ib_unregister_device(), but nobody is calling __ib_unregister_device().
