Re: [PATCH] RDMA: don't ignore client->add() failures

[Jason Gunthorpe <jgg@ziepe.ca>]

On Wed, Mar 29, 2023 at 07:17:26AM +0900, Tetsuo Handa wrote:
> 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?

Yes, if it fails. It isn't returning a failure code, so of course it
doesn't fail.

You changed this code and forced it to always fail, so of course it
radically changes everything about the bug you are looking at.

> 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().

On the success path this stuff happens during dellink

Jason