Re: [PATCH V1 mlx5-next 4/4] vfio/mlx5: Run the SAVE state command in an async mode

[Alex Williamson <alex.williamson@redhat.com>]

On Sun, 8 May 2022 16:10:53 +0300
Yishai Hadas <yishaih@nvidia.com> wrote:
> diff --git a/drivers/vfio/pci/mlx5/cmd.h b/drivers/vfio/pci/mlx5/cmd.h
> index 2a20b7435393..d053d314b745 100644
> --- a/drivers/vfio/pci/mlx5/cmd.h
> +++ b/drivers/vfio/pci/mlx5/cmd.h
> @@ -10,10 +10,20 @@
>  #include <linux/vfio_pci_core.h>
>  #include <linux/mlx5/driver.h>
>  
> +struct mlx5vf_async_data {
> +	struct mlx5_async_work cb_work;
> +	struct work_struct work;
> +	int status;
> +	u32 pdn;
> +	u32 mkey;
> +	void *out;
> +};
> +
>  struct mlx5_vf_migration_file {
>  	struct file *filp;
>  	struct mutex lock;
>  	bool disabled;
> +	u8 is_err:1;

Convert @disabled to bit field as well to pack these?

...
> @@ -558,6 +592,13 @@ static int mlx5vf_pci_probe(struct pci_dev *pdev,
>  		return -ENOMEM;
>  	vfio_pci_core_init_device(&mvdev->core_device, pdev, &mlx5vf_pci_ops);
>  	mlx5vf_cmd_set_migratable(mvdev);
> +	if (mvdev->migrate_cap) {
> +		mvdev->cb_wq = alloc_ordered_workqueue("mlx5vf_wq", 0);
> +		if (!mvdev->cb_wq) {
> +			ret = -ENOMEM;
> +			goto out_free;
> +		}
> +	}

Should this be rolled into mlx5vf_cmd_set_migratable(), updating the
function to return -errno?

>  	ret = vfio_pci_core_register_device(&mvdev->core_device);
>  	if (ret)
>  		goto out_free;
> @@ -566,8 +607,11 @@ static int mlx5vf_pci_probe(struct pci_dev *pdev,
>  	return 0;
>  
>  out_free:
> -	if (mvdev->migrate_cap)
> +	if (mvdev->migrate_cap) {
>  		mlx5vf_cmd_remove_migratable(mvdev);
> +		if (mvdev->cb_wq)
> +			destroy_workqueue(mvdev->cb_wq);
> +	}
>  	vfio_pci_core_uninit_device(&mvdev->core_device);
>  	kfree(mvdev);
>  	return ret;
> @@ -578,8 +622,10 @@ static void mlx5vf_pci_remove(struct pci_dev *pdev)
>  	struct mlx5vf_pci_core_device *mvdev = dev_get_drvdata(&pdev->dev);
>  
>  	vfio_pci_core_unregister_device(&mvdev->core_device);
> -	if (mvdev->migrate_cap)
> +	if (mvdev->migrate_cap) {
>  		mlx5vf_cmd_remove_migratable(mvdev);
> +		destroy_workqueue(mvdev->cb_wq);
> +	}
>  	vfio_pci_core_uninit_device(&mvdev->core_device);
>  	kfree(mvdev);
>  }

This looks like more evidence for expanding remove_migratable(),
rolling this in as well.  If this workqueue were setup in
set_migratable() we'd not need the special condition to test if cb_wq
is NULL while migrate_cap is set.  Thanks,

Alex