Re: [PATCH for-next v2 3/3] RDMA/bnxt_re: Use driver_unregister and unregistration API

[Jason Gunthorpe <jgg@mellanox.com>]

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