Incase of usnic, only the first vf for each enic pf (and not all vfs) registers with ib. However, each vf that registers with ib has it’s own netdevice. usnic_ib_ibdev_list_lock protects a list of the those “first” vfs that have registered with ib. This series : https://github.com/jgunthorpe/linux/commits/device_locking_cleanup , will not allow us to get rid of this list (and hence usnic_ib_ibdev_list_lock) completely. We can modify the current patch such that driver doesn’t hold any lock while doing ib_unregsiter_device() Thanks, Parvi > On Feb 5, 2019, at 7:22 PM, Jason Gunthorpe <jgg@xxxxxxxx> wrote: > > On Tue, Feb 05, 2019 at 05:24:37PM -0800, Parvi Kaustubhi wrote: >> There is a dead lock in usnic ib_register and netdev_notify >> path. >> >> usnic_ib_discover_pf() >> | mutex_lock(&usnic_ib_ibdev_list_lock); >> | usnic_ib_device_add(); >> | ib_register_device() >> | usnic_ib_query_port() >> | mutex_lock(&us_ibdev->usdev_lock); >> | ib_get_eth_speed() >> | rtnl_lock() > >> order of lock: &usnic_ib_ibdev_list_lock -> usdev_lock -> rtnl_lock >> >> rtnl_lock() >> | usnic_ib_netdevice_event() >> | mutex_lock(&usnic_ib_ibdev_list_lock); >> >> order of lock: rtnl_lock -> &usnic_ib_ibdev_list_lock >> >> Solution is to add notifier blocks (netdev_nb and inet_nb) for each pf >> and use container_of to lookup usnic_ib_dev while handling netdev/ inet >> events. This eliminates the need to acquire usnic_ib_ibdev_list_lock to >> obtain the corresponding usnic_ib_dev pointer. > > What you should do is use this series: > > https://github.com/jgunthorpe/linux/commits/device_locking_cleanup > > And make a patch to convert usnic to use ib_device_set_netdev() and > ib_device_get_by_netdev() and try to eliminate as many uses of the > usnic_ib_ibdev_list as you can. > > Also, drivers should not hold any locks while calling > ib_unregister_device() > > Jason
