On Thu, 18 Mar 2021 14:38:53 -0400 Tony Krowiak akrowiak@linux.ibm.com wrote:
On 3/17/21 7:17 PM, Halil Pasic wrote:
On Wed, 10 Mar 2021 10:05:59 -0500 Tony Krowiak akrowiak@linux.ibm.com wrote:
ret = vfio_ap_mdev_reset_queues(mdev);
matrix_mdev = mdev_get_drvdata(mdev);Is it guaranteed that matrix_mdev can't be NULL here? If yes, please remind me of the mechanism that ensures this.
/** If the KVM pointer is in the process of being set, wait until* the process has completed.*/wait_event_cmd(matrix_mdev->wait_for_kvm,matrix_mdev->kvm_busy == false,mutex_unlock(&matrix_dev->lock),mutex_lock(&matrix_dev->lock));if (matrix_mdev->kvm)ret = vfio_ap_mdev_reset_queues(mdev);elseret = -ENODEV;Didn't we agree to make the call to vfio_ap_mdev_reset_queues() unconditional again (for reference please take look at Message-ID: 64afa72c-2d6a-2ca1-e576-34e15fa579ed@linux.ibm.com)?
How about this:
Looks good. I will check the mdev code if the checkeck is really needed. I'm curious when the sysfs files associated with a new mdev are created. My guess is that this one comes in via a device specific file (not the parent like in case of the create), and that those may be created after the create. But we can get rid of the check any time so I really don't see it as something that would preclude merging this.
Regards, Halil
static ssize_t vfio_ap_mdev_ioctl(struct mdev_device *mdev, unsigned int cmd, unsigned long arg) { int ret = 0; struct ap_matrix_mdev *matrix_mdev;
... case VFIO_DEVICE_RESET: matrix_mdev = mdev_get_drvdata(mdev); WARN(!matrix_mdev, "Driver data missing from mdev!!");
if (matrix_mdev) { /* * If the KVM pointer is in the process of being set, wait until * the process has completed. */ wait_event_cmd(matrix_mdev->wait_for_kvm, matrix_mdev->kvm_busy == false, mutex_unlock(&matrix_dev->lock), mutex_lock(&matrix_dev->lock));
ret = vfio_ap_mdev_reset_queues(mdev); } break; ...
return ret; }
Regards, Halil