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.
> 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.
But be very careful about lifetime. All the other drivers had problems
managing the lifetime of the pointers in their device lists.
Jason
