On Mon, Feb 24, 2020 at 7:13 PM Jason Gunthorpe <jgg@xxxxxxxxxxxx> wrote: > > On Mon, Feb 24, 2020 at 02:49:55AM -0800, Selvin Xavier wrote: > > > > @@ -1785,32 +1777,23 @@ static int __init bnxt_re_mod_init(void) > > > > static void __exit bnxt_re_mod_exit(void) > > { > > - struct bnxt_re_dev *rdev, *next; > > - LIST_HEAD(to_be_deleted); > > + struct bnxt_re_dev *rdev; > > > > + flush_workqueue(bnxt_re_wq); > > What is this for? Usually flushing a work queue before preventing new > work from queueing (ie via unregister) is racy. To flush out any netdev events scheduled to be handled by bnxt_re_task. Mainly to wait for case where we are in the middle of NETDEV_REGISTER event handled from bnxt_re_task. > > > mutex_lock(&bnxt_re_dev_lock); > > - /* Free all adapter allocated resources */ > > - if (!list_empty(&bnxt_re_dev_list)) > > - list_splice_init(&bnxt_re_dev_list, &to_be_deleted); > > - mutex_unlock(&bnxt_re_dev_lock); > > - /* > > - * Cleanup the devices in reverse order so that the VF device > > - * cleanup is done before PF cleanup > > - */ > > - list_for_each_entry_safe_reverse(rdev, next, &to_be_deleted, list) { > > - ibdev_info(&rdev->ibdev, "Unregistering Device"); > > - /* > > - * Flush out any scheduled tasks before destroying the > > - * resources > > + list_for_each_entry(rdev, &bnxt_re_dev_list, list) { > > + /* VF device removal should be called before the removal > > + * of PF device. Queue VFs unregister first, so that VFs > > + * shall be removed before the PF during the call of > > + * ib_unregister_driver. Commands to any VFs after the > > + * PF removal will timeout and driver will proceed with > > + * unregisteration and free up the host resources. > > */ > > - flush_workqueue(bnxt_re_wq); > > - bnxt_re_dev_stop(rdev); > > - bnxt_re_ib_uninit(rdev); > > - /* Acquire the rtnl_lock as the L2 resources are freed here */ > > - rtnl_lock(); > > - bnxt_re_remove_device(rdev); > > - rtnl_unlock(); > > + if (rdev->is_virtfn) > > + ib_unregister_device_queued(&rdev->ibdev); > > Why do it queued? Just call ib_unregister_device(). Otherwise it won't > order reliably. Sure. Will change it. > > But be very careful about lifetime. All the other drivers had problems > managing the lifetime of the pointers in their device lists. > > Jason