RE: [PATCH v4 10/10] vfio/qat: Add vfio_pci driver for Intel QAT VF devices

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Thanks for the comments, Kevin.
On Tuesday, March 5, 2024 3:38 PM, Tian, Kevin <kevin.tian@xxxxxxxxx> wrote:
> > From: Zeng, Xin <xin.zeng@xxxxxxxxx>
> > Sent: Wednesday, February 28, 2024 10:34 PM
> > +
> > +static long qat_vf_precopy_ioctl(struct file *filp, unsigned int cmd,
> > +				 unsigned long arg)
> > +{
> > +	struct qat_vf_migration_file *migf = filp->private_data;
> > +	struct qat_vf_core_device *qat_vdev = migf->qat_vdev;
> > +	struct qat_mig_dev *mig_dev = qat_vdev->mdev;
> > +	struct vfio_precopy_info info;
> > +	loff_t *pos = &filp->f_pos;
> > +	unsigned long minsz;
> > +	int ret = 0;
> > +
> > +	if (cmd != VFIO_MIG_GET_PRECOPY_INFO)
> > +		return -ENOTTY;
> > +
> > +	minsz = offsetofend(struct vfio_precopy_info, dirty_bytes);
> > +
> > +	if (copy_from_user(&info, (void __user *)arg, minsz))
> > +		return -EFAULT;
> > +	if (info.argsz < minsz)
> > +		return -EINVAL;
> > +
> > +	mutex_lock(&qat_vdev->state_mutex);
> > +	if (qat_vdev->mig_state != VFIO_DEVICE_STATE_PRE_COPY) {
> > +		mutex_unlock(&qat_vdev->state_mutex);
> > +		return -EINVAL;
> > +	}
> 
> what about PRE_COPY_P2P?

Good catch, will add it in next version.

> 
> > +static struct qat_vf_migration_file *
> > +qat_vf_save_device_data(struct qat_vf_core_device *qat_vdev, bool
> > pre_copy)
> > +{
> > +	struct qat_vf_migration_file *migf;
> > +	int ret;
> > +
> > +	migf = kzalloc(sizeof(*migf), GFP_KERNEL);
> > +	if (!migf)
> > +		return ERR_PTR(-ENOMEM);
> > +
> > +	migf->filp = anon_inode_getfile("qat_vf_mig", &qat_vf_save_fops,
> > +					migf, O_RDONLY);
> > +	ret = PTR_ERR_OR_ZERO(migf->filp);
> > +	if (ret) {
> > +		kfree(migf);
> > +		return ERR_PTR(ret);
> > +	}
> > +
> > +	stream_open(migf->filp->f_inode, migf->filp);
> > +	mutex_init(&migf->lock);
> > +
> > +	if (pre_copy)
> > +		ret = qat_vf_save_setup(qat_vdev, migf);
> > +	else
> > +		ret = qat_vf_save_state(qat_vdev, migf);
> > +	if (ret) {
> > +		fput(migf->filp);
> > +		return ERR_PTR(ret);
> > +	}
> 
> lack of kfree(migf). Using goto can avoid such typo in error unwind.

kfree(migf) will be invoked in fput. This was pointed out by
Shameer and I updated it in v2. :-)

> 
> > +
> > +static struct file *qat_vf_pci_step_device_state(struct
> qat_vf_core_device
> > *qat_vdev, u32 new)
> > +{
> > +	u32 cur = qat_vdev->mig_state;
> > +	int ret;
> > +
> > +	if ((cur == VFIO_DEVICE_STATE_RUNNING && new ==
> > VFIO_DEVICE_STATE_RUNNING_P2P) ||
> > +	    (cur == VFIO_DEVICE_STATE_PRE_COPY && new ==
> > VFIO_DEVICE_STATE_PRE_COPY_P2P)) {
> > +		ret = qat_vfmig_suspend(qat_vdev->mdev);
> > +		if (ret)
> > +			return ERR_PTR(ret);
> > +		return NULL;
> > +	}
> 
> could you arrange the transitions in pair as mlx does e.g. following
> above is the reverse one calling qat_vfmig_resume()?

Sure, will update it in next version.

> 
> > +
> > +MODULE_LICENSE("GPL");
> > +MODULE_AUTHOR("Intel Corporation");
> 
> please use your name as the author.

Sure, will do.






[Index of Archives]     [Kernel]     [Gnu Classpath]     [Gnu Crypto]     [DM Crypt]     [Netfilter]     [Bugtraq]
  Powered by Linux